1805601 - https://wpt.fyi/ is broken after pinch zooming then resetting

Status: VERIFIED FIXED

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

Description

[Gregory Pappas [:gregp]]

Attachment: page_cut_in_half.webm

Steps to reproduce:

1. Navigate to https://wpt.fyi/
2. Pinch zoom at the top right corner
3. Reset pinch zoom with Ctrl+0 keyboard shortcut

Actual results:
Page is cut in half

Expected results:
Page is normal and centered

Comment 1

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

Set release status flags based on info from the regressing bug 1803863

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

For more information, please visit auto_nag documentation.

Comment 2

[Gregory Pappas [:gregp]]

Whoops, found a better range

Comment 3

[Gregory Pappas [:gregp]]

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

For more information, please visit auto_nag documentation.

Comment 4

[Botond Ballo [:botond]]

Confirmed. Seems to happen to other sites too.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Interestingly I can't reproduce either on Windows 11 or on Linux, but I can see the issue on Mac.

Comment 6

[Botond Ballo [:botond]]

The specific regressing patch is https://phabricator.services.mozilla.com/D163532.

Comment 7

[Botond Ballo [:botond]]

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

The specific regressing patch is https://phabricator.services.mozilla.com/D163532.

This patch fixes a bug where, after zooming out, which shrinks the visual scroll range, the visual viewport offset is not re-clamped to the new scroll range on the main thread, resulting in the main thread's copy of the visual viewport offset temporarily being out of bounds.

That bug only affects cases where the zooming out is done by calling SetResolutionAndScaleTo() directly, rather than through APZ pinch-zoom or double-tap zoom. The test in bug 1519339 does this, and indeed I thought only tests could do this, though I now realize that the implementation of Ctrl+0 does this as well.

The reason this fix caused a regression is that it exposed another latent bug where, in the affected scenario, APZ didn't re-clamp the visual viewport offset either.

The two bugs effectively cancelled each other out by interacting in a weird way (basically, the first bug led to the main thread having an invalid visual viewport offset, which then caused the main-thread logic for trying to preserve the relative scroll offset to scroll to another invalid offset, which triggered a scroll offset update to APZ, which caused APZ to take a codepath where it does re-clamp the visual viewport offset). Fixing the first bug exposed the second and therefore regressed Ctrl+0 behaviour.

Comment 8

[Botond Ballo [:botond]]

Attachment: Bug 1805601 - Reclamp the visual viewport offset in APZ if the zoom changes. r=tnikkel,dlrobertson

We already had logic for reclamping the visual viewport offset if the
composition bounds changes, but since the composition bounds is in
ParentLayer (post-zoom) coordinates, this didn't handle the case where
the zoom changes.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5d669d49e62
Reclamp the visual viewport offset in APZ if the zoom changes. r=tnikkel

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/e5d669d49e62

Comment 12

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

The patch landed in nightly and beta is affected.
:botond, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox109 to wontfix.

For more information, please visit auto_nag documentation.

Comment 13

[Botond Ballo [:botond]]

Yes, this is something we'd like to uplift.

Comment 14

[Botond Ballo [:botond]]

Comment on attachment 9308881 [details]
Bug 1805601 - Reclamp the visual viewport offset in APZ if the zoom changes. r=tnikkel,dlrobertson

Beta/Release Uplift Approval Request

• User impact if declined: After pinch-zooming in, resetting the zoom level via Ctrl+0 (or View --> Zoom --> Actual Size) may temporarily result in the page being rendered at an out-of-bounds scroll offset, until the page is scrolled or something else triggers a repaint
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a simple change to clamp an out-of-bounds scroll offset to its valid range, unlikely to have unexpected effects
• String changes made/needed:
• Is Android affected?: No

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9308881 [details]
Bug 1805601 - Reclamp the visual viewport offset in APZ if the zoom changes. r=tnikkel,dlrobertson

Approved for Desktop 109.0b6 and Mobile 109.0b3.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0a143142f96d

Comment 17

[Bogdan Maris, Desktop Test Engineering]

I tried to reproduce this on my MacBook with macOS 13.1 using an old Nightly from 2022-12-13 but I was unable to. Also due to holidays we are in lack of a Laptop or a touchscreen with Linux, Windows so we are unable to verify this issue.

Gregory Pappas [:gregp] would you mind verifying that this is fixed for you in the latest builds of Nightly and Beta (109.0b6)? You can download them using the following links:
Beta - https://archive.mozilla.org/pub/firefox/candidates/109.0b6-candidates/build1/linux-x86_64/en-US/firefox-109.0b6.tar.bz2
Nightly - https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/firefox-110.0a1.en-US.linux-x86_64.tar.bz2

Comment 18

[Gregory Pappas [:gregp]]

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #17)

Gregory Pappas [:gregp] would you mind verifying that this is fixed for you in the latest builds of Nightly and Beta (109.0b6)? You can download them using the following links

I can verify that the bug is fixed in Firefox Nightly 20221220093956 and Firefox Beta 109.0b6.

Comment 19

[Bogdan Maris, Desktop Test Engineering]

Thanks for verifying! Closing as verified based on the above.