Closed Bug 1909181 Opened 3 months ago Closed 3 months ago

Blank area at bottom of screen when zoomed in on Android with dynamic toolbar

Categories

(Core :: Layout: Scrolling and Overflow, defect)

Firefox 130
Unspecified
Android
defect

Tracking

()

VERIFIED FIXED
131 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 --- unaffected
firefox128 --- unaffected
firefox129 --- unaffected
firefox130 --- verified
firefox131 --- verified

People

(Reporter: ke5trel, Assigned: hiro)

References

(Regression)

Details

(Keywords: regression)

Attachments

(5 files)

STR:

  1. Have the dynamic toolbar enabled (default) on latest Android Nightly 130.0a1.
  2. Visit https://en.wikipedia.org.
  3. Zoom in.
  4. Scroll down.

Blank area appears at bottom of screen, larger with increased zoom.

Does not happen with static toolbar.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=980f79f57946301607ffb6a3d091ca965f9c4140&tochange=111141cbf85939ddd0511a7456ec242487a0a60a

Regressed by Bug 1788504.

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

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)

Hiro mind taking a look? You're more familiar with MVM stuff than I am :)

Flags: needinfo?(emilio) → needinfo?(hikezoe.birchill)

The code regressed is this GetScrollPortRectAccountingForMaxDynamicToolbar call in ScrollContainerFrame::BuildDisplayList. Since bug 1788504, the rect became slightly smaller than before because the max dynamic toolbar height is divided by the current pinch zoom scale.

Flags: needinfo?(hikezoe.birchill)

Since bug 1788504 the root scroll container's clipped region had been
over-clipped because GetScrollPortRectAccountingForMaxDynamicToolbar returns
the toolbar height divided by the current pinch zoom ratio. For example, given
that the toolbar height is 60px (in screen units) and the pinch zoom ratio is
2.0, then GetScrollPortRectAccountingForMaxDynamicToolbar returns 30px (in CSS
units), it's not sufficient for the clipped region.

The JUnit test in this change fails without this change, succeeds with this
change.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED

I also wondered that Chrome's IntersectionObserver divides the toolbar height by the current pinch zoom scale or not, but it's quite hard to tell since IntersectionObserver doesn't check the intersections between the visual viewport, it rather checks the layout viewport, thus while the document is zoomed in, the intersections happen outside of the visual viewport.

So I will punt off the question here. I think some edge cases will reveal the question. (My intuitive answer is Chrome uses the minimum scale, I am quite unsure though)

Since bug 1788504, with dynamic toolbar, viewport units had depended on
pinch-zoom scale. That's totally wrong. Viewport units should be independent
from pinch-zoom scale. A couple of examples have been attached in bug 1910518.

This is a 130 regression, can we have the fix landed before we merge to beta? Thanks

Flags: needinfo?(hikezoe.birchill)

I'd want to, but it depends on #geckoview-reviewers.

Flags: needinfo?(hikezoe.birchill)

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

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

For more information, please visit BugBot documentation.

Flags: needinfo?(hikezoe.birchill)
Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8d92be4e0f88 Revert bug 1788504 partially. r=emilio,geckoview-reviewers,ohall https://hg.mozilla.org/integration/autoland/rev/f56f6d107cfe Make viewport units independent from pinch-zoom scale. r=emilio

With renaming the test file name from test_bug1909181.html to test_viewport_units_with_dynamic_toolbar.html. There was no suspicious failure; https://treeherder.mozilla.org/jobs?repo=try&revision=1381e80713ab682d8a56e8030ebbc8c8d213598b

At least for the test failure on test_bug1909181.html, it's somehow failed by the previous test, test_bug1836801.html. Attaching file is a screenshot when test_bug1909181.html failed. It's fullscren state.

Flags: needinfo?(hikezoe.birchill)

CCing Timothy. ^

Given that there's no test failure with renaming the test file, I will land patches here with the renaming change (tomorrow).

Pushed by hikezoe.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eb80cb85023b Revert bug 1788504 partially. r=emilio,geckoview-reviewers,ohall https://hg.mozilla.org/integration/autoland/rev/7ee9d19b1659 Make viewport units independent from pinch-zoom scale. r=emilio
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 131 Branch

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

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

For more information, please visit BugBot documentation.

Flags: needinfo?(hikezoe.birchill)
See Also: → 1912070

Comment on attachment 9416712 [details]
Bug 1909181 - Make viewport units independent from pinch-zoom scale. r?emilio

Beta/Release Uplift Approval Request

  • User impact if declined: Users will see un-rendered area on pinch-zoomed in contents
  • 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 basically harmless changes, it will never cause any critical issues such as crash, it does just expand the rendering area user is seeing
  • String changes made/needed: none
  • Is Android affected?: Yes
Flags: needinfo?(hikezoe.birchill)
Attachment #9416712 - Flags: approval-mozilla-beta?

Hiro, should eb80cb85023b also be uplifted to beta?

Flags: needinfo?(hikezoe.birchill)

Yeah, both changes in this bug need to land together. Thanks.

Flags: needinfo?(hikezoe.birchill)

Comment on attachment 9416712 [details]
Bug 1909181 - Make viewport units independent from pinch-zoom scale. r?emilio

Approved for 130 beta 5, thanks.

Attachment #9416712 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1910244

Verified as fixed on Nightly 130.0a1 from 16.08.2024 and Firefox 130.0b6 with Xiaomi Pad5 (Android 13) , Oppo Find N2 Flip (Android 14) and Oneplus 6T (Android 11).

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: