Daniil Sakhapov (Gerrit)

unread,

Jan 29, 2026, 7:06:39 AMJan 29

to Helmut Januschka, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Helmut Januschka

Daniil Sakhapov added 3 comments

File third_party/blink/renderer/core/css/element_rule_collector.cc

Line 855, Patchset 3 (Latest):

      (pseudo_style_request_.pseudo_id == kPseudoIdPlaceholder ||
       pseudo_style_request_.pseudo_id == kPseudoIdFileSelectorButton)) {

Daniil Sakhapov . unresolved

maybe also extract this into a separate function?

File third_party/blink/renderer/core/css/selector_checker.cc

Line 3199, Patchset 3 (Latest):

      // For getComputedStyle, match the pseudo-element selector even if
      // the actual shadow element doesn't exist on this element type.
      if (context.for_computed_style &&
          context.pseudo_id == kPseudoIdFileSelectorButton) {
        result.dynamic_pseudo = kPseudoIdFileSelectorButton;
        return true;
      }

Daniil Sakhapov . unresolved

I wonder if we can pull this pattern (from kPseudoFileSelectorButton and kPseudoPlaceholder) into some function and check before this switch? And just add new such pseudo-elements there if needed in the future?

Line 3653, Patchset 3 (Latest): result.dynamic_pseudo = context.pseudo_id;

Daniil Sakhapov . unresolved

I wonder if it did work before without setting dynamic_pseudo? Does it break something if we don't add it here? (just wondering, as this part of the code hasn't been really finished)

I suspect that DynamicPseudoExtractor is doing this dynamic_pseudo line here instead.

Open in Gerrit

Related details

Attention is currently required from:

Helmut Januschka

Submit Requirements:

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

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Feb 1, 2026, 11:24:04 AMFeb 1

to Helmut Januschka, Chromium LUCI CQ, Daniil Sakhapov, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov

Helmut Januschka added 3 comments

File third_party/blink/renderer/core/css/element_rule_collector.cc

Line 855, Patchset 3:

      (pseudo_style_request_.pseudo_id == kPseudoIdPlaceholder ||
       pseudo_style_request_.pseudo_id == kPseudoIdFileSelectorButton)) {

Daniil Sakhapov . resolved

maybe also extract this into a separate function?

Helmut Januschka

Done

File third_party/blink/renderer/core/css/selector_checker.cc

Line 3199, Patchset 3: // For getComputedStyle, match the pseudo-element selector even if


      // the actual shadow element doesn't exist on this element type.
      if (context.for_computed_style &&
          context.pseudo_id == kPseudoIdFileSelectorButton) {
        result.dynamic_pseudo = kPseudoIdFileSelectorButton;
        return true;
      }

Daniil Sakhapov . resolved

I wonder if we can pull this pattern (from kPseudoFileSelectorButton and kPseudoPlaceholder) into some function and check before this switch? And just add new such pseudo-elements there if needed in the future?

Helmut Januschka

Done

Line 3653, Patchset 3: result.dynamic_pseudo = context.pseudo_id;

Daniil Sakhapov . unresolved

I wonder if it did work before without setting dynamic_pseudo? Does it break something if we don't add it here? (just wondering, as this part of the code hasn't been really finished)
I suspect that DynamicPseudoExtractor is doing this dynamic_pseudo line here instead.

Helmut Januschka

I tried dropping the dynamic_pseudo assignment here, but it’s safer to keep it setting dynamic_pseudo ensures ElementRuleCollector treats the match as a pseudo-element match rather than attributing it to the originating element.

Even if DynamicPseudoExtractor sets it in other flows, we still need it here for consistency.

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov

Submit Requirements:

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

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Daniil Sakhapov (Gerrit)

unread,

Feb 2, 2026, 7:14:28 AMFeb 2

to Helmut Januschka, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Helmut Januschka

Daniil Sakhapov added 1 comment

File third_party/blink/renderer/core/css/selector_checker.cc

Line 3653, Patchset 3: result.dynamic_pseudo = context.pseudo_id;

Daniil Sakhapov . unresolved

