1619187 - Pinch-to-zoom gesture on Google Maps affects the entire page instead of only the map with apz.allow_zooming=true

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[hujq]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

1. Set apz.allow_zooming=true
2. Open https://maps.google.com/ and pinch-to-zoom on the touchpad.

Actual results:

The entire page is zooming, including the map and the search bar and other buttons.

Expected results:

With apz.allow_zooming=false, only the map is zooming. The other elements stay the same.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Botond Ballo [:botond]]

Markus had some ideas about how to handle this.

Comment 3

[Markus Stange [:mstange]]

When apz.allow_zooming is off, nsChildView.mm translates pinch gestures into "fake" wheel events that have the Ctrl key modifier set. Google Maps consumes those wheel events and zooms the map.
When apz.allow_zooming is on, this code does not run and the page is zoomed indiscriminately.

The two ways of responding to pinch gestures are in conflict. We need to find out how to resolve this conflict and check what other browser do to resolve it. The most straightforward resolution I can see is this one:

1. First, always send the fake wheel event.
2. If preventDefault() was called on the event by content JS, stop here and do not zoom.
3. Otherwise, if preventDefault() was not called, proceed to do APZ zooming.

We should check whether this is what Chrome does.
Also, this description is deceivingly simple: The problem with it is that pinch gesture events arrive in the parent process but content JS runs in the content process. So we may need to engage our full APZ delayed-preventDefault-handling machinery for this. Here's a revised description to take this into account:

1. Hit test the event to see whether we are over a dispatch-to-content region due to a wheel event listener.
2. If we are not over a dispatch-to-content region, continue to APZ zoom. End.
3. If we are over a dispatch-to-content region, send a fake wheel event to content, and start queuing up magnification gesture updates while we haven't received a result from content yet.
4. Once the response from content comes in: If preventDefault() has been called, translate all queued up magnification gesture updates into fake wheel events and don't do any APZ zooming. If preventDefault() was not called, start APZ zooming and consume all pending magnification gesture updates into the APZ zoom gesture.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Simple test page with div that consumes wheel events

Indeed, Chrome (on macOS) doesn't do pinch-zooming if the pinch action happens while the cursor is over an element that consumes the wheel event. Safari zooms unconditionally (same as on Google maps). So Markus' suggestion of waiting to see if the wheel event is consumed seems reasonable. I think we mostly have code to do this already, it might just need to be hooked up more properly.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Also confirmed that deleting the wheel event listener off <div id="scene"> in google maps results in pinch-zooming the entire page, not just the map.

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1619187. Handle content prevent defaulting pinch gestures. r?kats

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70d9a096070b
Handle content prevent defaulting pinch gestures. r=kats

Comment 8

[Cosmin Sabou [:CosminS]]

Backed out for causing Gtest failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=os%2Cx%2C10.14%2Cdebug%2Ctest-macosx1014-64%2Fdebug-gtest-1proc%2C%28gtest%29&revision=70d9a096070be5920d3560cfd9cdfd43dca179a3&selectedTaskRun=X_t9U2WMQT-J6CZCG90o-Q-0

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=300449564&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/5daf558349272b05cee2c9097a9499ff4d7363e6

[task 2020-05-02T00:16:19.825Z] 00:16:19     INFO -  TEST-START | APZCBasicTester.Overzoom
[task 2020-05-02T00:16:19.825Z] 00:16:19  WARNING -  TEST-UNEXPECTED-FAIL | APZCBasicTester.Overzoom | Expected equality of these values:
[task 2020-05-02T00:16:19.825Z] 00:16:19     INFO -    expectedStatus
[task 2020-05-02T00:16:19.825Z] 00:16:19     INFO -      Which is: 1
[task 2020-05-02T00:16:19.825Z] 00:16:19     INFO -    statuses[0]
[task 2020-05-02T00:16:19.825Z] 00:16:19     INFO -      Which is: 2 @ /builds/worker/checkouts/gecko/gfx/layers/apz/test/gtest/APZTestCommon.h:892
[task 2020-05-02T00:16:19.825Z] 00:16:19  WARNING -  TEST-UNEXPECTED-FAIL | APZCBasicTester.Overzoom | Expected equality of these values:
[task 2020-05-02T00:16:19.826Z] 00:16:19     INFO -    expectedStatus
[task 2020-05-02T00:16:19.826Z] 00:16:19     INFO -      Which is: 1
[task 2020-05-02T00:16:19.826Z] 00:16:19     INFO -    statuses[1]
[task 2020-05-02T00:16:19.826Z] 00:16:19     INFO -      Which is: 2 @ /builds/worker/checkouts/gecko/gfx/layers/apz/test/gtest/APZTestCommon.h:893
[task 2020-05-02T00:16:19.826Z] 00:16:19  WARNING -  TEST-UNEXPECTED-FAIL | APZCBasicTester.Overzoom | test completed (time: 0ms)
[task 2020-05-02T00:16:19.826Z] 00:16:19     INFO -  TEST-START | APZCBasicTester.SimpleTransform

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d624406bd3c
Handle content prevent defaulting pinch gestures. r=kats

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4d624406bd3c

Comment 11

[Bogdan Maris, Desktop Test Engineering]

Reproduced the initial issue using an old Nightly from 2020-03-01 with both the testcase and google maps, verified that the issue is not reproducible anymore using latest Firefox 77.0b7 on macOS 10.15.5 beta 3.