Bug 80542 – [Qt WK2] Disable/enable mouse events when displaying dialogs only for desktop view

• Component: WebKit2
• Status: RESOLVED FIXED

[Dinu Jacob]

Mouse events are disabled when displaying dialogs and then enabled on dismissing them for both touch view and desktop views. This causes the mouse events to be enabled for touch views after dismissing any dialog.

[Dinu Jacob]

Mouse events are enabled only for desktop view on creation. It seems wrong that they get enabled for touch view as well after a dialog is dismissed.

[Dinu Jacob]

Created attachment 130701 [details]
Patch

[Simon Hausmann]

I wonder if they should be enabled all the way. This distinction between touch and desktop view makes less sense when you think of desktop computers with real touch screens and a mouse attached, or laptops like the dell latitude with touch screen and touch pad.

[Dinu Jacob]

(In reply to comment #3)
> I wonder if they should be enabled all the way. This distinction between touch and desktop view makes less sense when you think of desktop computers with real touch screens and a mouse attached, or laptops like the dell latitude with touch screen and touch pad.

A couple of questions:

- It seems like WebView will not receive the mouse events as flickable ( a child of webview )  always accepts the events.  Which means enabling/disabling the flag for WebView has no impact (though it would still be wrong to enable it after a dialog is dismissed). Is this right that flickable always accepts the mpuse events?

- If the mouse events are received by WebView and are sent down to WebPage, for such a device, wouldn’t it result in two sets of mouse events for a tap gesture as one set of fake mouse events are also generated for a tap gesture?
On a side note, on such devices, will Qt  still send mouse events for the touch events as well? (So, will the flickable for example receive two sets of mouse events in this case?)

[Andras Becsi]

(In reply to comment #4)
> (In reply to comment #3)
> > I wonder if they should be enabled all the way. This distinction between touch and desktop view makes less sense when you think of desktop computers with real touch screens and a mouse attached, or laptops like the dell latitude with touch screen and touch pad.
> 
> A couple of questions:
> 
> - It seems like WebView will not receive the mouse events as flickable ( a child of webview )  always accepts the events.  Which means enabling/disabling the flag for WebView has no impact (though it would still be wrong to enable it after a dialog is dismissed). Is this right that flickable always accepts the mpuse events?

The Flickable reacting on mouse events was the reason why we needed to swallow real mouse events in MiniBrowser, and synthesize mouse events out of touch events in QtFlickProvider.

This should only be an issue when testing the flickable web view on desktop, so when setting the flag in the WebView we could also set flickable.setInteractive(false), which should disable the Flickable.

> 
> - If the mouse events are received by WebView and are sent down to WebPage, for such a device, wouldn’t it result in two sets of mouse events for a tap gesture as one set of fake mouse events are also generated for a tap gesture?
> On a side note, on such devices, will Qt  still send mouse events for the touch events as well? (So, will the flickable for example receive two sets of mouse events in this case?)

In legacy mode (desktop / mouse mode) we do not have a Flickable at all, Qt only sends native mouse events and we should not synthesize touch events, hence there is no tap gesture in that case.
Note that this behaviour was recently fixed for MiniBrowser --desktop, since the mocking was not disabled by default.

The problems only arise for the mobile / flickable WebView, both when testing it on desktop (we have to do touch mocking for the WebView), and on the device (we let Qt synthesize mouse events for the URL bar).

[Dinu Jacob]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > I wonder if they should be enabled all the way. This distinction between touch and desktop view makes less sense when you think of desktop computers with real touch screens and a mouse attached, or laptops like the dell latitude with touch screen and touch pad.
> > 
> > A couple of questions:
> > 
> > - It seems like WebView will not receive the mouse events as flickable ( a child of webview )  always accepts the events.  Which means enabling/disabling the flag for WebView has no impact (though it would still be wrong to enable it after a dialog is dismissed). Is this right that flickable always accepts the mpuse events?
> 
> The Flickable reacting on mouse events was the reason why we needed to swallow real mouse events in MiniBrowser, and synthesize mouse events out of touch events in QtFlickProvider.
> 

On a touch device with mouse, even though the mouse events are swallowed by MiniBrowser, wouldn't it still result in two sets of touch events (the synthesized touch events and the actual touch events) that would potentially mess up the gesture recognition? (And for a thirdparty client using the webView who doesn't filter the mouse events, flickable will end up receiving the native mouse events as well the mouse events synthesized by Qt).

> This should only be an issue when testing the flickable web view on desktop, so when setting the flag in the WebView we could also set flickable.setInteractive(false), which should disable the Flickable.
> 
I didn't quite follow this...
> > 
> > - If the mouse events are received by WebView and are sent down to WebPage, for such a device, wouldn’t it result in two sets of mouse events for a tap gesture as one set of fake mouse events are also generated for a tap gesture?
> > On a side note, on such devices, will Qt  still send mouse events for the touch events as well? (So, will the flickable for example receive two sets of mouse events in this case?)
> 
> In legacy mode (desktop / mouse mode) we do not have a Flickable at all, Qt only sends native mouse events and we should not synthesize touch events, hence there is no tap gesture in that case.
> Note that this behaviour was recently fixed for MiniBrowser --desktop, since the mocking was not disabled by default.
> 
> The problems only arise for the mobile / flickable WebView, both when testing it on desktop (we have to do touch mocking for the WebView), and on the device (we let Qt synthesize mouse events for the URL bar).

Yes, it was the webView I was thinking of when mentioning the tapGesture.

[Dinu Jacob]

As flickable webView wouldn't receive the mouse events is there any benefit of enabling mouse events for it?

[Andras Becsi]

(In reply to comment #7)
> As flickable webView wouldn't receive the mouse events is there any benefit of enabling mouse events for it?

Sorry, I was somewhat superficial when reading your questions and did not look at the patch.

If the Flickable is in use the mouse handling on the WebView has to be disabled since they will be delivered to the Flickable first, which is undesirable.
This is another reason why we need to subclass Flickable for the WebView item instead of internally instantiating it.

Qt only synthesizes mouse events for touch events which were not accepted, so if the touch event sent by the touch device would be accepted we would not receive a mouse event from Qt.

MiniBrowser and the WebView is indeed not yet prepared for the mentioned hybrid devices and fixing this is clearly not the scope of this bug.

For the currently supported configurations we should keep the distinction between the touch and desktop view, hence I think this patch is correct and fixes inconsistent behavior of the web view.

[Dinu Jacob]

(In reply to comment #8)
> (In reply to comment #7)
> > As flickable webView wouldn't receive the mouse events is there any benefit of enabling mouse events for it?
> 
> Sorry, I was somewhat superficial when reading your questions and did not look at the patch.
> 
> If the Flickable is in use the mouse handling on the WebView has to be disabled since they will be delivered to the Flickable first, which is undesirable.
> This is another reason why we need to subclass Flickable for the WebView item instead of internally instantiating it.
> 
> Qt only synthesizes mouse events for touch events which were not accepted, so if the touch event sent by the touch device would be accepted we would not receive a mouse event from Qt.
> 
> MiniBrowser and the WebView is indeed not yet prepared for the mentioned hybrid devices and fixing this is clearly not the scope of this bug.
> 
> For the currently supported configurations we should keep the distinction between the touch and desktop view, hence I think this patch is correct and fixes inconsistent behavior of the web view.

Simon, can you take a look at this again?

[WebKit Review Bot]

Comment on attachment 130701 [details]
Patch

Clearing flags on attachment: 130701

Committed r112105: <http://trac.webkit.org/changeset/112105>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.