Bug 30048 - [Qt] mouseUp() and mouseDown() in EventSender ignore the button argument
: [Qt] mouseUp() and mouseDown() in EventSender ignore the button argument
Status: RESOLVED FIXED
: WebKit
Platform
: 528+ (Nightly build)
: Other Mac OS X 10.5
: P2 Normal
Assigned To:
:
: Qt
:
:
  Show dependency treegraph
 
Reported: 2009-10-03 12:30 PST by
Modified: 2009-10-06 04:42 PST (History)


Attachments
patch (3.60 KB, patch)
2009-10-03 12:33 PST, Jakub Wieczorek
eric: review-
Review Patch | Details | Formatted Diff | Diff
patch (4.15 KB, patch)
2009-10-05 11:14 PST, Jakub Wieczorek
no flags Review Patch | Details | Formatted Diff | Diff
patch (4.23 KB, patch)
2009-10-05 11:28 PST, Jakub Wieczorek
no flags Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2009-10-03 12:30:25 PST
The argument indicates which mouse button should the events concern and is used in some layout tests.
------- Comment #1 From 2009-10-03 12:33:13 PST -------
Created an attachment (id=40581) [details]
patch
------- Comment #2 From 2009-10-05 09:22:28 PST -------
(From update of attachment 40581 [details])
What is the NoButton default useful for?  And why don't we assert() that it's not called?  Or are there reasons why mouseDown(3) should silently "fail"?

This should list tests which it fixes (assuming the list is relatively short).  But in general it looks fine.  r- for the possible NoButton confusion.  Please explain why NoButton is correct there.
------- Comment #3 From 2009-10-05 11:14:21 PST -------
Created an attachment (id=40645) [details]
patch

(In reply to comment #2)
> (From update of attachment 40581 [details] [details])
> What is the NoButton default useful for?  And why don't we assert() that it's
> not called?  Or are there reasons why mouseDown(3) should silently "fail"?
> 
> This should list tests which it fixes (assuming the list is relatively short). 
> But in general it looks fine.  r- for the possible NoButton confusion.  Please
> explain why NoButton is correct there.

Right, NoButton does not make much sense. I took a look at the EventSender from the Windows port and changed the patch to do something similar.

So mouseDown(3) will send a middle button event and the fallback button will be the left button.
------- Comment #4 From 2009-10-05 11:28:17 PST -------
Created an attachment (id=40647) [details]
patch

Added the affected layout test to the ChangeLog.
------- Comment #5 From 2009-10-05 13:46:34 PST -------
(From update of attachment 40647 [details])
Thanks! (also for addressing Eric's comments)

Could you make a patch for unskipping the test, too?

Btw, please set cq? if you'd like this patch to go through the queue :)
------- Comment #6 From 2009-10-05 14:07:04 PST -------
(From update of attachment 40647 [details])
(In reply to comment #5)
> Could you make a patch for unskipping the test, too?

I must say I was going to make a collective patch, which would unskip all the tests that are passing after the recent DRT fixes. Not only the tests that I have spotted the issues with but also those that I have  not gone through yet, and which make use of the added/fixed functions and as a result may be fixed now too. Is it fine or should I make individual patches to the Skipped list corresponding to each DRT fix?
------- Comment #7 From 2009-10-06 04:42:36 PST -------
(From update of attachment 40647 [details])
Clearing flags on attachment: 40647

Committed r49167: <http://trac.webkit.org/changeset/49167>
------- Comment #8 From 2009-10-06 04:42:39 PST -------
All reviewed patches have been landed.  Closing bug.