Bug 27289 – Click/releases on scrollbars misbehave when mouse move is missed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Viet-Trung Luu]

Created attachment 32765 [details]
Demonstrates bug.

The bug is in WebKit/WebCore/page/EventHandler.cpp.

When a mouse click occurs on a scrollbar, but is not preceded by a mouse move onto that scrollbar, the release is not processed correctly.

This occurs in a number of situations:
- when you move the mouse after activating a context menu, and refocus onto the scrollbar without moving the mouse again after the context menu is gone (this is Google Chromium bug 6823 <http://code.google.com/p/chromium/issues/detail?id=6823>;
- when a scrollbar appears due to some event (and your mouse just happens to be in the right place to click).

This applies to enabled and disabled scrollbars; see also bug 19033 <https://bugs.webkit.org/show_bug.cgi?id=19033>.

The "context menu" incarnation happens on all current (including dev) versions of Chromium, on Safari 3.x and 4.x on Windows; it does not occur on Safari 4.x on Mac (don't know about 3.x; perhaps a Safari developer "fixed" it by creating a mouse move event after the context menu is dismissed?).

The "appearing scrollbar" incarnation happens on everything I've tried (Chromium, Safari).

Patch coming in a jiffy, as soon as I've run tests.

[Viet-Trung Luu]

Created attachment 32769 [details]
Fixes bug 27289 by updating m_lastScrollbarUnderMouse on mouse down.

EventHandler::m_lastScrollbarUnderMouse is currently only updated on mouse move and not on mouse down, which makes it fail (or at least behave oddly) when a mouse down on a scrollbar occurs without a mouse move onto the scrollbar; this patch fixes this.

[Viet-Trung Luu]

Created attachment 32881 [details]
Fixes bug 27289 (same as previous version of patch) and adds test cases.

[Viet-Trung Luu]

Created attachment 32882 [details]
Patch and test cases for bug 27289.

Got rid of duplicate Changelog entry.

[Adam Barth]

This bug drives me nuts.  I think Hyatt is the expert on scrolling.

[Dave Hyatt]

Comment on attachment 32882 [details]
Patch and test cases for bug 27289.

One minor comment.  You repeat the same if statement block twice. Can you make that into a helper function, e.g., something like updateLastScrollbarUnderMouse?

[Viet-Trung Luu]

Created attachment 33287 [details]
Patch and test cases for bug 27289

Added a helper method to avoid repeating code.

[Dave Hyatt]

Comment on attachment 33287 [details]
Patch and test cases for bug 27289

r=me

[Adam Treat]

Landed with r46273.