FNX-14556 ⁃ [Bug] pinch to zoom doesn't work on https://www.lightpollutionmap.info/ with simultaneous touch due to pointer events
Categories
(Core :: DOM: Events, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox82 | --- | unaffected |
firefox83 | --- | fixed |
People
(Reporter: amoya, Assigned: kats)
References
(Regression, )
Details
(Keywords: regression, Whiteboard: [leaflet.js fixed by bug 1648491])
Attachments
(1 file)
From github: https://github.com/mozilla-mobile/fenix/issues/13376.
Steps to reproduce
So this is an issue which has been reported in two cases with different sites using maps, but they don't seem to be using the same library.
Exhibit A
https://github.com/webcompat/web-bugs/issues/56090
- Go to https://leafletjs.com/
- Pinch to zoom
Exhibit B
https://github.com/webcompat/web-bugs/issues/54615
- Go to https://www.lightpollutionmap.info/
- Pinch to zoom
In both cases, this is working only if you put one finger, wait, and put a second finger, then making the gesture.
Device information
- Android device: 10, Pixel 2
- Fenix version: Firefox Preview 81
Assignee | ||
Comment 1•4 years ago
|
||
So far I'm not able to reproduce the problem.
Comment 2•4 years ago
|
||
The lightpollutionmap.info seems quite busted in Fenix Nightly. Sometimes zooming will zoom all the elements of the page. Pinch zooming out will nearly always zoom in on the page. Tested using a Pixel 2XL and a Moto G5.
Assignee | ||
Comment 3•4 years ago
|
||
Hm. Now I can repro the lightpollutionmap problem. Seems kind of like putting down two fingers together triggers a double-tap zoom event.
For leaflet.js, I can sort of repro on release but not Nightly. It may have been fixed by bug 1648491.
Assignee | ||
Comment 4•4 years ago
|
||
Can you confirm that the leaflet.js site works for you on Nightly? Also, if you set dom.w3c_pointer_events.multiprocess.android.enabled
and dom.w3c_pointer_events.enabled
to false on Fenix (browser restart needed to be safe), do you still see the issues on lightpollutionmap.info? I don't, which makes me suspect it's a pointer events bug. The site has some slightly-obfuscated JS but it looks like it does some gesture detection with pointer events, so maybe we're sending bad info on the pointer events.
Comment 5•4 years ago
|
||
Leaflet is working mostly perfectly on nightly. Might have had a missed touch once or twice over a few minutes of interacting with the map box. Though it might have been a pinch outside the box border. Zooming in and out, double tap to zoom in, the buttons and using a pointing stick (s-pen) all functioned as expected. Used a Galaxy Note 9.
Yes, with touch events disabled the lightpollutionmap.info page behaves as I would expect. With a few minutes of usage I do not see any of the bad behavior I was seeing with pointer events enabled.
Assignee | ||
Comment 6•4 years ago
|
||
Great, thanks for confirming! I'm going to modify this bug to track just the lightpollutioninfo.map page, and move it to DOM events.
Comment 7•4 years ago
|
||
Changed the default type to 'defect' for the component.
Assignee | ||
Comment 8•4 years ago
|
||
I assume you meant to post that to bug 1666562?
Comment 9•4 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
I assume you meant to post that to bug 1666562?
Oops. yeah. sorry bout that :)
Comment 10•4 years ago
|
||
This is also reproducible in Windows build and is regressed by:
Updated•4 years ago
|
Comment 11•4 years ago
|
||
Turning off apz.allow_zooming
fixes the issue on both Android and Windows. :kats, mind taking a look?
Updated•4 years ago
|
Updated•4 years ago
|
Comment 12•4 years ago
|
||
The site uses the OpenLayers library. We have an existing issue on file related to pointer events involving the OpenLayers library: bug 1607463. This may be another manifestation of the same isssue.
Comment 13•4 years ago
|
||
allow_zooming does not affect the mentioned DuckDuckgo map and Bing Maps, so I think they are different.
Comment 14•4 years ago
|
||
The issue is related to us getting into this code block (and therefore sending a pointercancel
event) when we shouldn't (or at least, when the site / library is not expecting us to).
Comment 15•4 years ago
•
|
||
Unlike the diagnosis in bug 1607463 comment 3, here the site does not appear to be preventDefault()
ing the pointerdown
event in the first place, suggesting a possible site issue.
Comment 16•4 years ago
|
||
Possibly the sites in question have since upgraded to a version of OpenLayers which includes this PR, titled "Do not preventDefault on pointerdown".
Assignee | ||
Comment 17•4 years ago
|
||
Calling preventDefault()
on pointerdown should not be relevant here; that should only prevent some mouse compat events from being dispatched.
That being said, if we are triggering a pointercancel via the block of code you linked in comment 14, then it's because APZ determines it can consume the events with some default action. This makes sense when apz.allow_zooming=true because the pinch action is permitted. If the page wants to override that behaviour it should really be setting a touch-action
property that disallows pinching. So touch-action:none
or maybe touch-action: pan-x pan-y
.
I'm not entirely sure why it works in Chrome though.
Assignee | ||
Comment 18•4 years ago
|
||
I filed https://github.com/openlayers/openlayers/issues/11614 to hopefully arrive at a solution.
Assignee | ||
Comment 19•4 years ago
|
||
Looking at our code a bit more it's possible that we're erroneously entering that pointercancel block too early. Normally we wait for both the touchstart and (if the touchstart wasn't preventDefaulted) the first touchmove before notifying APZ of the content response. However what I'm seeing here is that we send the pointercancel right after the second touchstart, instead of waiting for the touchmove on that block. This might be because sentContentResponse
gets set to true here but the condition at this line is really wanting to know if the current event triggered a content response. In this case the sentContentResponse
was set to true for the previous event and so that might be a bit of a mismatch.
Try push with that changed to see if it breaks anything and/or fixes the bug: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=d3cf499902514bcfafc10ac71ad6f58dccd26b6b
Assignee | ||
Comment 21•4 years ago
•
|
||
Patch seems to work, both on Windows and in GVE. Also doesn't seem to fail any existing tests, so it should be good.
Assignee | ||
Comment 22•4 years ago
|
||
Assignee | ||
Comment 23•4 years ago
|
||
The existing code would send the pointercancel to web content as soon as the
second touchstart event was triggered for a pinch gesture. Instead, we want
to wait and see if the subsequent touchmove is cancelled before doing that.
Comment 24•4 years ago
|
||
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7006f2f19b52 For two-touch gestures, don't send the pointercancel too early. r=botond
Comment 25•4 years ago
|
||
bugherder |
Comment 26•4 years ago
|
||
Verified as fixed on Firefox Preview Nightly 201025 (Build #2015771787) using a Pixel 3 XL (Android 9) and Samsung Galaxy S9 (Android 8.0.0).
Updated•4 years ago
|
Description
•