Bug 30048 – [Qt] mouseUp() and mouseDown() in EventSender ignore the button argument

• Component: Platform
• Status: RESOLVED FIXED

[Jakub Wieczorek]

The argument indicates which mouse button should the events concern and is used in some layout tests.

[Jakub Wieczorek]

Created attachment 40581 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 40581 [details]
patch

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.

[Jakub Wieczorek]

Created attachment 40645 [details]
patch

(In reply to comment #2)
> (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.

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.

[Jakub Wieczorek]

Created attachment 40647 [details]
patch

Added the affected layout test to the ChangeLog.

[Simon Hausmann]

Comment on attachment 40647 [details]
patch

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

[Jakub Wieczorek]

Comment on attachment 40647 [details]
patch

(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?

[WebKit Commit Bot]

Comment on attachment 40647 [details]
patch

Clearing flags on attachment: 40647

Committed r49167: <http://trac.webkit.org/changeset/49167>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.