1669982 - Blank page is displayed on a pinch zoomed page after hitting the back button

Status: VERIFIED FIXED

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

Description

[Hani Yacoub, Desktop QA]

Affected versions

Firefox Nightly 83.0a1

Affected platforms

Windows 10 x64
MacOS x 10.15

Preconditions

fx.webrender.all = true OR fx.webrender.all = false
apz.allow_zooming = true
apz.windows.use_direct_manipulation = true

Steps to reproduce

1. Open Firefox
2. Go to ebay.com
3. Pinch zoom on any link/button
4. Click on it
5. Click the back button

Expected result

The content of the page should be correctly displayed on a pinch zoomed page after hitting the back button

Actual Result

• Blank page is displayed on a pinch zoomed page after hitting the back button
• The page is correctly displayed only after clicking or pinch zoom the page

Suggested severity

S2

Comment 1

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

This seems like more similar to bug 1670877. It looks like the displayport is correct initially, but then some time around page load it gets clobbered. Initially the DisplayPortMargins struct has sane margins, layout, and visual offsets. But then the new one that comes in is either empty or just has the margins, and so loses the layout vs relative delta. So then the painted displayport ends up relative to the layout offset instead of the visual offset.

Sadly I couldn't repro on my Linux machine and use rr so I need to use more painful debugging methods on macOS.

Comment 2

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

https://searchfox.org/mozilla-central/rev/803b368879fa332e8e2c1840bf1ec164f7ed2c32/layout/base/GeckoMVMContext.cpp#190-191 is the culprit it seems. At least the main one; there might be other codepaths that trigger the same thing.

Comment 3

[Kris Taeleman (:ktaeleman)]

Attachment: firefox-2020-10-13.zip

Logs of repro case. Was able to repro this on desktop with mousewheel.with_control.action= 5 and ctrl zoom on button on the left side of the screen

Comment 4

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

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=06e916c7cf6530165eab10577bef13e00d13833e

Comment 5

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

Attachment: Bug 1669982 - Add a log to catch suspicious calls to SetDisplayPortMargins. r?botond

Comment 6

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

Attachment: Bug 1669982 - Keep the margin adjustment when calculating margins on the main thread. r?botond

The code in CalculateAndSetDisplayPortMargins computes metrics for the content,
which may be the RCD and therefore have separate visual and layout scroll offsets.
The code then uses CalculatePendingDisplayPort to compute displayport margins,
but that function computes the margins based on the visual scroll offset. The
code then uses that as the final margins, when in fact those margins might
need adjusting so that they can be applied to the base rect (which is based
on the layout scroll offset).

This function is invoked by MobileViewportManager after load complete, at
which point the displayport may already be set and the layout and visual offsets
may have diverged. This can happen if, for example, the user manipulates the
visual viewport early during page load, or if a visual viewport is restored
after navigating backwards. In these scenarios the existing "with adjustment"
displayport margins are clobbered by the new, incorrect "with no adjustment"
margins. This patch corrects this by specifying the necessary adjustment.

All the other call sites of this function only call it to initialize the
displayport for the first time, so they cannot run into this problem of
"clobbering" an existing margins. However, I kept this patch general enough
so that if any of those call sites were to change in the future, it wouldn't
run into the same problem.

Depends on D93493

Comment 7

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

Attachment: Bug 1669982 - Preserve margins adjustment for a fallback codepath. r?botond

The warning log statement from two patches up was also triggered by this
codepath early during page load while I was doing manual testing. It's
not clear to me if this has user-observable effects, but from a code inspection
it seems like the margins here should also be subject to the same adjustment.
The recentering code simply normalizes the margin around the same "base rect"
which in this case is the visual offset, since the margins are coming from APZ.
So for the margins to then be applied to main-thread base rect, it needs an
adjustment from the visual space to the layout space.

Depends on D93494

Comment 8

[Kris Taeleman (:ktaeleman)]

Tried the above build and can no longer reproduce this issue and 1670877.

Comment 9

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

(In reply to Kris Taeleman (:ktaeleman) from comment #8)

Tried the above build and can no longer reproduce this issue and 1670877.

Thanks for confirming!

I wrote a test that fails without the patch, seems to pass with the patch: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=e7ec6f64551b9d2a9abd8337e713cea6816d9e6f

Comment 10

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

Attachment: Bug 1669982 - Add a test. r?botond

Depends on D93494

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e469c93effb1
Add a log to catch suspicious calls to SetDisplayPortMargins. r=botond
https://hg.mozilla.org/integration/autoland/rev/e6611beb37d6
Keep the margin adjustment when calculating margins on the main thread. r=botond
https://hg.mozilla.org/integration/autoland/rev/73df8ab9e6e5
Add a test. r=botond

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e469c93effb1
https://hg.mozilla.org/mozilla-central/rev/e6611beb37d6
https://hg.mozilla.org/mozilla-central/rev/73df8ab9e6e5

Comment 13

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox Nightly 83.0a1 (2020-10-18) on Windows 10 x64 and on MacOS x 10.15 using touchpad and a touchscreen.