80542 2012-03-07 14:26:58 -0800 [Qt WK2] Disable/enable mouse events when displaying dialogs only for desktop view 2012-03-26 08:50:31 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 dinu.jacob webkit-unassigned abecsi hausmann menard webkit.review.bot zoltan oldest_to_newest 573240 0 dinu.jacob 2012-03-07 14:26:58 -0800 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. 573242 1 dinu.jacob 2012-03-07 14:28:43 -0800 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. 573271 2 130701 dinu.jacob 2012-03-07 14:57:21 -0800 Created attachment 130701 Patch 573811 3 hausmann 2012-03-08 07:07:04 -0800 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. 573983 4 dinu.jacob 2012-03-08 11:16:21 -0800 (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?) 574698 5 abecsi 2012-03-09 00:29:05 -0800 (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). 574845 6 dinu.jacob 2012-03-09 05:51:06 -0800 (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. 574848 7 dinu.jacob 2012-03-09 05:59:58 -0800 As flickable webView wouldn't receive the mouse events is there any benefit of enabling mouse events for it? 575509 8 abecsi 2012-03-10 05:58:18 -0800 (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. 583057 9 dinu.jacob 2012-03-20 07:37:12 -0700 (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? 587789 10 130701 webkit.review.bot 2012-03-26 08:50:25 -0700 Comment on attachment 130701 Patch Clearing flags on attachment: 130701 Committed r112105: <http://trac.webkit.org/changeset/112105> 587790 11 webkit.review.bot 2012-03-26 08:50:31 -0700 All reviewed patches have been landed. Closing bug. 130701 2012-03-07 14:57:21 -0800 2012-03-26 08:50:25 -0700 Patch 80542.patch text/plain 3420 dinu.jacob ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAzZjFjZDRjLi45NjhlY2U2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjAg QEAKIDIwMTItMDMtMDcgIERpbnUgSmFjb2IgIDxkaW51LmphY29iQG5va2lhLmNvbT4KIAorICAg ICAgICBbUXQgV0syXSBEaXNhYmxlL2VuYWJsZSBtb3VzZSBldmVudHMgd2hlbiBkaXNwbGF5aW5n IGRpYWxvZ3Mgb25seSBmb3IgZGVza3RvcCB2aWV3CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD04MDU0MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorICAgICAgIAorICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvcXQvcXF1aWNrd2Vi dmlldy5jcHA6CisgICAgICAgIChRUXVpY2tXZWJWaWV3TGVnYWN5UHJpdmF0ZTo6ZW5hYmxlTW91 c2VFdmVudHMpOgorICAgICAgICAoUVF1aWNrV2ViVmlld0xlZ2FjeVByaXZhdGU6OmRpc2FibGVN b3VzZUV2ZW50cyk6CisgICAgICAgICogVUlQcm9jZXNzL0FQSS9xdC9xcXVpY2t3ZWJ2aWV3X3Bf cC5oOgorICAgICAgICAoUVF1aWNrV2ViVmlld1ByaXZhdGU6OmVuYWJsZU1vdXNlRXZlbnRzKToK KyAgICAgICAgKFFRdWlja1dlYlZpZXdQcml2YXRlOjpkaXNhYmxlTW91c2VFdmVudHMpOgorICAg ICAgICAoUVF1aWNrV2ViVmlld0xlZ2FjeVByaXZhdGUpOgorCisyMDEyLTAzLTA3ICBEaW51IEph Y29iICA8ZGludS5qYWNvYkBub2tpYS5jb20+CisKICAgICAgICAgW1F0XSBBdXRoZW50aWNhdGlv biBkaWFsb2cgZG9lcyBub3Qgd29yawogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9Nzk3MzgKIApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL0FQSS9xdC9xcXVpY2t3ZWJ2aWV3LmNwcCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9B UEkvcXQvcXF1aWNrd2Vidmlldy5jcHAKaW5kZXggOWIzYThiOC4uZWIxMjFlMiAxMDA2NDQKLS0t IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9xdC9xcXVpY2t3ZWJ2aWV3LmNwcAorKysg Yi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXcuY3BwCkBAIC0x MjYsMjAgKzEyNiw2IEBAIHZvaWQgUVF1aWNrV2ViVmlld1ByaXZhdGU6OmluaXRpYWxpemUoV0tD b250ZXh0UmVmIGNvbnRleHRSZWYsIFdLUGFnZUdyb3VwUmVmIHBhCiAgICAgd2ViUGFnZVByb3h5 LT5pbml0aWFsaXplV2ViUGFnZSgpOwogfQogCi12b2lkIFFRdWlja1dlYlZpZXdQcml2YXRlOjpl bmFibGVNb3VzZUV2ZW50cygpCi17Ci0gICAgUV9RKFFRdWlja1dlYlZpZXcpOwotICAgIHEtPnNl dEFjY2VwdGVkTW91c2VCdXR0b25zKFF0OjpNb3VzZUJ1dHRvbk1hc2spOwotICAgIHEtPnNldEFj Y2VwdEhvdmVyRXZlbnRzKHRydWUpOwotfQotCi12b2lkIFFRdWlja1dlYlZpZXdQcml2YXRlOjpk aXNhYmxlTW91c2VFdmVudHMoKQotewotICAgIFFfUShRUXVpY2tXZWJWaWV3KTsKLSAgICBxLT5z ZXRBY2NlcHRlZE1vdXNlQnV0dG9ucyhRdDo6Tm9CdXR0b24pOwotICAgIHEtPnNldEFjY2VwdEhv dmVyRXZlbnRzKGZhbHNlKTsKLX0KLQogUVBvaW50RiBRUXVpY2tXZWJWaWV3UHJpdmF0ZTo6cGFn ZUl0ZW1Qb3MoKQogewogICAgIEFTU0VSVChwYWdlVmlldyk7CkBAIC01MjUsNiArNTExLDIwIEBA IHZvaWQgUVF1aWNrV2ViVmlld0xlZ2FjeVByaXZhdGU6OnVwZGF0ZVZpZXdwb3J0U2l6ZSgpCiAg ICAgd2ViUGFnZVByb3h5LT5kcmF3aW5nQXJlYSgpLT5zZXRWaXNpYmxlQ29udGVudHNSZWN0Rm9y U2NhbGluZyhJbnRSZWN0KEludFBvaW50KCksIHZpZXdwb3J0U2l6ZSksIDEpOwogfQogCit2b2lk IFFRdWlja1dlYlZpZXdMZWdhY3lQcml2YXRlOjplbmFibGVNb3VzZUV2ZW50cygpCit7CisgICAg UV9RKFFRdWlja1dlYlZpZXcpOworICAgIHEtPnNldEFjY2VwdGVkTW91c2VCdXR0b25zKFF0OjpN b3VzZUJ1dHRvbk1hc2spOworICAgIHEtPnNldEFjY2VwdEhvdmVyRXZlbnRzKHRydWUpOworfQor Cit2b2lkIFFRdWlja1dlYlZpZXdMZWdhY3lQcml2YXRlOjpkaXNhYmxlTW91c2VFdmVudHMoKQor eworICAgIFFfUShRUXVpY2tXZWJWaWV3KTsKKyAgICBxLT5zZXRBY2NlcHRlZE1vdXNlQnV0dG9u cyhRdDo6Tm9CdXR0b24pOworICAgIHEtPnNldEFjY2VwdEhvdmVyRXZlbnRzKGZhbHNlKTsKK30K KwogUVF1aWNrV2ViVmlld0ZsaWNrYWJsZVByaXZhdGU6OlFRdWlja1dlYlZpZXdGbGlja2FibGVQ cml2YXRlKFFRdWlja1dlYlZpZXcqIHZpZXdwb3J0KQogICAgIDogUVF1aWNrV2ViVmlld1ByaXZh dGUodmlld3BvcnQpCiAgICAgLCBwb3N0VHJhbnNpdGlvblN0YXRlKGFkb3B0UHRyKG5ldyBQb3N0 VHJhbnNpdGlvblN0YXRlKHRoaXMpKSkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJv Y2Vzcy9BUEkvcXQvcXF1aWNrd2Vidmlld19wX3AuaCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9BUEkvcXQvcXF1aWNrd2Vidmlld19wX3AuaAppbmRleCBiMTQxOWM0Li41ZDM0NjBhIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXdfcF9w LmgKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9xdC9xcXVpY2t3ZWJ2aWV3X3Bf cC5oCkBAIC02Niw4ICs2Niw4IEBAIHB1YmxpYzoKIAogICAgIHZpcnR1YWwgdm9pZCBpbml0aWFs aXplKFdLQ29udGV4dFJlZiBjb250ZXh0UmVmID0gMCwgV0tQYWdlR3JvdXBSZWYgcGFnZUdyb3Vw UmVmID0gMCk7CiAKLSAgICB2b2lkIGVuYWJsZU1vdXNlRXZlbnRzKCk7Ci0gICAgdm9pZCBkaXNh YmxlTW91c2VFdmVudHMoKTsKKyAgICB2aXJ0dWFsIHZvaWQgZW5hYmxlTW91c2VFdmVudHMoKSB7 IH0KKyAgICB2aXJ0dWFsIHZvaWQgZGlzYWJsZU1vdXNlRXZlbnRzKCkgeyB9CiAKICAgICB2aXJ0 dWFsIFFQb2ludEYgcGFnZUl0ZW1Qb3MoKTsKICAgICB2aXJ0dWFsIHZvaWQgdXBkYXRlQ29udGVu dHNTaXplKGNvbnN0IFFTaXplRiYpIHsgfQpAQCAtMTc1LDYgKzE3NSw4IEBAIHB1YmxpYzoKICAg ICB2aXJ0dWFsIHZvaWQgaW5pdGlhbGl6ZShXS0NvbnRleHRSZWYgY29udGV4dFJlZiA9IDAsIFdL UGFnZUdyb3VwUmVmIHBhZ2VHcm91cFJlZiA9IDApOwogCiAgICAgdmlydHVhbCB2b2lkIHVwZGF0 ZVZpZXdwb3J0U2l6ZSgpOworICAgIHZpcnR1YWwgdm9pZCBlbmFibGVNb3VzZUV2ZW50cygpOwor ICAgIHZpcnR1YWwgdm9pZCBkaXNhYmxlTW91c2VFdmVudHMoKTsKIH07CiAKIGNsYXNzIFFRdWlj a1dlYlZpZXdGbGlja2FibGVQcml2YXRlIDogcHVibGljIFFRdWlja1dlYlZpZXdQcml2YXRlIHsK