1608995 - Scroll anchoring: Toggling block display on child of inline element can set a bad anchor node

Status: NEW

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

Description

[Nick Fagerlund]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:73.0) Gecko/20100101 Firefox/73.0

Steps to reproduce:

Test case is available at http://www.sedumphotos.net/nfagerlund/testcases/re-anchoring.html. The crucial bits of the test case are

1: There's an element nested inside a "display: inline" element.
2: Some JS changes that child element's display property from "none" to "block", or vice-versa.
3: There's some amount of other random stuff above and below the elements in question.

STEPS:

• In about:config, set "layout.css.scroll-anchoring.highlight" to true, so you can see changes to the anchor node.
• In the test case linked above, scroll to the bottom.
• Note that the anchor node is visible via the purple highlight, and is correctly set to a paragraph near the top of the viewpoint.
• Click the button labeled "this is the cut tag thing".
• Note that the height of the content in the scrollable container does not change.

Actual results:

The anchor node is incorrectly changed to the "big mood" element below the button, which is near the bottom of the viewport.

Expected results:

Either the anchor node should not change, or the recalculation should choose a valid candidate element near the top of the viewport (i.e. one of the paragraphs), like it normally does when the scroll position changes.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

This is a valid bug, but it's pretty hard to fix. The issue is that changing display from / to block inside an inline creates an IB-split (see data:text/html,<span style="border: 1px solid">Foo <div>Bar</div> Baz</span>).

We generally don't deal terribly well with dynamic changes inside an IB split, so when they happen we reconstruct the whole containing block. Chrome deals with it in a more efficient but sometimes-incorrect way...

Part of the issue is that scroll anchoring, by definition, uses the dirty style and layout information to select the anchor (as you want to detect the shift caused by layout). In this case the layout information for all the elements in the reconstructed containing block is gone, and the relevant frames have a 0x0 rect (so we think it's outside of the viewport and select the next thing, the footer).

So TLDR, this is unfortunately somewhat hard to fix. Possible solutions:

• Deal better with IB split changes. It's possible that for simple cases like this we can just fix up the frame tree in place instead of rebuilding it. That'd allow the old layout information to persist.
• Somehow detect reframes and avoid throwing away the anchor when it gets reframed... This seems somewhat exploratory, but potentially doable...

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Now in general, IB splits are horrible and I hope they died ;)

Comment 4

[Nick Fagerlund]

Yeah, my practical conclusion was "lol, guess we'd better not do that, then." But I figured I'd report the bug anyway. :)

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Yeah, that is a nice test-case and description, thank you so much for that!

People create a ton of silly IB-splits unintentionally, and mostly without borders / backgrounds, so it's ~non-observable... But all layout engines need to do something terribly complicated to support this instead... :-)