Closed Bug 1658001 Opened 4 years ago Closed 4 years ago

Can't zoom Bing Maps out all the way using pinch zoom trackpad gesture

Categories

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

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
81 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox79 --- unaffected
firefox80 --- unaffected
firefox81 --- verified

People

(Reporter: cpeterson, Assigned: tnikkel)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

This bug is a regression from allow_zooming bug 1620055. I bisected the regression to this pushlog:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a518405abe4c93e6d8df622f59d5f01c4766e1d5&tochange=a09e4ce9ebba8fa4a29a4833560c8ee807018c03

STR

  1. Load https://www.bing.com/maps/ in Fx81 Nightly.
  2. Zoom the map in and out using a pinch zoom gesture on my laptop's trackpad.

Expected Result

You should be able to zoom the map out to show the full world like in Chrome and Firefox 80.

Actual Result

You can't zoom out beyond the 10 mile zoom level using the pinch zoom trackpad gesture. You can still zoom out using the map's +/- zoom buttons.

I'm using Windows 10.

Does zooming in work properly? For me, zooming in is handled by the browser rather than the page, suggesting a breakdown of the dispatch-to-content mechanism (which then explains not being able to zoom out too).

(In reply to Botond Ballo [:botond] from comment #1)

For me, zooming in is handled by the browser rather than the page, suggesting a breakdown of the preventDefault() mechanism (which then explains not being able to zoom out too).

(This is likely a different issue as I was testing with mousewheel configured to perform pinch-zoom via the mousewheel.with_<modifier>.action prefs. That goes through a different codepath for which the dispatch-to-content mechanism is just altogether broken.)

(In reply to Botond Ballo [:botond] from comment #1)

Does zooming in work properly? For me, zooming in is handled by the browser rather than the page, suggesting a breakdown of the preventDefault() mechanism (which then explains not being able to zoom out too).

Like you said, it looks like zooming in is handled by the browser. The map text gets big and the map controls move off screen, but I don't a % page zoom level in the browser's address bar and can't Ctrl+0 to reset the page zoom level.

The map code handled zooming in and out before apz.allow_zooming = true.

(Sometimes when I zoom in and out far, the pinch zoom in and out gestures seem to get reversed. But that seems unrelated to this bug.)

(This is likely a different issue as I was testing with mousewheel configured to perform pinch-zoom via the mousewheel.with_<modifier>.action prefs. That goes through a different codepath for which the dispatch-to-content mechanism is just altogether broken.)

Filed bug 1658009 for this.

(In reply to Chris Peterson [:cpeterson] from comment #3)

(In reply to Botond Ballo [:botond] from comment #1)

Does zooming in work properly? For me, zooming in is handled by the browser rather than the page, suggesting a breakdown of the preventDefault() mechanism (which then explains not being able to zoom out too).

Like you said, it looks like zooming in is handled by the browser. The map text gets big and the map controls move off screen, but I don't a % page zoom level in the browser's address bar and can't Ctrl+0 to reset the page zoom level.

Right, I guess there are three different behaviours to distinguish between:

  1. Zooming is handled by the page, not the browser. This is what sites like Google Maps and Bing Maps want to do (and they do it by registering event listeners for the relevant event types and preventDefault()ing the events).
  2. Zooming is handled by the browser, and the browser performs scaling zoom. This is the new thing that was enabled in bug 1620055 (on desktop; mobile has always had it). Scaling zoom does not use the % indicator and does not use shortcuts like Ctrl+0.
  3. Zooming is handled by the browser, and the browser performs reflowing zoom. This uses the % indicator and responds to shortcuts like Ctrl+0.

So, on a normal (non-Maps-like) page, getting (2) instead of (3) in response to a trackpad pinch gesture (including not having a % indicator) is an intended effect of bug 1620055.

However, on Maps-like pages that want to handle zoom themselves, we should continue to get behaviour (1).

Summary: Can't zoom Bing Maps out all the way using pinch zoom trackpad gesture → Dispatch-to-content mechanism is broken on Bing Maps (trackpad pinch gesture is handled by browser)

(In reply to Botond Ballo [:botond] from comment #5)

  1. Zooming is handled by the page, not the browser. This is what sites like Google Maps and Bing Maps want to do (and they do it by registering event listeners for the relevant event types and preventDefault()ing the events).

("Dispatch-to-content mechanism" is our name for our mechanism to wait and see if the page preventDefault()s the relevant events, and handle zooming in the browser if it does not.)

When I test this (on mac) I find that bing maps does not respond at all to pinch gestures when apz.allow_zooming is false.
When apz.allow_zooming is true the browser handles the zooming, not the page.

In Chrome pinch gestures are handled by the page.

In Safari pinch gestures are handled by the browser.

On Windows I see the same as Chris (apz.allow_zooming == false => pinch gestures handled by page, apz.allow_zooming == true => pinch gestures handled by browser).

On Chrome there is a mousewheel listener that does the zooming from pinch gestures (confirmed by disabling it).

On Firefox there is no mousewhell listener, there is a DOMMouseScroll listener. I can't tell if the event handler calls preventdefault or not from the inspector.

I did try a test page with a DOMMouseScroll listener and calling preventDefault does work on my simple test page.

Disable dmanip fixes the issue, which is not too surprising: the dmanip implementation of pinch gestures was based on the macos code for the same and when dmanip is disable pinch gestures send ctrl+mousewheel events at the widget level.

(In reply to Timothy Nikkel (:tnikkel) from comment #7)

I did try a test page with a DOMMouseScroll listener and calling preventDefault does work on my simple test page.

I must have made a mistake in testing this. This appears to be a false statement.

Summary: Dispatch-to-content mechanism is broken on Bing Maps (trackpad pinch gesture is handled by browser) → Can't zoom Bing Maps out all the way using pinch zoom trackpad gesture

We don't send DOMMouseScroll events from pinch gestures generated from direct manipulation because the widget wheel events created in PinchGestureInput::ToWidgetWheelEvent don't set a non-zero mLineOrPageDeltaX/Y (the condition that EventStateManager::DispatchLegacyMouseScrollEvents uses). So this is basically bug 1653700 except for pan gestures instead of pinch gestures. If I set mLineOrPageDeltaY based on the computed deltaY in the same way as pan gestures it fixes the bug.

We do this analogously to how PanGestureInput does it except that the delta's that we compute mLineOrPageDeltaY from are computed by us instead of provided to us.

mLineOrPageDeltaY being non-zero is what EventStateManager::DispatchLegacyMouseScrollEvents uses to decide to send legacy mouse events, so we need to populate it to get those legacy events to send.

This fix is Windows only on purpose as pinches on macOS don't seem to send wheel events (Windows sends ctrl+wheel). When Linux gets implemented it will need to be determined what to do.

Assignee: nobody → tnikkel
Status: NEW → ASSIGNED
Severity: -- → S3
Priority: -- → P3
See Also: → 1658647
Blocks: 1630912
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bd9655461cbc Populate wheelEvent.mLineOrPageDeltaY in PinchGestureInput::ToWidgetWheel for pinch gestures produced from direct manipulation. r=kats
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Flags: qe-verify+

Hi Chris, can you please help us check if the bug is fixed on 81 RC2? It seems that I am not having any luck in preproducing this bug with your steps, on an affected Nightly build 81.0a1 (20200807093158); the map is correctly zoomed out to its maximum value, on my Win 10 machine (via trackpad pinch zoom gesture).

Flags: needinfo?(cpeterson)

(In reply to Ciprian Georgiu [:ciprian_georgiu], Release Desktop QA from comment #14)

Hi Chris, can you please help us check if the bug is fixed on 81 RC2? It seems that I am not having any luck in preproducing this bug with your steps, on an affected Nightly build 81.0a1 (20200807093158); the map is correctly zoomed out to its maximum value, on my Win 10 machine (via trackpad pinch zoom gesture).

Verified fixed. Pinch zooming Bing Maps works correctly for me in the latest 81 RC, with or without the apz.allow_zooming pref enabled.

Status: RESOLVED → VERIFIED
Flags: needinfo?(cpeterson)

Thank you, Chris.

Flags: qe-verify+
See Also: → 1694311
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: