Bug 33062 – Middle clicking the primary scroll bars on Chromium Linux triggers a paste event

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Steve VanDeBogart]

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.

[Steve VanDeBogart]

Created attachment 45674 [details]
Check click point against content area.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45674 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 45674 [details]
Check click point against content area.

This should be easy to test via a LayoutTest, no?

[Eric Seidel (no email)]

Comment on attachment 45674 [details]
Check click point against content area.

Needs a LayoutTest or explanation of why one is impossible.

[Steve VanDeBogart]

Created attachment 45689 [details]
Add a layout test for middle clicking scrollbars.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45689 [details] without any errors.

[Evan Martin]

+julie, who has an opinion on setTimeout in layout tests

[Steve VanDeBogart]

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.

[Steve VanDeBogart]

Created attachment 45690 [details]
remove setTimeout from new layout test

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45690 [details] without any errors.

[Adam Langley]

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.)

[Adam Barth]

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.

[Steve VanDeBogart]

Created attachment 46023 [details]
A better test for the main scrollbars

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 46023 [details] without any errors.

[Steve VanDeBogart]

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)

[Eric Seidel (no email)]

Comment on attachment 45690 [details]
remove setTimeout from new layout test

Clearing Adam Barth's r+ from this obsolete patch.

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

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. :(

[Eric Seidel (no email)]

The test appears to outright fail on Gtk.  Either we can skip it on Gtk or roll this out.

[Eric Seidel (no email)]

Committed r53784: <http://trac.webkit.org/changeset/53784>

[Eric Seidel (no email)]

Skipped the test on Gtk for now.

[Tim Horton]

Also skipping on mac: https://bugs.webkit.org/show_bug.cgi?id=81410