Closed Bug 1619187 Opened 4 years ago Closed 4 years ago

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

Categories

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

75 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla77
Tracking Status
firefox77 --- verified

People

(Reporter: hujq, Assigned: tnikkel)

References

(Blocks 1 open bug)

Details

(Whiteboard: apz-planning)

Attachments

(2 files)

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.

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

Component: Untriaged → Panning and Zooming
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true

Markus had some ideas about how to handle this.

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.

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.

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.

Whiteboard: desktop-zoom-nightly
Whiteboard: desktop-zoom-nightly → apz-planning
Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70d9a096070b
Handle content prevent defaulting pinch gestures. r=kats

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
Flags: needinfo?(tnikkel)
Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4d624406bd3c
Handle content prevent defaulting pinch gestures. r=kats
Flags: needinfo?(tnikkel)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Flags: qe-verify+
Regressions: 1638535

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Regressions: 1643517
See Also: → 1648489
See Also: → 1658001
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: