Don't automatically make a zoomable scrollframe the primary displayport-ized scrollframe
Categories
(Core :: Panning and Zooming, task)
Tracking
()
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.
Assignee | ||
Comment 1•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=47ba97fd135f689a2442a058576c03d6b12cf504 (this one doesn't have apz.allow_zooming=true, to ensure there's no regressions in that configuration). https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=4e29954be48c6a7d90fcf44ba7b5378d52b4e2f9 includes these patches and has apz.allow_zooming=true.
Assignee | ||
Comment 2•4 years ago
|
||
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.
Assignee | ||
Comment 3•4 years ago
|
||
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
Assignee | ||
Comment 4•4 years ago
|
||
This is useful for debugging purposes.
Depends on D82778
Comment 6•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8b6bf433713e
https://hg.mozilla.org/mozilla-central/rev/545733300dad
https://hg.mozilla.org/mozilla-central/rev/865ee4c1a387
Description
•