Bug 27289

Summary: Click/releases on scrollbars misbehave when mouse move is missed
Product: WebKit Reporter: Viet-Trung Luu <viettrungluu>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, hyatt, manyoso
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Demonstrates bug.
none
Fixes bug 27289 by updating m_lastScrollbarUnderMouse on mouse down.
none
Fixes bug 27289 (same as previous version of patch) and adds test cases.
none
Patch and test cases for bug 27289.
hyatt: review-
Patch and test cases for bug 27289 hyatt: review+

Description Viet-Trung Luu 2009-07-14 22:18:00 PDT
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.
Comment 1 Viet-Trung Luu 2009-07-14 23:32:00 PDT
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.
Comment 2 Viet-Trung Luu 2009-07-16 09:41:43 PDT
Created attachment 32881 [details]
Fixes bug 27289 (same as previous version of patch) and adds test cases.
Comment 3 Viet-Trung Luu 2009-07-16 09:54:33 PDT
Created attachment 32882 [details]
Patch and test cases for bug 27289.

Got rid of duplicate Changelog entry.
Comment 4 Adam Barth 2009-07-17 23:05:46 PDT
This bug drives me nuts.  I think Hyatt is the expert on scrolling.
Comment 5 Dave Hyatt 2009-07-22 10:53:40 PDT
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?
Comment 6 Viet-Trung Luu 2009-07-22 13:55:22 PDT
Created attachment 33287 [details]
Patch and test cases for bug 27289

Added a helper method to avoid repeating code.
Comment 7 Dave Hyatt 2009-07-22 14:34:23 PDT
Comment on attachment 33287 [details]
Patch and test cases for bug 27289

r=me
Comment 8 Adam Treat 2009-07-23 09:56:12 PDT
Landed with r46273.