Bug 33062

Summary:

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

Product:

WebKit

Reporter:

Steve VanDeBogart <vandebo>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

agl, bweinstein, commit-queue, eric, evan, gustavo, jparent, thorton, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

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

Steve VanDeBogart

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

Attachments
Check click point against content area. (2.36 KB, patch) 2009-12-30 11:22 PST, Steve VanDeBogart	eric: review-	Details Formatted Diff Diff
Add a layout test for middle clicking scrollbars. (5.31 KB, patch) 2009-12-30 17:22 PST, Steve VanDeBogart	no flags	Details Formatted Diff Diff
remove setTimeout from new layout test (5.28 KB, patch) 2009-12-30 17:41 PST, Steve VanDeBogart	abarth: commit-queue-	Details Formatted Diff Diff
A better test for the main scrollbars (5.23 KB, patch) 2010-01-06 22:44 PST, Steve VanDeBogart	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Steve VanDeBogart

Comment 1 2009-12-30 11:22:53 PST

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

WebKit Review Bot

Comment 2 2009-12-30 11:27:32 PST

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

Eric Seidel (no email)

Comment 3 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?

Eric Seidel (no email)

Comment 4 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.

Steve VanDeBogart

Comment 5 2009-12-30 17:22:59 PST

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

WebKit Review Bot

Comment 6 2009-12-30 17:26:29 PST

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

Evan Martin

Comment 7 2009-12-30 17:29:43 PST

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

Steve VanDeBogart

Comment 8 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.

Steve VanDeBogart

Comment 9 2009-12-30 17:41:14 PST

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

WebKit Review Bot

Comment 10 2009-12-30 17:42:01 PST

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

Adam Langley

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

Adam Barth

Comment 12 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.

Steve VanDeBogart

Comment 13 2010-01-06 22:44:07 PST

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

WebKit Review Bot

Comment 14 2010-01-06 22:46:40 PST

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

Steve VanDeBogart

Comment 15 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)

Eric Seidel (no email)

Comment 16 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.

WebKit Commit Bot

Comment 17 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>

WebKit Commit Bot

Comment 18 2010-01-22 21:11:38 PST

All reviewed patches have been landed. Closing bug.

Eric Seidel (no email)

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

Eric Seidel (no email)

Comment 20 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.

Eric Seidel (no email)

Comment 21 2010-01-24 14:30:49 PST

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

Eric Seidel (no email)

Comment 22 2010-01-24 14:32:05 PST

Skipped the test on Gtk for now.

Tim Horton

Comment 23 2012-03-16 14:50:17 PDT

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

Note You need to log in before you can comment on or make changes to this bug.