Bug 61322 - [Qt] Selectstart event tests added by r87096 are failing on Qt
Summary: [Qt] Selectstart event tests added by r87096 are failing on Qt
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Nobody
URL:
Keywords: LayoutTestFailure, Qt, QtTriaged
Depends on: 19489 61393
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-23 16:37 PDT by Ryosuke Niwa
Modified: 2014-02-03 03:17 PST (History)
8 users (show)

See Also:


Attachments
Patch (5.39 KB, patch)
2011-05-24 13:57 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-05-23 16:37:47 PDT
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
Comment 1 Robert Hogan 2011-05-24 13:57:15 PDT
Created attachment 94682 [details]
Patch
Comment 2 Robert Hogan 2011-05-24 13:59:43 PDT
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.
Comment 3 Ryosuke Niwa 2011-05-24 14:03:24 PDT
(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?
Comment 4 Ryosuke Niwa 2011-05-24 15:25:43 PDT
Mn... after looking at the failures on WK2, I concur with your point that Qt doesn't implement dragMode.
Comment 5 Chang Shu 2011-05-25 12:31:39 PDT
(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
Comment 6 Ryosuke Niwa 2011-05-25 12:32:57 PDT
(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?
Comment 7 Chang Shu 2011-05-25 12:40:28 PDT
(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.
Comment 8 Ryosuke Niwa 2011-05-25 12:49:35 PDT
It was landed on the bug 61393.
Comment 9 Robert Hogan 2011-05-25 15:09:46 PDT
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?
Comment 10 Yael 2011-05-27 16:56:09 PDT
(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 ?
.
Comment 11 Ryosuke Niwa 2011-08-21 00:28:35 PDT
(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.
Comment 12 Yael 2011-08-21 05:21:20 PDT
(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.
Comment 13 Jocelyn Turcotte 2014-02-03 03:17:50 PST
=== 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.