| 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: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Linux | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Steve VanDeBogart
2009-12-30 11:19:25 PST
Created attachment 45674 [details]
Check click point against content area.
style-queue ran check-webkit-style on attachment 45674 [details] without any errors.
Comment on attachment 45674 [details]
Check click point against content area.
This should be easy to test via a LayoutTest, no?
Comment on attachment 45674 [details]
Check click point against content area.
Needs a LayoutTest or explanation of why one is impossible.
Created attachment 45689 [details]
Add a layout test for middle clicking scrollbars.
style-queue ran check-webkit-style on attachment 45689 [details] without any errors.
+julie, who has an opinion on setTimeout in layout tests 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. Created attachment 45690 [details]
remove setTimeout from new layout test
style-queue ran check-webkit-style on attachment 45690 [details] without any errors.
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 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. Created attachment 46023 [details]
A better test for the main scrollbars
style-queue ran check-webkit-style on attachment 46023 [details] without any errors.
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 on attachment 45690 [details]
remove setTimeout from new layout test
Clearing Adam Barth's r+ from this obsolete patch.
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> All reviewed patches have been landed. Closing bug. 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. :( The test appears to outright fail on Gtk. Either we can skip it on Gtk or roll this out. Committed r53784: <http://trac.webkit.org/changeset/53784> Skipped the test on Gtk for now. Also skipping on mac: https://bugs.webkit.org/show_bug.cgi?id=81410 |