Rune Lillesveen (Gerrit)

unread,

Sep 4, 2025, 9:41:39 AM (5 days ago) Sep 4

to Rune Lillesveen, Mason Freed, Anders Hartvoll Ruud, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Anders Hartvoll Ruud and Mason Freed

Rune Lillesveen added 1 comment

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Rune Lillesveen . resolved

ptal

masonf@: I'm unsure how much impact this change will have. We should probably have an I2S for this. If you agree, I'll make a chromestatus entry as well. Also interested in your take on making the implicit-anchor counter a sticky flag.

Open in Gerrit

Related details

Attention is currently required from:

Anders Hartvoll Ruud
Mason Freed

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Mason Freed (Gerrit)

unread,

Sep 4, 2025, 5:58:08 PM (5 days ago) Sep 4

to Rune Lillesveen, Anders Hartvoll Ruud, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Anders Hartvoll Ruud and Rune Lillesveen

Mason Freed voted and added 7 comments

Votes added by Mason Freed

Code-Review

+1

7 comments

Patchset-level comments

File-level comment, Patchset 7 (Latest):

Mason Freed . resolved

LGTM - just small nits.

File third_party/blink/renderer/core/css/resolver/style_resolver_test.cc

Line 4126, Patchset 7 (Latest): auto set_sheet_text = [this](HTMLElement* style_element,

Mason Freed . unresolved

nit: you could just capture `style_element` since it's the same for all cases.

File third_party/blink/renderer/core/dom/element.h

Line 1751, Patchset 7 (Latest): bool IsImplicitAnchor() const;

Mason Freed . unresolved

Given the new logic, should this perhaps be `MayBeImplicitAnchor`?

Line 1750, Patchset 7 (Latest): // Returns true if any element is implicitly anchored to this element.

Mason Freed . unresolved

Maybe add a comment that there's no way to unset this, on purpose?

File third_party/blink/renderer/platform/runtime_enabled_features.json5

Line 3397, Patchset 7 (Latest): // This is a spec change since anchor positioning was shipped in Blink.

Mason Freed . unresolved

Would be nice to have a bug or spec issue link

File third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-function-pseudo-element-implicit-anchor-expected.txt

Line 2, Patchset 7 (Parent):[FAIL] The implicit anchor element of a pseudo-element is its originating element

Mason Freed . unresolved

This sounds like it's an existing test that is testing the new spec? If so, should this test be `.tentative`?

File third_party/blink/web_tests/external/wpt/css/css-anchor-position/pseudo-element-implicit-anchor-center.html

File-level comment, Patchset 7 (Latest):

Mason Freed . unresolved

Ditto - should this be `.tentative`?

Open in Gerrit

Related details

Attention is currently required from:

Anders Hartvoll Ruud
Rune Lillesveen

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Anders Hartvoll Ruud (Gerrit)

unread,

Sep 5, 2025, 4:15:31 AM (5 days ago) Sep 5

to Rune Lillesveen, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Rune Lillesveen

Anders Hartvoll Ruud voted and added 2 comments

Votes added by Anders Hartvoll Ruud

Code-Review

+1

2 comments

Commit Message

Line 20, Patchset 7 (Latest):

difficult and error prone to do for pseudo elements. Instead use a
single flag that will never be reset once set. The assumption here is

Anders Hartvoll Ruud . unresolved

And we're sure that nothing bad happens if an element continues to believe it's an implicit anchor, while it's actually not (anymore)?

File third_party/blink/renderer/core/dom/element.h

Line 1751, Patchset 7 (Latest): bool IsImplicitAnchor() const;

Mason Freed . unresolved

Given the new logic, should this perhaps be `MayBeImplicitAnchor`?

Anders Hartvoll Ruud

+1

Open in Gerrit

Related details

Attention is currently required from:

Rune Lillesveen

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rune Lillesveen (Gerrit)

unread,

Sep 8, 2025, 4:13:02 AM (yesterday) Sep 8

to Rune Lillesveen, Anders Hartvoll Ruud, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Anders Hartvoll Ruud and Mason Freed

Rune Lillesveen added 7 comments

Commit Message

Line 20, Patchset 7:difficult and error prone to do for pseudo elements. Instead use a


single flag that will never be reset once set. The assumption here is

Anders Hartvoll Ruud . resolved

And we're sure that nothing bad happens if an element continues to believe it's an implicit anchor, while it's actually not (anymore)?

Rune Lillesveen

It's an indication that the layout code needs to propagate it as an anchor in case it is used as one.

The actual implicit anchor lookup here is not affected by the flag:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=12459-12491;drc=577a7699f159e7f307bde09c247ba502d376929d;bpv=1;bpt=1

The consequence for the flag not being reset, is that if a lot of elements are temporarily being made implicit anchors and then reset again, we would have a performance degradation in that we will have to do a lot of unnecessary accounting and propagation for anchors during layout. I don't see that as a likely issue.

File third_party/blink/renderer/core/css/resolver/style_resolver_test.cc

Line 4126, Patchset 7: auto set_sheet_text = [this](HTMLElement* style_element,

Mason Freed . resolved

nit: you could just capture `style_element` since it's the same for all cases.

Rune Lillesveen

Done

File third_party/blink/renderer/core/dom/element.h

Line 1751, Patchset 7: bool IsImplicitAnchor() const;

Mason Freed . resolved

Given the new logic, should this perhaps be `MayBeImplicitAnchor`?

Anders Hartvoll Ruud

+1

Rune Lillesveen

Done

Line 1750, Patchset 7: // Returns true if any element is implicitly anchored to this element.

Mason Freed . resolved

Maybe add a comment that there's no way to unset this, on purpose?

Rune Lillesveen

Done

File third_party/blink/renderer/platform/runtime_enabled_features.json5

Line 3397, Patchset 7: // This is a spec change since anchor positioning was shipped in Blink.

Mason Freed . resolved

Would be nice to have a bug or spec issue link

Rune Lillesveen

Done

File third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-function-pseudo-element-implicit-anchor-expected.txt

Line 2, Patchset 7 (Parent):[FAIL] The implicit anchor element of a pseudo-element is its originating element

Mason Freed . resolved

This sounds like it's an existing test that is testing the new spec? If so, should this test be `.tentative`?

Rune Lillesveen

This matches the current version of the spec, so it shouldn't be tentative.

File third_party/blink/web_tests/external/wpt/css/css-anchor-position/pseudo-element-implicit-anchor-center.html

File-level comment, Patchset 7:

Mason Freed . resolved

Ditto - should this be `.tentative`?

Rune Lillesveen

No, this also matches the current spec.

Open in Gerrit

Related details

Attention is currently required from:

Anders Hartvoll Ruud
Mason Freed

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rune Lillesveen (Gerrit)

unread,

Sep 8, 2025, 4:15:14 AM (yesterday) Sep 8

to Rune Lillesveen, Anders Hartvoll Ruud, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Anders Hartvoll Ruud and Mason Freed

Rune Lillesveen added 1 comment

File third_party/blink/renderer/core/layout/physical_fragment.h

Line 452, Patchset 8 (Latest): bool IsImplicitAnchor() const;

Rune Lillesveen . unresolved

I didn't do the renaming to "MayBe" here. Wasn't clear that was the right choice here.

Open in Gerrit

Related details

Attention is currently required from:

Anders Hartvoll Ruud
Mason Freed

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rune Lillesveen (Gerrit)

unread,

Sep 8, 2025, 5:42:47 AM (yesterday) Sep 8

to Rune Lillesveen, Chromium LUCI CQ, Anders Hartvoll Ruud, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Anders Hartvoll Ruud and Mason Freed

Rune Lillesveen added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Rune Lillesveen . resolved

I lost +1s. Needs another pass.

Open in Gerrit

Related details

Attention is currently required from:

Anders Hartvoll Ruud
Mason Freed

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Anders Hartvoll Ruud (Gerrit)

unread,

Sep 8, 2025, 6:04:35 AM (yesterday) Sep 8

to Rune Lillesveen, Chromium LUCI CQ, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Mason Freed and Rune Lillesveen

Anders Hartvoll Ruud voted and added 1 comment

Votes added by Anders Hartvoll Ruud

Code-Review

+1

1 comment

File third_party/blink/renderer/core/layout/physical_fragment.h

Line 452, Patchset 8: bool IsImplicitAnchor() const;

Rune Lillesveen . resolved

I didn't do the renaming to "MayBe" here. Wasn't clear that was the right choice here.

Anders Hartvoll Ruud

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

Mason Freed
Rune Lillesveen

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Rune Lillesveen (Gerrit)

unread,

Sep 8, 2025, 6:16:09 AM (yesterday) Sep 8

to Rune Lillesveen, Anders Hartvoll Ruud, Chromium LUCI CQ, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Attention needed from Mason Freed

Rune Lillesveen voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Mason Freed

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Sep 8, 2025, 6:20:02 AM (yesterday) Sep 8

to Rune Lillesveen, Anders Hartvoll Ruud, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Originating element is implicit anchor for pseudo elements

Implementation in Blink instead used the implicit anchor for the
originating element. This is a spec change in css-anchor-position-1
after Chrome shipped.

We are marking the originating element as an implicit anchor based on
the pseudo element styles to make sure they are considered as anchors
during layout.

Previously, we used a counter that was incremented/decremented for
implicit anchor, which had to be correctly balanced. This is error prone
and has lead to bugs before, and would have been increasingly more


difficult and error prone to do for pseudo elements. Instead use a
single flag that will never be reset once set. The assumption here is

that we will not make a lot of elements temporarily implicit anchors,
continuously growing the amount of anchor candidates, that would have
eventually lead to performance issues.

A use counter for pseudo elements referencing its originating element
as an implicit anchor is added to measure the impact of the changed
behavior.

Bug: 408223892

Change-Id: I37c4ce03e8d7e85248fb86afbdce3daf1d257280

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6827741

Reviewed-by: Anders Hartvoll Ruud <and...@chromium.org>

Commit-Queue: Rune Lillesveen <fut...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1512296}

Files:

M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
M third_party/blink/renderer/core/css/resolver/style_resolver_test.cc
M third_party/blink/renderer/core/dom/element.cc
M third_party/blink/renderer/core/dom/element.h
M third_party/blink/renderer/core/dom/element_rare_data_vector.cc
M third_party/blink/renderer/core/dom/element_rare_data_vector.h
M third_party/blink/renderer/core/html/anchor_element_observer.cc
M third_party/blink/renderer/core/html/anchor_element_observer.h
M third_party/blink/renderer/core/html/forms/select_type.cc
M third_party/blink/renderer/core/html/forms/select_type.h
M third_party/blink/renderer/core/html/html_element.cc
M third_party/blink/renderer/core/html/html_element_test.cc
M third_party/blink/renderer/core/layout/layout_inline.cc
M third_party/blink/renderer/core/layout/layout_object.cc
M third_party/blink/renderer/core/layout/physical_fragment.cc
M third_party/blink/renderer/core/layout/physical_fragment.h
M third_party/blink/renderer/core/style/computed_style.h
M third_party/blink/renderer/platform/runtime_enabled_features.json5
M third_party/blink/web_tests/TestExpectations
D third_party/blink/web_tests/external/wpt/css/css-anchor-position/anchor-function-pseudo-element-implicit-anchor-expected.txt
A third_party/blink/web_tests/external/wpt/css/css-anchor-position/pseudo-element-implicit-anchor-center.html
M tools/metrics/histograms/metadata/blink/enums.xml

Change size: L

Delta: 22 files changed, 212 insertions(+), 121 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Anders Hartvoll Ruud

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Blink W3C Test Autoroller (Gerrit)

unread,

Sep 8, 2025, 7:12:18 AM (yesterday) Sep 8

to Rune Lillesveen, Chromium LUCI CQ, Anders Hartvoll Ruud, Mason Freed, Alexis Menard, Chromium Metrics Reviews, AyeAye, chromium...@chromium.org, blink-re...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, apavlo...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org

Message from Blink W3C Test Autoroller

The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/54744

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Originating element is implicit anchor for pseudo elements [chromium/src : main]

Rune Lillesveen (Gerrit)

Rune Lillesveen added 1 comment

Related details

Mason Freed (Gerrit)

Mason Freed voted and added 7 comments

Votes added by Mason Freed

7 comments

Related details

Anders Hartvoll Ruud (Gerrit)

Anders Hartvoll Ruud voted and added 2 comments

Votes added by Anders Hartvoll Ruud

2 comments

Related details

Rune Lillesveen (Gerrit)

Rune Lillesveen added 7 comments

Related details

Rune Lillesveen (Gerrit)

Rune Lillesveen added 1 comment

Related details

Rune Lillesveen (Gerrit)

Rune Lillesveen added 1 comment

Related details

Anders Hartvoll Ruud (Gerrit)

Anders Hartvoll Ruud voted and added 1 comment

Votes added by Anders Hartvoll Ruud

1 comment

Related details

Rune Lillesveen (Gerrit)

Rune Lillesveen voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Blink W3C Test Autoroller (Gerrit)

Message from Blink W3C Test Autoroller

Related details