1988032 - Assign the ASR (active scrolled root) of the anchor to the anchored element

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[David Shin[:dshin]]

See Bug 1923392 Comment 15

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Assign the ASR of the anchor to the anchored content. r?#layout-reviewers,botond,hiro

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Add some basic anchor pos plus async scroll reftests. r?hiro

Comment 3

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Add a pref to control async scroll of CSS anchor positioned content. r?#layout-reviewers

This gives an easy way to disable in case of issues and allows toggling for testing purposes (to narrow down if a potential issue is async or main thread scrolling related).

Comment 4

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Don't async scroll with anchors if there is an active view transition. r?#layout-reviewers

Being inside an active view transition forces a scroll frame to be inactive.

https://searchfox.org/firefox-main/rev/70425199e9a5fa80aa7419fa51763013a67226e1/layout/generic/ScrollContainerFrame.cpp#4349

IsInViewTransitionCapture is set on a stacking context basis. This means that when painting walks from a placeholder frame to its out of flow then IsInViewTransitionCapture follows. But the ASR tree (which we must construct for async scrolling the anchor pos) strictly follows the parent (not the placeholder). So we'd have to do a completely separate walk, and I'm not sure it's worth it. If it turns out to be important we can implement something, or even better just remove that code in DecideScrollableLayer which I'm pretty sure we don't need but I don't want to open that can of worms right now.

Comment 5

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Disable RDL if are there active CSS anchor pos. r?#layout-reviewers

Anchored content can get assigned an ASR that is "far away" from it with no relation in the frame tree. So if something about the ASR changes we need to invalidate the anchored content but that is not straight forward, and not worth it until/unless we find it causing an issue.

Comment 6

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Disable RDL if are there active CSS anchor pos. r?#layout-reviewers

Anchored content can get assigned an ASR that is "far away" from it with no relation in the frame tree. So if something about the ASR changes we need to invalidate the anchored content but that is not straight forward, and not worth it until/unless we find it causing an issue.

Comment 7

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Tweak ASR assert to handle unusual native anonymous content in the custom content container in view transition case. r?emilio,#layout-reviewers

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cb45424f429d
https://hg.mozilla.org/integration/autoland/rev/f8ad4916eec4
Assign the ASR of the anchor to the anchored content. r=hiro,layout-reviewers,layout-anchor-positioning-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/7165d46e3b24
https://hg.mozilla.org/integration/autoland/rev/9c6896a99939
Add some basic anchor pos plus async scroll reftests. r=hiro,layout-reviewers
https://github.com/mozilla-firefox/firefox/commit/98d14e2dc819
https://hg.mozilla.org/integration/autoland/rev/dec66d84e74c
Add a pref to control async scroll of CSS anchor positioned content. r=layout-reviewers,hiro
https://github.com/mozilla-firefox/firefox/commit/472901d51347
https://hg.mozilla.org/integration/autoland/rev/aa8819fe8168
Don't async scroll with anchors if there is an active view transition. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/9063b3c208f7
https://hg.mozilla.org/integration/autoland/rev/b8ee0d9bc9d4
Disable RDL if are there active CSS anchor pos. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/8244ecad8b53
https://hg.mozilla.org/integration/autoland/rev/9daaf6d08950
Tweak ASR assert to handle unusual native anonymous content in the custom content container in view transition case. r=emilio,layout-reviewers

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b75b0abc10ea
https://hg.mozilla.org/integration/autoland/rev/6108ececd044
Revert (Bug 1988032, Bug 1988030) for causing build bustages @ DisplayPortUtils.cpp

Comment 10

[amarc]

Backed out for causing build bustages

• Backout link
• Push with failures
• Failure Log

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/56298 for changes under testing/web-platform/tests

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/94b9e3475381
https://hg.mozilla.org/integration/autoland/rev/ae3b4e95cd36
Assign the ASR of the anchor to the anchored content. r=hiro,layout-reviewers,layout-anchor-positioning-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/0aec81c009fd
https://hg.mozilla.org/integration/autoland/rev/74ae9f86faab
Add some basic anchor pos plus async scroll reftests. r=hiro,layout-reviewers
https://github.com/mozilla-firefox/firefox/commit/f24cbbb7a2d1
https://hg.mozilla.org/integration/autoland/rev/1c2d8a47374e
Add a pref to control async scroll of CSS anchor positioned content. r=layout-reviewers,hiro
https://github.com/mozilla-firefox/firefox/commit/66d347cb0dba
https://hg.mozilla.org/integration/autoland/rev/bc2eb1b66aae
Don't async scroll with anchors if there is an active view transition. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/ebad30b3999d
https://hg.mozilla.org/integration/autoland/rev/cfd58f087220
Disable RDL if are there active CSS anchor pos. r=layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/5f4e0ab4bf4d
https://hg.mozilla.org/integration/autoland/rev/b2284f1c94ea
Tweak ASR assert to handle unusual native anonymous content in the custom content container in view transition case. r=emilio,layout-reviewers

Comment 13

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1988032. Add fuzzy annotations to two wpt anchor tests. r?#layout-reviewers,#gfx-reviewers

It's a single pixel just at the edge of a lower case letter t (two instances in each test).

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e506d5c226cc
https://hg.mozilla.org/integration/autoland/rev/227774938f7b
Add fuzzy annotations to two wpt anchor tests. r#layout-reviewers,#gfx-reviewers

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ae3b4e95cd36
https://hg.mozilla.org/mozilla-central/rev/74ae9f86faab
https://hg.mozilla.org/mozilla-central/rev/1c2d8a47374e
https://hg.mozilla.org/mozilla-central/rev/bc2eb1b66aae
https://hg.mozilla.org/mozilla-central/rev/cfd58f087220
https://hg.mozilla.org/mozilla-central/rev/b2284f1c94ea
https://hg.mozilla.org/mozilla-central/rev/227774938f7b

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot