Closed Bug 1646385 Opened 1 year ago Closed 1 year ago

Make sure the visual viewport is properly updated when layout scrollbars are added or removed

Categories

(Core :: Panning and Zooming, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files)

The visual viewport is, per spec supposed to exclude layout (i.e. non-overlay) scrollbars. The MobileViewportManager does this correctly when it's asked to compute the visual viewport, here. However, the problem is that it's not asked to recompute the visual viewport after the root scrollframe does a reflow, which may change the present of the scrollbars.

So after the root scrollframe does a reflow, we need to trigger a recomputation of the visual viewport. That's easy enough.

The next thing to keep in mind is that when the visual viewport changes, we need to reflow the fixed-pos items, because they take the visual viewport into account when positioning themselves, here. So if the root scrollframe does a reflow which adds/removes scrollbars, and then we update the visual viewport, we then need to do another reflow to update the fixed-pos items. OR, we can do it all in a single reflow, if we make sure to recompute the visual viewport between the root scrollframe doing it's thing, and the fixed-pos items doing theirs. This latter approach will reduce work, and so is preferable.

Ok, so that's not hard either. The only other complication is that when the visual viewport updates, it marks various things as dirty, here, but doing that is not allowed in the middle of a reflow (presumably because the items being marked dirty may already have been reflowed, and then the dirty flags get cleared at the end of the reflow, leading to potentially dirty things left behind).

Oh and the one last complication is that if font inflation is enabled, we also call this function to reflow descendant subframes. Although the placement of this call is kind of unorthodox (i.e. it happens on any call to set the visual viewport, even if there was no change), it still fits within the overall solution model we have, because we can (a) reflow the root scrollframe, (b) update the visual viewport, and then (c) reflow fixed-pos and mark subdocuments dirty, and that can all happen within a single reflow.

There are two parts here. One is the "backstop" in the scrollframe's
ReflowFinished callback, that recomputes the visual viewport size if layout
scrollbars are being used in the root scrollframe. This ensures that the
visual viewport gets resized properly after a reflow, possibly at the expense
of a second reflow to reposition fixed-pos items.

There is also an update to the visual viewport during the reflow itself, after
we have reflowed the in-flow contents (including the root scrollframe) but
before we reflow the fixed-pos items. This allows us to avoid the second reflow
by using the new visual viewport for positioning the fixed-pos items correctly.
This early reflow also skips marking things for a second reflow.

This patch fixes a problem described in bug 1644271 comment 2, among other
things. Specifically, it ensures that when the scrollbar properties (e.g.
maxpos, minpos) are computed, they are computed using an up-to-date visual
viewport size. The up-to-date visual viewport size correctly excludes the space
taken up by non-overlay scrollbars, which wasn't happening without this patch.

Depends on D80039

There's no need to put this in nsLayoutUtils since it's so closely bound
to presShell anyway. So we move it from being a static nsLayoutUtils function
that takes a PresShell pointer, to being a method on the PresShell itself.

Two functional changes here:

  1. Don't run the MaybeReflowForInflationScreenSizeChange code unless the
    visual viewport actually changes
  2. Run the MaybeReflowForInflationScreenSizeChange if the visual viewport
    is reset.

These functional changes can be logically thought of as "group the font-
inflation reflow stuff together with the other visual-viewport-triggered
reflow stuff".

Depends on D80040

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/457df9116f25
Ensure we recompute the visual viewport size when the root scrollframe's scrollbars get added or removed. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/cccaf6992a22
Move SetVisualViewport goop from nsLayoutUtils to PresShell. r=tnikkel
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
Depends on: 1647953
No longer depends on: 1647953
You need to log in before you can comment on or make changes to this bug.