1529702 - Consider not applying scroll anchor adjustments during layout flushes

Status: NEW

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

Description

[Ryan Hunt [:rhunt]]

Currently we apply scroll anchor adjustments after reflow has finished. This is required by the spec when it says [1]:

Every movement of an anchor node occurs within a window of time called the suppression window, defined as follows:

    The suppression window begins at the start of the current iteration of the HTML Processing Model event loop, or at the end of the most recently completed suppression window, whichever is more recent.

    The suppression window ends at the end of the current iteration of the HTML Processing Model event loop, or immediately before the next operation whose result or side effects would differ as a result of a change in the scroll position (for example, an invocation of getBoundingClientRect()), whichever comes sooner

The problem with this is that 'whose result or side effects would differ as a result of a change in the scroll position' is unspecified but has observable consequences.

For example

// scroll 10px down to activate scroll anchoring
scrollBy({top: 10});

// move the scroll anchor down 10px by expanding sibling
div.style.marginTop = '10px';

// request something that could trigger layout
document.documentElement.clientWidth;

// remove anchor
anchor.remove();

// did we try to apply an anchor adjustment before the anchor was removed
assert(window.scrollY === 20);
// or wait until now?
assert(window.scrollY === 10);

This has caused real interop issues in bug 1520666, bug 1524281, and bug 1526776.

When layout is performed and how browser engines perform batching is unspecified and optimized.

We should consider moving anchor adjustments to happen in a different way. Possibly it could happen as a part of rAF.

[1] https://drafts.csswg.org/css-scroll-anchoring/#suppression-windows

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Do you know what Chrome does for this?

Comment 2

[Ryan Hunt [:rhunt]]

Chrome applies scroll anchor adjustments during layout flushes like we do. This behavior is actually required by all of the WPT scroll anchoring suite.

While updating layout in a block, if the block performs scroll anchoring it queues a callback to the frame view [1] [2]. After layout is finished, the frame view runs all the callbacks [3] [4]. The callback will check if the anchor node moved, and apply scrolling to keep it in the same relative place.

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_block.cc?l=459&rcl=fedbf75b80f08a43bfb576c54d7ea900f2f531f8
[2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/scroll_anchor.cc?l=405&rcl=7645e91f2da8b4e9e5a3985d53cc66d335a4ae5d
[3] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/document.cc?l=2482&rcl=7645e91f2da8b4e9e5a3985d53cc66d335a4ae5d
[4] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/frame/local_frame_view.cc?l=2501&rcl=735d9af4047665c8abc1533ed98a7d380abf28fd

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So if a layout change that happens in a layout flush would have caused an adjustment, would your proposal be that the resulting adjustment (a) is deferred until rAF or (b) is dropped?

Comment 4

[Ryan Hunt [:rhunt]]

I don't have a full proposal yet, just concerns about interop with the current state of things.

A rough idea right now would be to do (a) defer all adjustments until rAF. Here's the process in my mind:

// at some point in rAF, not sure when
for element in scrollableElements:
  if 'anchor was invalidated (scrolling, heuristics, DOM removal)':
    // compute anchor node
  else:
    // compare anchor position since last rAF and perform adjustment 

The reason I think rAF could work here is that it's before we paint and could prevent scroll anchoring from interfering with JS synchronous layout stuff.

But there's a lot of implications I haven't thought through yet.

Comment 5

[Ryan Hunt [:rhunt]]

I thought about this some more last week and discussed the issue with some people at the all-hands.

The current spec requires us to apply scroll anchor adjustments when we flush layout, which is bad for interoperability for the reasons stated above.

My proposal is to move scroll anchor adjustments to happen as part of the steps to 'update the rendering' [1].

Specifically,

  12. Invoke the mark paint timing algorithm for each Document object in docs.
+ 13. Perform the 'scroll anchoring steps.'
  14. For each fully active Document in docs, update the rendering or user interface of that Document and its browsing context to reflect the current state

# Scroll anchoring steps

For each scrollable element within each Document
  1. If a scroll anchor suppression occurred for this element since the last paint, invalidate the scroll anchor
  2. If the element doesn't have a scroll anchor, perform anchor selection
  3. If the element contains a scroll anchor and the scroll anchor offset has changed since the last paint, perform a scroll anchor adjustment

The described scroll anchoring steps would need to occur after layout has been performed (to get an updated scroll offset) as part of the refresh tick, which is why it happens next to the final step of 'Update the rendering'.

The one problem with this, is when do we fire the scroll events from scroll anchor adjustments? We have already fired the scroll events from this paint in step (6) of 'update the rendering', so we have two options.
a. Add another step to fire scroll events from scroll anchor adjustments within this refresh tick
b. Defer scroll events to the next refresh tick

We already have this problem today as we may perform scroll anchor adjustments during layout during a refresh tick. The current code defers the scroll events to the next refresh tick, so it seems like it could be okay to just continue doing this.

I'm going to cc some scrolling stakeholders to see if anyone has more thoughts on this. I think this would help us solve some of our scroll anchoring regressions, which often result from synchronous layout causing unintentional adjustments. It would be good to experiment with this at the least.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#update-the-rendering