I wonder if it did work before without setting dynamic_pseudo? Does it break something if we don't add it here? (just wondering, as this part of the code hasn't been really finished)
I suspect that DynamicPseudoExtractor is doing this dynamic_pseudo line here instead.

Helmut Januschka

I tried dropping the dynamic_pseudo assignment here, but it’s safer to keep it setting dynamic_pseudo ensures ElementRuleCollector treats the match as a pseudo-element match rather than attributing it to the originating element.
Even if DynamicPseudoExtractor sets it in other flows, we still need it here for consistency.

Daniil Sakhapov

CheckVirtualPseudo has been implemented for a new way to match pseudo-elements, it really doesn't need setting dynamic_pseudo here, as the matching order for selector is changed, so it can even be bad to set it here tbh.

Open in Gerrit

Related details

Attention is currently required from:

Helmut Januschka

Submit Requirements:

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

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Feb 3, 2026, 5:56:42 PMFeb 3

to Helmut Januschka, Chromium LUCI CQ, Daniil Sakhapov, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov

Helmut Januschka added 1 comment

File third_party/blink/renderer/core/css/selector_checker.cc

Line 3653, Patchset 3: result.dynamic_pseudo = context.pseudo_id;

Daniil Sakhapov . resolved

I wonder if it did work before without setting dynamic_pseudo? Does it break something if we don't add it here? (just wondering, as this part of the code hasn't been really finished)
I suspect that DynamicPseudoExtractor is doing this dynamic_pseudo line here instead.

Helmut Januschka

I tried dropping the dynamic_pseudo assignment here, but it’s safer to keep it setting dynamic_pseudo ensures ElementRuleCollector treats the match as a pseudo-element match rather than attributing it to the originating element.
Even if DynamicPseudoExtractor sets it in other flows, we still need it here for consistency.

Daniil Sakhapov

CheckVirtualPseudo has been implemented for a new way to match pseudo-elements, it really doesn't need setting dynamic_pseudo here, as the matching order for selector is changed, so it can even be bad to set it here tbh.

Helmut Januschka

removed it, still works! thanks for pushing back! must have been some other change that lead me to the believe tests fail with the dynamic_pseudo force!

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Daniil Sakhapov (Gerrit)

unread,

Feb 4, 2026, 5:39:32 AMFeb 4

to Helmut Januschka, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Helmut Januschka

Daniil Sakhapov added 3 comments

File third_party/blink/renderer/core/css/element_rule_collector.cc

