Closed Bug 1651050 Opened 4 years ago Closed 4 years ago

Don't automatically make a zoomable scrollframe the primary displayport-ized scrollframe

Categories

(Core :: Panning and Zooming, task)

task

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files)

Spinoff from https://bugzilla.mozilla.org/show_bug.cgi?id=1621740#c23

With apz.allow_zooming=true the root scrollframe generally has mZoomable = true, which makes it return true from WantAsyncScroll(). In cases where the page is not scrollable (because content is too short, or because of overflow:hidden), returning true from WantAsyncScroll() is kind of undesirable, because it ends up getting the displayport that we create here even though it's not really needed for that scrollframe. The primary scroller that the user will actually scroll is more likely to be the first nested scroller, and that should be the one getting the displayport. The displayport can be applied to the root scrollframe lazily, on zooming in.

The current behaviour also causes a bunch of tests to fail with apz.allow_zooming=true because of this change in layerization and where the displayport ends up.

If the scrollable div ends up as the first-encountered scrollframe, it gets a
big displayport that is just over 5000px CSS pixels tall (on GeckoView). When we
check for images being "nearly visible" we expand that displayport further by a
pref-controlled multiplier, which ends up doubling the displayport height to
over 10000 CSS pixels. So for the image to actually get detected as outside the
"nearly visible rect" it needs to be positioned further down in the iframe.
This test was previously passing on GeckoView because the scrollable div was
not getting detected as the first-encountered scrollframe; the root scrollframe
for the top-level document was the one getting the displayport even though it
didn't really need it. A later patch in this bug changes the first-encountered
scrollframe, causing this test to fail without this patch.

If the root scrollframe has the zoomable flag set, it automatically returns
true from WantAsyncScroll which makes it the "primary" scrollframe, and
automatically gets a displayport. However, this can be undesirable in cases
where the root scrollframe is not actually scrollable (i.e. document content
doesn't overflow) because we don't actually need that displayport on the root
scrollframe and instead want it on a scrollframe that is actually scrollable.

This patch removes the behaviour of WantAsyncScroll returning true for such
scrollframes, but maintains the layerization changes needed to support zooming.
This reduces the set of differences between running with apz.allow_zooming on
and off, which in turn eliminates a bunch of test failures when that pref is
enabled.

Depends on D82777

This is useful for debugging purposes.

Depends on D82778

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8b6bf433713e Make sure the iframe is actually outside the "nearly visible" displayport. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/545733300dad Change how the zoomable flag is used in scrollframes. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/865ee4c1a387 Add a logging module for code that sets displayports. r=tnikkel
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: