| Summary: | [Qt WK2] Disable/enable mouse events when displaying dialogs only for desktop view | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dinu Jacob <dinu.jacob> | ||||
| Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abecsi, hausmann, menard, webkit.review.bot, zoltan | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Dinu Jacob
2012-03-07 14:26:58 PST
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. Created attachment 130701 [details]
Patch
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. (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?) (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). (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. As flickable webView wouldn't receive the mouse events is there any benefit of enabling mouse events for it? (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. (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? Comment on attachment 130701 [details] Patch Clearing flags on attachment: 130701 Committed r112105: <http://trac.webkit.org/changeset/112105> All reviewed patches have been landed. Closing bug. |