RESOLVED FIXED 178237
Support ::before and ::after pseudo elements after ::slotted
https://bugs.webkit.org/show_bug.cgi?id=178237
Summary Support ::before and ::after pseudo elements after ::slotted
Rob Dodson
Reported 2017-10-12 15:13:29 PDT
As discussed in https://github.com/w3c/webcomponents/issues/655 you should be able to use pseudo-element selectors after ::slotted()
Attachments
Patch (41.16 KB, patch)
2021-10-27 09:49 PDT, Antti Koivisto
no flags
Patch (41.24 KB, patch)
2021-10-27 11:36 PDT, Antti Koivisto
no flags
Patch (41.67 KB, patch)
2021-10-27 23:59 PDT, Antti Koivisto
no flags
diegocardoso
Comment 1 2020-04-27 02:57:58 PDT
Just checked that this issue is still opened. Linking the code to reproduce it taken from the one of the webcomponents issue: https://jsfiddle.net/webpadawan/v3ksqhcx/
Stephen Belovarich
Comment 2 2021-03-09 21:46:25 PST
This issue blocks my ability as a library author to create a custom element called radio-group that supports slotted <input type="radio"> and style those radio buttons from the context of the radio-group.
Antti Koivisto
Comment 3 2021-10-27 09:49:55 PDT
Antti Koivisto
Comment 4 2021-10-27 11:36:23 PDT
Emilio Cobos Álvarez (:emilio)
Comment 5 2021-10-27 17:14:09 PDT
Comment on attachment 442615 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442615&action=review Looks sensible to me. non-reviewer r+ I guess :-) > Source/WebCore/css/SelectorChecker.cpp:431 > + slot = slot->assignedSlot(); Do you need to bail out if `scopeDepth` is not one after the loop? > Source/WebCore/css/SelectorChecker.cpp:1171 > + if (auto* subselector = context.selector->selectorList()->first()) { Hmm, can this really be null? > Source/WebCore/css/parser/CSSSelectorParser.cpp:347 > + // FIXME: A WPT indicates ::is/::where should be parsed but reduce to nothing as their content is not legal in the context. nit: Only one colon before :is/:where > Source/WebCore/css/parser/CSSSelectorParser.cpp:372 > + case CSSSelector::PseudoElementMarker: Gecko also includes ::placeholder and ::file-selector-button, fwiw: https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/servo/components/style/gecko/pseudo_element.rs#40-41 > Source/WebCore/style/ElementRuleCollector.cpp:-352 > - ASSERT(is<HTMLSlotElement>(element())); It's nice to see this code gone fwiw. > Source/WebCore/style/ElementRuleCollector.cpp:445 > auto* selectorForMatching = selector; You might want to remove this temporary and just use `selector` everywhere?
Antti Koivisto
Comment 6 2021-10-27 21:56:27 PDT
Comment on attachment 442615 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442615&action=review >> Source/WebCore/css/SelectorChecker.cpp:431 >> + slot = slot->assignedSlot(); > > Do you need to bail out if `scopeDepth` is not one after the loop? (or 0). Yeah, probably. >> Source/WebCore/css/SelectorChecker.cpp:1171 >> + if (auto* subselector = context.selector->selectorList()->first()) { > > Hmm, can this really be null? Not really, should have failed parsing. >> Source/WebCore/css/parser/CSSSelectorParser.cpp:347 >> + // FIXME: A WPT indicates ::is/::where should be parsed but reduce to nothing as their content is not legal in the context. > > nit: Only one colon before :is/:where wonder if anyone gets these consistently right >> Source/WebCore/css/parser/CSSSelectorParser.cpp:372 >> + case CSSSelector::PseudoElementMarker: > > Gecko also includes ::placeholder and ::file-selector-button, fwiw: https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/servo/components/style/gecko/pseudo_element.rs#40-41 We don't have dedicated enum values for those for some reason. I'll add a comment and figure out what to do with them separately. >> Source/WebCore/style/ElementRuleCollector.cpp:445 >> auto* selectorForMatching = selector; > > You might want to remove this temporary and just use `selector` everywhere? true > LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/slotted-parsing-expected.txt:23 > +FAIL "::slotted(*):is()" should be a valid selector The string did not match the expected pattern. > +FAIL "::slotted(*):is(:hover)" should be a valid selector The string did not match the expected pattern. > +FAIL "::slotted(*):is(#id)" should be a valid selector The string did not match the expected pattern. > +FAIL "::slotted(*):where()" should be a valid selector The string did not match the expected pattern. > +FAIL "::slotted(*):where(:hover)" should be a valid selector The string did not match the expected pattern. > +FAIL "::slotted(*):where(#id)" should be a valid selector The string did not match the expected pattern. Do you happen to know what is the logic/spec text behind these?
Antti Koivisto
Comment 7 2021-10-27 23:59:16 PDT
Ryosuke Niwa
Comment 8 2021-10-28 00:50:07 PDT
Comment on attachment 442680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442680&action=review > Source/WebCore/style/ElementRuleCollector.cpp:-350 > -std::unique_ptr<RuleSet::RuleDataVector> ElementRuleCollector::collectSlottedPseudoElementRulesForSlot() Nice! > Source/WebCore/style/RuleFeature.cpp:91 > + // FIXME: Implement accurate invalidation. > + return matchElement; Do we have a test for this? Can we add revalidation tests since we've had a bunch of style revalidation bugs in this area in the past?
Alan Davalos
Comment 9 2021-10-28 01:22:12 PDT
Just a note on the side, I think this other issue: ( https://bugs.webkit.org/show_bug.cgi?id=223814 ) is a duplicate of sorts of this one, you might want to close it once the work here is done
Antti Koivisto
Comment 10 2021-10-28 01:50:06 PDT
> Do we have a test for this? > Can we add revalidation tests since we've had a bunch of style revalidation > bugs in this area in the past? There is a related invalidation WPT that we are failing. I'll look into this in that context.
EWS
Comment 11 2021-10-28 01:59:10 PDT
Committed r284973 (243623@main): <https://commits.webkit.org/243623@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 442680 [details].
Radar WebKit Bug Importer
Comment 12 2021-10-28 02:00:25 PDT
Emilio Cobos Álvarez (:emilio)
Comment 13 2021-10-28 04:03:10 PDT
(In reply to Antti Koivisto from comment #6) > > LayoutTests/imported/w3c/web-platform-tests/css/css-scoping/slotted-parsing-expected.txt:23 > > +FAIL "::slotted(*):is()" should be a valid selector The string did not match the expected pattern. > > +FAIL "::slotted(*):is(:hover)" should be a valid selector The string did not match the expected pattern. > > +FAIL "::slotted(*):is(#id)" should be a valid selector The string did not match the expected pattern. > > +FAIL "::slotted(*):where()" should be a valid selector The string did not match the expected pattern. > > +FAIL "::slotted(*):where(:hover)" should be a valid selector The string did not match the expected pattern. > > +FAIL "::slotted(*):where(#id)" should be a valid selector The string did not match the expected pattern. > > Do you happen to know what is the logic/spec text behind these? Hmm, so I changed that test in https://github.com/web-platform-tests/wpt/commit/6f085e0f00f64f40b6066edef20344eac8211ab4 because after other pseudos where pseudo-classes are allowed it makes sense to allow :where(). That said, spec-wise I guess it's not clear, I filed https://github.com/w3c/csswg-drafts/issues/5093 a while ago, and there seems to be consensus about that behavior... bug 212049 is relevant here. For ::slotted(), where other pseudo-classes after it aren't allowed anywhere, I doubt it matters much either way.
Note You need to log in before you can comment on or make changes to this bug.