1917488 - Switching tabs while zooming in/out causes further zoom in/outs to happen in wrong tab

Status: RESOLVED FIXED

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

Description

[mcccs]

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

Steps to reproduce:

I encounter this bug very often in my normal browsing experience but it's harder to trigger it instantly for testing experience:

Crazily zooming in and out while having two tabs open and switching between them causes at some point not being able to zoom in/out, and it turns out that the tab in the background did zoom in/out instead!

Mozregression (it is actually hard to use this because it's hard to decide "it has had enough testing" with 100% confidence when it's working correctly.):
2024-09-08T19:50:19.886000: DEBUG : Found commit message:
Bug 1902017 - Update the event layers id to that of the APZ pan gesture block state. r=botond

Ensure that weel events for a pan gesture are sent to the content
process for the layers id of the APZC that is handling the pan gesture.
The corresponding content process should also have a content side wheel
transaction that will ensure that content that is scrolled over cannot
halt a user scroll with preventDefault on a wheel event listener.

Differential Revision: https://phabricator.services.mozilla.com/D215143

Comment 1

[mcccs]

By "zooming in", I mean pressing CTRL with the pref "mousewheel.with_control.action" set to 3

Comment 2

[mcccs]

Also I'm not sure if it happens on other than MacOS as well

Comment 3

[Botond Ballo [:botond]]

Thank you for the report and for finding a regression window!

Comment 4

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

:dlrobertson, since you are the author of the regressor, bug 1902017, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 5

[Dan Robertson (:dlrobertson)]

Thanks for the bug report, we probably need to ignore any future momentum events on mac after a tab switch.

Comment 6

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

The severity field is not set for this bug.
:botond, could you have a look please?

For more information, please visit BugBot documentation.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Setting P2 since it's kinda new-ish regression.

Comment 8

[[:estemal]]

For me it reproduces reliably. No need for any crazy switching. Just switch the tab once, then try to zoom with Ctrl+scroll (I use touchpad two-finger scroll) and it doesn't zoom the new tab, zooms the previous tab instead. OS - Linux.

Comment 10

[Botond Ballo [:botond]]

There are more precise reproduction steps in the duplicate bug 1918858, specifically bug 1918858 comment 5.

Comment 11

[Botond Ballo [:botond]]

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

There are more precise reproduction steps in the duplicate bug 1918858, specifically bug 1918858 comment 5.

Iterating on this further, here are steps that reproduce the issue for me reliably on Linux:

1. Run Firefox in a clean profile (this is important -- some addons seems to interfere with reproducibility), with MOZ_USE_XINPUT2=1 in the environment in an X11 session
2. Open two tabs, with https://theres-waldo.github.io/mozilla-testcases/long-10000px.html in the first tab and http://example.com/ in the second
3. Switch to the first tab. Scroll with the trackpad. Wait for the momentum scrolling to stop.
4. Switch to the second tab with Ctrl+Tab.
5. Try to reflow-zoom in on the second tab with Ctrl + trackpad scroll.

Comment 12

[Botond Ballo [:botond]]

I believe I understand why this is happening:

• Nothing clears InputQueue::mActivePanGestureBlock, it just gets overwritten when the next pan gesture starts
• However, the creation of a new pan gesture block happens in InputQueue, and processing of a pan gesture input by InputQueue is conditional on mHandledByApz...
• ... which is false for pan gesture inputs with the Ctrl key held down (because those are indeed not handled in APZ)
• MaybeOverrideLayersIdForWheelEvent however (which was modified to also handle pan gesture inputs in the regressing bug, bug 1902017) is not conditional on mHandledByApz
• So when the pan gesture inputs received while Ctrl is held down are processed, the function overrides the LayersId of those events to the one stored in the old pan gesture block (which is still in mActivePanGestureBlock), and the events are routed to the wrong content process

Comment 13

[Botond Ballo [:botond]]

Attachment: Bug 1917488 - Allow MockHitTester to hit layers with a LayersId other than the default of 0. r=dlrobertson

Comment 14

[Botond Ballo [:botond]]

Attachment: Bug 1917488 - Do not group events which are not handled by APZ into a transaction. r=dlrobertson

Comment 15

[Botond Ballo [:botond]]

Attachment: Bug 1917488 - Avoid a debug assertion in nsBaseWidget::MayStartSwipeForAPZ(). r=dlrobertson

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac40d29ea5dd
Allow MockHitTester to hit layers with a LayersId other than the default of 0. r=dlrobertson
https://hg.mozilla.org/integration/autoland/rev/10ebaec709a5
Do not group events which are not handled by APZ into a transaction. r=dlrobertson
https://hg.mozilla.org/integration/autoland/rev/f782d2f3bb48
Avoid a debug assertion in nsBaseWidget::MayStartSwipeForAPZ(). r=dlrobertson

Comment 17

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/ac40d29ea5dd
https://hg.mozilla.org/mozilla-central/rev/10ebaec709a5
https://hg.mozilla.org/mozilla-central/rev/f782d2f3bb48

Comment 19

[Botond Ballo [:botond]]

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

• Nothing clears InputQueue::mActivePanGestureBlock, it just gets overwritten when the next pan gesture starts

I filed bug 1933312 as a follow-up to be more proactive about clearing mActivePanGestureBlock.