Line 889, Patchset 10 (Latest): if (CollectRequestedPseudoRulesForComputedStyle<stop_at_first_match>(

Daniil Sakhapov . unresolved

nit: having this return `true` only for stop_at_first_match is a bit confusing, don't you think? Like if `false` means that nothing has been collected?

When I asked to pull this part into a separate function I meant only pseudo_id if, so that we can add new such pseudo-elements just to that function.

Maybe keep the for loop here as you did originally and just have that if function?

File third_party/blink/renderer/core/css/selector_checker.cc

Line 3653, Patchset 3: result.dynamic_pseudo = context.pseudo_id;

Daniil Sakhapov . resolved

I wonder if it did work before without setting dynamic_pseudo? Does it break something if we don't add it here? (just wondering, as this part of the code hasn't been really finished)
I suspect that DynamicPseudoExtractor is doing this dynamic_pseudo line here instead.

Helmut Januschka

I tried dropping the dynamic_pseudo assignment here, but it’s safer to keep it setting dynamic_pseudo ensures ElementRuleCollector treats the match as a pseudo-element match rather than attributing it to the originating element.
Even if DynamicPseudoExtractor sets it in other flows, we still need it here for consistency.

Daniil Sakhapov

CheckVirtualPseudo has been implemented for a new way to match pseudo-elements, it really doesn't need setting dynamic_pseudo here, as the matching order for selector is changed, so it can even be bad to set it here tbh.

Helmut Januschka

removed it, still works! thanks for pushing back! must have been some other change that lead me to the believe tests fail with the dynamic_pseudo force!

Daniil Sakhapov

no problem! but that flag is disabled now, so I wonder if we need to create a virtual test to also cover this cide path?
though Gerrit says it's covered by tests...

Line 3664, Patchset 10 (Latest):

          if (context.pseudo_id ==
              selector.GetPseudoId(selector.GetPseudoType())) {
            return true;
          }
          return false;

Daniil Sakhapov . unresolved

nit: leave as it used to be? (single return)

Open in Gerrit

Related details

Attention is currently required from:

Helmut Januschka

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Feb 4, 2026, 4:55:09 PMFeb 4

to Helmut Januschka, Chromium LUCI CQ, Daniil Sakhapov, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov

Helmut Januschka added 3 comments

File third_party/blink/renderer/core/css/element_rule_collector.cc

Line 889, Patchset 10 (Latest): if (CollectRequestedPseudoRulesForComputedStyle<stop_at_first_match>(

Daniil Sakhapov . resolved

nit: having this return `true` only for stop_at_first_match is a bit confusing, don't you think? Like if `false` means that nothing has been collected?
When I asked to pull this part into a separate function I meant only pseudo_id if, so that we can add new such pseudo-elements just to that function.
Maybe keep the for loop here as you did originally and just have that if function?

Helmut Januschka

Done. extracted the pseudo_id check into a helper and made the stop_at_first_match return explicit while keeping the loop here.

File third_party/blink/renderer/core/css/selector_checker.cc

Line 3653, Patchset 3: result.dynamic_pseudo = context.pseudo_id;

Daniil Sakhapov . resolved

I wonder if it did work before without setting dynamic_pseudo? Does it break something if we don't add it here? (just wondering, as this part of the code hasn't been really finished)
I suspect that DynamicPseudoExtractor is doing this dynamic_pseudo line here instead.

Helmut Januschka

I tried dropping the dynamic_pseudo assignment here, but it’s safer to keep it setting dynamic_pseudo ensures ElementRuleCollector treats the match as a pseudo-element match rather than attributing it to the originating element.
Even if DynamicPseudoExtractor sets it in other flows, we still need it here for consistency.

Daniil Sakhapov

CheckVirtualPseudo has been implemented for a new way to match pseudo-elements, it really doesn't need setting dynamic_pseudo here, as the matching order for selector is changed, so it can even be bad to set it here tbh.

Helmut Januschka

removed it, still works! thanks for pushing back! must have been some other change that lead me to the believe tests fail with the dynamic_pseudo force!

Daniil Sakhapov

no problem! but that flag is disabled now, so I wonder if we need to create a virtual test to also cover this cide path?
though Gerrit says it's covered by tests...

Helmut Januschka

Good point. I added a virtualsuite to cover this code path and confirmed it passes with CSSLogicalCombinationPseudo enabled.

While testing, figured out i still need `result.dynamic_pseudo = context.pseudo_id` in `CheckVirtualPseudo` without it, the virtual test fails (computed ::before styles do not apply).

Line 3664, Patchset 10 (Latest):

          if (context.pseudo_id ==
              selector.GetPseudoId(selector.GetPseudoType())) {
            return true;
          }
          return false;

Daniil Sakhapov . resolved

nit: leave as it used to be? (single return)

Helmut Januschka

Done

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Daniil Sakhapov (Gerrit)

unread,

Feb 6, 2026, 7:31:39 AMFeb 6

to Helmut Januschka, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Daniil Sakhapov voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Mar 4, 2026, 4:03:42 PM (7 days ago) Mar 4

to Helmut Januschka, Daniil Sakhapov, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov and Helmut Januschka

Message from Helmut Januschka

Set Ready For Review

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov
Helmut Januschka

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Jonathan Lee (Gerrit)

unread,

Mar 4, 2026, 4:42:08 PM (7 days ago) Mar 4

to Helmut Januschka, Daniil Sakhapov, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov and Helmut Januschka

Jonathan Lee voted and added 1 comment

Votes added by Jonathan Lee

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Jonathan Lee . resolved

VTS LGTM

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov
Helmut Januschka

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Mar 4, 2026, 5:24:59 PM (7 days ago) Mar 4

to Helmut Januschka, Jonathan Lee, Daniil Sakhapov, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov

Helmut Januschka voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Jonathan Lee (Gerrit)

unread,

Mar 6, 2026, 9:58:49 PM (5 days ago) Mar 6

to Helmut Januschka, Daniil Sakhapov, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov and Helmut Januschka

Jonathan Lee voted and added 1 comment

Votes added by Jonathan Lee

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 12:

Jonathan Lee . resolved

VTS LGTM

Jonathan Lee

VTS still LGTM, but please wait for @sakh...@chromium.org to sign off on the new expectations

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov
Helmut Januschka

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

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

Gerrit

satisfied_requirement

open

diffy

Daniil Sakhapov (Gerrit)

unread,

Mar 9, 2026, 6:37:42 AM (2 days ago) Mar 9

to Helmut Januschka, Jonathan Lee, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Helmut Januschka

Daniil Sakhapov added 1 comment

File third_party/blink/web_tests/virtual/cssom-getcomputedstyle-pseudo-no-colon-disabled/external/wpt/css/cssom/getComputedStyle-pseudo-expected.txt

Line 4, Patchset 14 (Latest):

[FAIL] Prefixed ::-webkit-file-upload-button does not resolve in getComputedStyle
  assert_equals: ::-webkit-file-upload-button should return empty style expected 0 but got 483

Daniil Sakhapov . unresolved

why this thing started to resolve even?

Open in Gerrit

Related details

Attention is currently required from:

Helmut Januschka

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Helmut Januschka (Gerrit)

unread,

Mar 9, 2026, 7:46:58 AM (2 days ago) Mar 9

to Helmut Januschka, Jonathan Lee, Daniil Sakhapov, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Daniil Sakhapov and Jonathan Lee

Helmut Januschka added 2 comments

File third_party/blink/renderer/core/css/parser/css_selector_parser.cc

File third_party/blink/web_tests/virtual/cssom-getcomputedstyle-pseudo-no-colon-disabled/external/wpt/css/cssom/getComputedStyle-pseudo-expected.txt

Line 4, Patchset 14:[FAIL] Prefixed ::-webkit-file-upload-button does not resolve in getComputedStyle


  assert_equals: ::-webkit-file-upload-button should return empty style expected 0 but got 483

Daniil Sakhapov . resolved

why this thing started to resolve even?

Helmut Januschka

the linux platform baseline (deleted in this CL) was masking it; fixed by returning kPseudoIdInvalid for unrecognized -webkit-* pseudos in ParsePseudoElement.

Open in Gerrit

Related details

Attention is currently required from:

Daniil Sakhapov
Jonathan Lee

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

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

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Match ::placeholder and ::file-selector-button for getComputedStyle on any element [chromium/src : main]

Daniil Sakhapov (Gerrit)

Daniil Sakhapov added 3 comments

Related details

Helmut Januschka (Gerrit)

Helmut Januschka added 3 comments

Related details

Daniil Sakhapov (Gerrit)

Daniil Sakhapov added 1 comment

Related details

Helmut Januschka (Gerrit)

Helmut Januschka added 1 comment

Related details

Daniil Sakhapov (Gerrit)

Daniil Sakhapov added 3 comments

Related details

Helmut Januschka (Gerrit)

Helmut Januschka added 3 comments

Related details

Daniil Sakhapov (Gerrit)

Daniil Sakhapov voted Code-Review+1

Related details

Helmut Januschka (Gerrit)

Message from Helmut Januschka

Related details

Jonathan Lee (Gerrit)

Jonathan Lee voted and added 1 comment

Votes added by Jonathan Lee

1 comment

Related details

Helmut Januschka (Gerrit)

Helmut Januschka voted Commit-Queue+2

Related details

Jonathan Lee (Gerrit)

Jonathan Lee voted and added 1 comment

Votes added by Jonathan Lee

1 comment

Related details

Daniil Sakhapov (Gerrit)

Daniil Sakhapov added 1 comment

Related details

Helmut Januschka (Gerrit)

Helmut Januschka added 2 comments

Related details