Bug 78444 - [Qt] Middle clicking a scrollbar causes text to be pasted.
Summary: [Qt] Middle clicking a scrollbar causes text to be pasted.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-12 16:43 PST by Wyatt Gosling
Modified: 2012-07-11 08:05 PDT (History)
5 users (show)

See Also:


Attachments
Patch with fix and improved layout test (2.96 KB, patch)
2012-05-22 08:25 PDT, Steffen Imhof
hausmann: review-
Details | Formatted Diff | Diff
Patch with more elaborate LayoutTests ChangeLog (3.28 KB, patch)
2012-05-24 02:26 PDT, Steffen Imhof
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wyatt Gosling 2012-02-12 16:43:52 PST
QtWebKit version 2.2.1

Whenever I have a text field selected and middle click on the scroll bar to scroll to a particular part of the page, the text from my clipboard is pasted to the text field.

Reproducible: Always
Reproducible with: rekonq, arora, qupzilla

Steps to Reproduce:
1. Select and copy the title of this bug report
2. Resize browser so the scroll bar is visible
3. Click on the comment text area in this bug report
4. Middle click on the scroll bar
3. Middle click on the scroll bar

Actual Results:  
The scroll bar jumps to the clicked location, then jumps back and the copied text is in the text field.

Expected Results:  
The scroll bar jumps to the clicked location, stays there, and no text is pasted
Comment 1 Steffen Imhof 2012-05-22 08:25:53 PDT
Created attachment 143305 [details]
Patch with fix and improved layout test

I had a look into this and think that the clipboard handling should simply not be executed when the event was already handled. When checking how to make a layout test for this, I stumbled upon an existing one. But this was a PASS all the time (at least for the Qt version). So I changed the test to be a bit more like the use case described in this bug (using an <input> field). Now it correctly switches from FAIL to PASS when the change in qwebpage.cpp is applied.
Comment 2 Simon Hausmann 2012-05-23 13:09:23 PDT
Comment on attachment 143305 [details]
Patch with fix and improved layout test

The patch (again) looks correct to me, but I would really like to see a slightly more elaborate explanation in the ChangeLog why the change in the layout test makes it more sensitive to triggering this bug. Since this is a cross-port/platform layout test it may not be obvious, especially not when looking at the ChangeLog in three months and all we see is "made it more sensitive" :)
Comment 3 Steffen Imhof 2012-05-24 02:26:54 PDT
Created attachment 143770 [details]
Patch with more elaborate LayoutTests ChangeLog

Ok, a reasonable request :-)

I tried to explain the changed test a bit more in the ChangeLog.
What I manually tried, besides the Qt case, was a recent Chromium build. For Chromium the original test seemed to work, but the new one works, too.

I am not sure if the not-triggering of onpaste on the non-editable <body> is another bug in Qt? I did not find something like a specification for onpaste... as far as I could see it was originally introduced in Internet Explorer, so it's probably not well specified. FWIW Firefox did not trigger onpaste for <body> either.

Just for my education: All green EWS bots should indicate that the changed test does not break any of the tested platforms now, right?
Comment 4 Simon Hausmann 2012-05-25 07:36:16 PDT
(In reply to comment #3)
> Just for my education: All green EWS bots should indicate that the changed test does not break any of the tested platforms now, right?

It merely indicates that the patch _builds_ on those platforms. Only the chromium EWS is fast enough currently to also _run_ the tests (unless that test is skipped). So this should probably be landed manually with care.
Comment 5 Steffen Imhof 2012-05-29 02:05:56 PDT
(In reply to comment #4)
> It merely indicates that the patch _builds_ on those platforms. Only the chromium EWS is fast enough currently to also _run_ the tests (unless that test is skipped). So this should probably be landed manually with care.

Ok, thanks for the info. So how will this proceed? Can you do the manual landing or shall I just ask "someone" on IRC?
Comment 6 Bruno Abinader (history only) 2012-07-11 05:33:29 PDT
Comment on attachment 143770 [details]
Patch with more elaborate LayoutTests ChangeLog

Asking "commit-queue" flag.
Comment 7 WebKit Review Bot 2012-07-11 08:05:40 PDT
Comment on attachment 143770 [details]
Patch with more elaborate LayoutTests ChangeLog

Clearing flags on attachment: 143770

Committed r122333: <http://trac.webkit.org/changeset/122333>
Comment 8 WebKit Review Bot 2012-07-11 08:05:44 PDT
All reviewed patches have been landed.  Closing bug.