Bug 61322 – [Qt] Selectstart event tests added by r87096 are failing on Qt

• Component: UI Events
• Status: RESOLVED INVALID

[Ryosuke Niwa]

The following tests are all failing on Qt:
fast/events/selectstart-by-double-triple-clicks.html
fast/events/selectstart-by-drag.html
fast/events/selectstart-by-single-click-with-shift.html

http://build.webkit.org/results/Qt%20Linux%20Release/r87096%20(33230)/results.html

[Robert Hogan]

Created attachment 94682 [details]
Patch

[Robert Hogan]

This fixes one of the tests. The others are due to EventSenderQt's leapForward(), which I don't fully understand yet. It at least looks like we don't have dragMode there, and are just delaying the firing of events instead of queuing them. 

This is partially borne out by the fact that you can get the other two tests to pass by commenting out some of the leapForward() calls.

[Ryosuke Niwa]

(In reply to comment #2)
> This fixes one of the tests. The others are due to EventSenderQt's leapForward(), which I don't fully understand yet. It at least looks like we don't have dragMode there, and are just delaying the firing of events instead of queuing them. 

Maybe dragMode is initialized to a different value on Qt?

[Ryosuke Niwa]

Mn... after looking at the failures on WK2, I concur with your point that Qt doesn't implement dragMode.

[Chang Shu]

(In reply to comment #4)
> Mn... after looking at the failures on WK2, I concur with your point that Qt doesn't implement dragMode.

You mean WK1, right? There's known issue of Drag&Drop on Qt DRT.
https://bugs.webkit.org/show_bug.cgi?id=31332

[Ryosuke Niwa]

(In reply to comment #5)
> (In reply to comment #4)
> > Mn... after looking at the failures on WK2, I concur with your point that Qt doesn't implement dragMode.
> 
> You mean WK1, right? There's known issue of Drag&Drop on Qt DRT.
> https://bugs.webkit.org/show_bug.cgi?id=31332

Yes.  Okay.  Should we merge this bug into that one?

[Chang Shu]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Mn... after looking at the failures on WK2, I concur with your point that Qt doesn't implement dragMode.
> > 
> > You mean WK1, right? There's known issue of Drag&Drop on Qt DRT.
> > https://bugs.webkit.org/show_bug.cgi?id=31332
> 
> Yes.  Okay.  Should we merge this bug into that one?

What happened to Robert's patch? I thought he at least fixed one test. After this, if the remaining issues are all related to drag, I agree we can merge the bugs.

[Ryosuke Niwa]

It was landed on the bug 61393.

[Robert Hogan]

Looking at EventSenderQt I can't really see a good reason for matching other ports' behaviour of introducing a delay in the event queue when leapForward() is called. The purpose of leapForward() (if I understand it correctly) is to introduce a delay long enough to simulate a drag event to the port's toolkit.

This isn't necessary for Qt, which lets the application decide when to create a drag event loop. Removing the delay in leapForward() (and replacing it with m_drag=true) allows the remaining two failing tests to pass and causes only one regression in the layout tests.

This can be fixed in the test itself:

[22:33] <rniwa> editing/selection/select-out-of-editable.html?
[22:33] <mwenge> rniwa:yup
[22:33] <rniwa> mwenge: oh you can remove the second mouseMoveTo, mouseDown, and leapForward now
[22:34] <mwenge> ok, why's that?
[22:34] <rniwa> mwenge: there was a bug in EventHandler
[22:34] <rniwa> mwenge: luckily, evmar found a reduction for the bug and I fixed it a couple of weeks ago :)
[22:34] <rniwa> mwenge: http://trac.webkit.org/changeset/81053
[22:34] <rniwa> mwenge: after this revision, the test shouldn't require additional moveMove

Yael, what are your thoughts on this? OK to remove the delay?

[Yael]

(In reply to comment #9)
> Yael, what are your thoughts on this? OK to remove the delay?
Can you elaborate on the regression? The fact that removing the timeout causes regression sounds to me that we should have a timeout.
I learnt recently that mac and chromium simply modify the time stamp on the event and do not sleep. Is it possible to do that for Qt too ?
.

[Ryosuke Niwa]

(In reply to comment #10)
> I learnt recently that mac and chromium simply modify the time stamp on the event and do not sleep. Is it possible to do that for Qt too ?
> .

The same bug exists on Windows, GTK, and Qt DRT and causing a bunch of tests in editing/selection to permanently fail.

[Yael]

(In reply to comment #11)
> (In reply to comment #10)
> > I learnt recently that mac and chromium simply modify the time stamp on the event and do not sleep. Is it possible to do that for Qt too ?
> > .
> 
> The same bug exists on Windows, GTK, and Qt DRT and causing a bunch of tests in editing/selection to permanently fail.

I added the delay based on the Windows implementation, which now seems to be wrong.
Since QEvent does not have a public API for setting a timestamp, I am not sure what is the alternative.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.