Bug 33062

Summary: Middle clicking the primary scroll bars on Chromium Linux triggers a paste event
Product: WebKit Reporter: Steve VanDeBogart <vandebo>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: agl, bweinstein, commit-queue, eric, evan, gns, jparent, thorton, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Check click point against content area.
eric: review-
Add a layout test for middle clicking scrollbars.
none
remove setTimeout from new layout test
abarth: commit-queue-
A better test for the main scrollbars none

Description Steve VanDeBogart 2009-12-30 11:19:25 PST
On Chromium Linux, middle clicking the primary scroll bars (along the right or bottom) triggers a paste event. Firefox on Linux does not fire a paste event in this case.
Comment 1 Steve VanDeBogart 2009-12-30 11:22:53 PST
Created attachment 45674 [details]
Check click point against content area.
Comment 2 WebKit Review Bot 2009-12-30 11:27:32 PST
style-queue ran check-webkit-style on attachment 45674 [details] without any errors.
Comment 3 Eric Seidel (no email) 2009-12-30 11:47:40 PST
Comment on attachment 45674 [details]
Check click point against content area.

This should be easy to test via a LayoutTest, no?
Comment 4 Eric Seidel (no email) 2009-12-30 11:50:55 PST
Comment on attachment 45674 [details]
Check click point against content area.

Needs a LayoutTest or explanation of why one is impossible.
Comment 5 Steve VanDeBogart 2009-12-30 17:22:59 PST
Created attachment 45689 [details]
Add a layout test for middle clicking scrollbars.
Comment 6 WebKit Review Bot 2009-12-30 17:26:29 PST
style-queue ran check-webkit-style on attachment 45689 [details] without any errors.
Comment 7 Evan Martin 2009-12-30 17:29:43 PST
+julie, who has an opinion on setTimeout in layout tests
Comment 8 Steve VanDeBogart 2009-12-30 17:38:39 PST
Hmmm.  I copied scrollbar-miss-mousemove.html and the setTimeout came from there. The test seems to run ok without setTimeout.  New patch coming up.
Comment 9 Steve VanDeBogart 2009-12-30 17:41:14 PST
Created attachment 45690 [details]
remove setTimeout from new layout test
Comment 10 WebKit Review Bot 2009-12-30 17:42:01 PST
style-queue ran check-webkit-style on attachment 45690 [details] without any errors.
Comment 11 Adam Langley 2010-01-04 12:12:10 PST
I don't get why !hitTestResult.scrollbar() doesn't catch this case but, assuming that there's a good reason: LGTM.

(Note: I am not a WebKit reviewer. You need a real r+ before landing.)
Comment 12 Adam Barth 2010-01-04 21:09:15 PST
Comment on attachment 45690 [details]
remove setTimeout from new layout test

(In reply to comment #11)
> I don't get why !hitTestResult.scrollbar() doesn't catch this case but,
> assuming that there's a good reason: LGTM.

Steve, can you answer this question before landing this patch?  Thanks.
Comment 13 Steve VanDeBogart 2010-01-06 22:44:07 PST
Created attachment 46023 [details]
A better test for the main scrollbars
Comment 14 WebKit Review Bot 2010-01-06 22:46:40 PST
style-queue ran check-webkit-style on attachment 46023 [details] without any errors.
Comment 15 Steve VanDeBogart 2010-01-06 22:58:06 PST
After looking at this code further, it seems that hitTestResult.scrollbar()
should detect the main scrollbars but doesn't.  

EventHandler::hitTestResultAtPoint tries to do the check for them with
view->scrollbarAtPoint() but never gets to that chunk of code because
n->renderer()->isWidget() is false (where n is HitTestResult.innerNode()).  But
fixing this chunk of code (if it is actually broken) is above my head.

There appears to be precedent for working around this issue in
WebView::gestureNotify (WebKit/win/WebView.cpp)
Comment 16 Eric Seidel (no email) 2010-01-06 23:43:14 PST
Comment on attachment 45690 [details]
remove setTimeout from new layout test

Clearing Adam Barth's r+ from this obsolete patch.
Comment 17 WebKit Commit Bot 2010-01-22 21:11:31 PST
Comment on attachment 46023 [details]
A better test for the main scrollbars

Clearing flags on attachment: 46023

Committed r53758: <http://trac.webkit.org/changeset/53758>
Comment 18 WebKit Commit Bot 2010-01-22 21:11:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 19 Eric Seidel (no email) 2010-01-24 14:12:05 PST
This caused failures on the Gtk Bot:
http://build.webkit.org/results/GTK%20Linux%20Release/r53758%20(8029)/scrollbars/scrollbar-middleclick-nopaste-pretty-diff.html

Please fix or roll out. :(
Comment 20 Eric Seidel (no email) 2010-01-24 14:21:18 PST
The test appears to outright fail on Gtk.  Either we can skip it on Gtk or roll this out.
Comment 21 Eric Seidel (no email) 2010-01-24 14:30:49 PST
Committed r53784: <http://trac.webkit.org/changeset/53784>
Comment 22 Eric Seidel (no email) 2010-01-24 14:32:05 PST
Skipped the test on Gtk for now.
Comment 23 Tim Horton 2012-03-16 14:50:17 PDT
Also skipping on mac: https://bugs.webkit.org/show_bug.cgi?id=81410