1664558 - Can't zoom embedded Google Maps using Ctrl + scroll trackpad gesture with Fission

Status: RESOLVED FIXED

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

Description

[Chris Peterson [:cpeterson]]

+++ This bug was initially created as a clone of Bug #1658647 +++

Steps to reproduce

1. In Fx82 Nightly, load a page with an embedded Google Map, such as https://developers.google.com/maps/documentation/javascript/interaction.
2. Scroll the embedded map on screen.
3. Try to zoom the map in and out using Ctrl + scroll trackpad gesture.

Expected result

The map should zoom in and out.

Actual result

The page scrolls instead of the map zooming. If I open the page in a non-Fission window, then I can zoom the map.

If I set the apz.allow_zooming pref = false and restart Firefox, then I can zoom the map (with or without Fission).

I'm using Fx82 Nightly on Windows 10.

Comment 1

[Botond Ballo [:botond]]

Hanging this off of desktop-zoom-post for now so it shows up on our desktop zooming dashboard. We can revisit (whether it needs to block desktop-zoom-release) once a Fission milestone is assigned.

Comment 2

[Chris Peterson [:cpeterson]]

tnikkel, you fixed this Google Maps bug for apz.allow_zooming in bug 1658647, but I can still reproduce the problem when Fission (fission.autostart) is enabled. Seems like APZ using a different code path with Fission?

Tracking this bug for Fission Nightly milestone M6c

Comment 3

[Timothy Nikkel (:tnikkel)]

My first thought is that it is related to the events not getting to where they need to be (hit testing/routing) and not particularly related to the dmanip issues I've previously fixed, but I haven't looked into this at all, so that could be completely wrong. There are other open bugs with hit testing and fission and I'm not completely clear on the state and what we expect to work and not work.

Comment 4

[Timothy Nikkel (:tnikkel)]

Botond, I think you have looked into some fission hit testing bugs recently. Is there a quick way to tell if this is a fission hit testing issue?

Comment 5

[Botond Ballo [:botond]]

I'd start by looking at the layers id that APZ Is assigning to the pan gesture event (assuming that's the relevant event type) here, and cross-referencing it against the layers ids of the RefLayers in a layer dump, to see if the event is being targeted to the content process we expect.

Comment 6

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

I can take a look at this, since I'm looking into bug 1541589 in nearby code.

Comment 7

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

The functions in InputData.cpp to convert from InputData types to WidgetEvent types aren't propagating the mLayersId in most cases. Fixing that makes the zooming work. However there's still a bug in that on macOS with non-fission, if I pan over the map, it gives me an overlay saying "use command + scroll to zoom the map". With my fix for this bug, zooming happens right away even if I'm not holding down the command modifier. It's not clear to me why, the modifiers seem correct as far as I can tell.

Comment 8

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

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #7)

With my fix for this bug, zooming happens right away even if I'm not holding down the command modifier. It's not clear to me why, the modifiers seem correct as far as I can tell.

Never mind. I didn't realize the page in question actually has multiple embedded maps with different gesture handling properties. I was scrolling over different maps in my local build vs non-fission Nightly.

Comment 9

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

I was wondering why our existing fission tests didn't catch this problem. Looks like most or all of our tests go through the InputData <-> WidgetEvent conversion code in APZInputBridge.cpp, e.g. this one. However things like PanGestureInput or PinchGestureInput which are created directly in the widget and then converted to gecko events after running them through APZ, go through the conversion in InputData.cpp and so were missing the LayersId propagation.

Comment 10

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

Attachment: Bug 1664558 - Propagate the layers id from InputData to gecko WidgetEvent types. r?botond

Seems like an oversight that these fields were never propagated from APZ to
the generated main-thread WidgetEvents.

Comment 11

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

Attachment: Bug 1664558 - Improve layers id logging in layer tree. r?botond

Depends on D94894

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66d877d0037e
Propagate the layers id from InputData to gecko WidgetEvent types. r=botond
https://hg.mozilla.org/integration/autoland/rev/cf70edfb90e9
Improve layers id logging in layer tree. r=botond

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/66d877d0037e
https://hg.mozilla.org/mozilla-central/rev/cf70edfb90e9