Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-shadow-parts] need definition of which pseudo-classes can follow ::part() that is sufficient to produce interoperability #10787

Open
dbaron opened this issue Aug 27, 2024 · 14 comments

Comments

@dbaron
Copy link
Member

dbaron commented Aug 27, 2024

The spec for ::part() says:

The ::part() pseudo-element can take additional pseudo-classes after it, such as x-button::part(label):hover, but never matches the structural pseudo-classes or any other pseudo-classes that match based on tree information rather than local element information.

This wording probably isn't precise enough to lead all implementors to classify every pseudo-class (works after ::part() or not) in the same way. I just added a test to WPT that exercises a bunch of these cases. (See also #10786 for a separate issue that relates to the same test. And also see #9795 for a case where people previously disagreed about this classification.)

We probably need a clearer definition of which selectors do or don't work here, and it probably needs to somehow end up in the way that we define pseudo-classes.

@dbaron
Copy link
Member Author

dbaron commented Aug 27, 2024

Perhaps a special case: I think it should probably be clear that :has() isn't allowed; it's probably not testable today but it could be in the future if we introduce something equivalent to what was allowed in early :has() drafts by :has(:scope:hover) (which is roughly equivalent to :is(:hover)).

@dbaron dbaron changed the title need definition of which pseudo-classes can follow :part() that is sufficient to produce interoperability [css-shadow-parts] need definition of which pseudo-classes can follow :part() that is sufficient to produce interoperability Aug 27, 2024
@tabatkins
Copy link
Member

tabatkins commented Sep 5, 2024

Agreed.

I think the definitely-disallowed ones are:

  • the structural pseudo-classes, as already disallowed
  • :has()
  • :scope
  • :host, :host(), and :host-context()

Slightly uncertain ones that I think should still be allowed:

  • :lang() and :dir() (we already resolved to allow them)
  • the :*-within pseudos (:target-within, :focus-within
  • :picture-in-picture

And I think the rest are all clearly fine.

I made this list by looking over https://drafts.csswg.org/indexes/#selectors which gets selectors from across the spec landscape, so should be pretty complete (anything it's missing are just mis-tagged).

@dbaron
Copy link
Member Author

dbaron commented Sep 5, 2024

I think also probably disallowed (as structural) should be :scope, :host-context() and :host.

@tabatkins
Copy link
Member

I'd considered them, but left them out as I figured they could never match. But you're right, for clarity they should be disallowed explicitly.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2024
Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
aarongable pushed a commit to chromium/chromium that referenced this issue Sep 6, 2024
Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351997}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2024
Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351997}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2024
Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351997}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 15, 2024
…sses after ::part()., a=testonly

Automatic update from web-platform-tests
Allow :active-view-transition pseudo-classes after ::part().

Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351997}

--

wpt-commits: a75813a805d96b9ef5236a2b31fe877967dc881a
wpt-pr: 48013
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 16, 2024
…sses after ::part()., a=testonly

Automatic update from web-platform-tests
Allow :active-view-transition pseudo-classes after ::part().

Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Commit-Queue: David Baron <dbaronchromium.org>
Cr-Commit-Position: refs/heads/main{#1351997}

--

wpt-commits: a75813a805d96b9ef5236a2b31fe877967dc881a
wpt-pr: 48013

UltraBlame original commit: c38fc2e2446120b0a73631d83718dc055830e074
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 16, 2024
…sses after ::part()., a=testonly

Automatic update from web-platform-tests
Allow :active-view-transition pseudo-classes after ::part().

Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Commit-Queue: David Baron <dbaronchromium.org>
Cr-Commit-Position: refs/heads/main{#1351997}

--

wpt-commits: a75813a805d96b9ef5236a2b31fe877967dc881a
wpt-pr: 48013

UltraBlame original commit: c38fc2e2446120b0a73631d83718dc055830e074
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 16, 2024
…sses after ::part()., a=testonly

Automatic update from web-platform-tests
Allow :active-view-transition pseudo-classes after ::part().

Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Commit-Queue: David Baron <dbaronchromium.org>
Cr-Commit-Position: refs/heads/main{#1351997}

--

wpt-commits: a75813a805d96b9ef5236a2b31fe877967dc881a
wpt-pr: 48013

UltraBlame original commit: c38fc2e2446120b0a73631d83718dc055830e074
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Sep 16, 2024
…sses after ::part()., a=testonly

Automatic update from web-platform-tests
Allow :active-view-transition pseudo-classes after ::part().

Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351997}

--

wpt-commits: a75813a805d96b9ef5236a2b31fe877967dc881a
wpt-pr: 48013
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Sep 17, 2024
…sses after ::part()., a=testonly

Automatic update from web-platform-tests
Allow :active-view-transition pseudo-classes after ::part().

Based on the discussion in
w3c/csswg-drafts#10806 and also in
w3c/csswg-drafts#10787, I think the conclusion
from #10806 applies to pseudo-classes as well, and I should reverse my
earlier decision to disallow the :active-view-transition pseudo-classes
after ::part(), even though today it is not possible for them to match.

This makes that change behind the CSSPartAllowsMoreSelectorsAfter flag
(status:experimental), which also contains other related changes.

This matches WebKit's current implementation of those pseudo-classes.

Bug: 40623497
Change-Id: I591fe30bb9e6c3a0953df318919f60dd6962d4a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5839421
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351997}

--

wpt-commits: a75813a805d96b9ef5236a2b31fe877967dc881a
wpt-pr: 48013
@dbaron
Copy link
Member Author

dbaron commented Sep 22, 2024

I made this list by looking over https://drafts.csswg.org/indexes/#selectors which gets selectors from across the spec landscape, so should be pretty complete (anything it's missing are just mis-tagged).

I checked the list in the index against Chrome's implementation. The only pseudo-elements and pseudo-classes I found that aren't in the list, aren't prefixed, and don't work only on something else that's prefixed (like ::-webkit-scrollbar) are:

I don't think there are any mistagged specs in the CSS WG for any of these, though maybe :xr-overlay should show up in the index.

@schenney-chromium
Copy link
Contributor

I checked the list in the index against Chrome's implementation. The only pseudo-elements and pseudo-classes I found that aren't in the list, aren't prefixed, and don't work only on something else that's prefixed (like ::-webkit-scrollbar) are:

...

This one is in the prototype phase in Chrome but I would very much expect it to behave the same as ::selection and other highlights. Maybe runtime enabled status affects the behavior?

@dbaron
Copy link
Member Author

dbaron commented Sep 23, 2024

After going over my test again and checking the index that tab used (above), I support Tab's conclusions in #10787 (comment).

I think it would probably be good to resolve on that list and add the details of it to the spec, while also documenting the principles that led to it so that we can hopefully classify future pseudo-classes correctly, and hopefully also defining terms that future specs can hook into (and maybe even using that as the mechanism for defining the current list).

@tabatkins
Copy link
Member

:window-inactive (shipping)

Oh, MDN just documents this as a Moz extension only. If it's unprefixed and shipping it needs to show up in a spec.

Anyway, yeah, it should obviously match, it doesn't expose anything.

:xr-overlay (shipping)

This was indeed mistagged; Bikeshed doesn't infer CSS definitions from the text by default (the option is turned on in the CSSWG). I've submitted a PR to fix it.

This seems fine to match too, it's element state.

:granted (not enabled yet)

Same, this is element state, it should match.

@tabatkins
Copy link
Member

For the pseudo-elements, I agree with Schenney that ::search-text should be treated identical to ::selection. Do we already have a term covering that category of things? If not, we should add one as it's probably useful to be able to refer to them as a group.

For the carousel pseudos, I think they should be supported. They're morally equivalent to ::before/::after for this purpose.

@schenney-chromium
Copy link
Contributor

::search-text and ::selection are part of the Highlight Pseudos family. Also includes ::highlight(...), ::target-text, ::spelling-error and ::grammar-error.

@LeaVerou LeaVerou changed the title [css-shadow-parts] need definition of which pseudo-classes can follow :part() that is sufficient to produce interoperability [css-shadow-parts] need definition of which pseudo-classes can follow ::part() that is sufficient to produce interoperability Sep 23, 2024
@LeaVerou
Copy link
Member

Shouldn’t :state() be one of these pseudo-classes?

@tabatkins
Copy link
Member

Yeah, it's also mismarked and thus didn't show up in the list. Just discussed this with WHATWG folks, they'd prefer Selectors actually be the "definition" of :state(), and just defer to HTML for behavior, like we do with many other host-lang-specific things.

But yes, it should absolutely be allowed.

@tabatkins
Copy link
Member

Ah, so :state() was actually marked up just fine, it just lives in Selectors 5. It definitely shows up in the index, tho, so it fell under the "everything else is clearly allowed" category I gave.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-shadow-parts] need definition of which pseudo-classes can follow `::part()` that is sufficient to produce interoperability, and agreed to the following:

  • RESOLVED: disallow :has() :scope :host, :host(), and :host-context()
The full IRC log of that discussion <dholbert> dbaron: this is now going into the details of the rules for which pseudo-classes are invalid or valid after ::not
<dholbert> dbaron: this is important because we're introducing concept of part-like pseudo-elements
<dholbert> dbaron: this set of rules is now not only applying to part but also to other things
<dholbert> dbaron: the spec says the part pseudo element can take pseudo-classes like :hover...tree information... local element information [quoting spec]
<dholbert> dbaron: this is a reasonable design guideline for authors, but it's not specific enough to produce interoperability
<dholbert> dbaron: some tests revealed differences between implementations on this, RE whether something is tree information vs. local-element information
<dholbert> dbaron: what I want to do is change the spec so that it's explicitly listing which pseudo-classes are & are not allowed
<dholbert> dbaron: still giving design principle behind that, but making it more explicit
<dbaron> https://github.com//issues/10787#issuecomment-2332414419
<dholbert> dbaron: proposal is what's in Tab's comment from 3 weeks ago^
<dholbert> dbaron: which is basically, the things that we'll disallow the things the spec defines as structural pseudo-classes - first-child, nth-*, etc
<dholbert> dbaron: you can't use :has, it's asking about descendants
<dholbert> dbaron: you can't use :scope, :host, :host-context
<dholbert> dbaron: but all the other current pseudo-classes would be allowed. We'd be allowing :lang and :dir (previuos resolution)
<dholbert> dbaron: we'd be allowing :focus-within and :target-within, and :picture-in-picture
<emilio> q+
<dholbert> dbaron: those are the ones Tab thought were somewhat ambiguous but probably allow
<dholbert> dbaron: that's the proposal unless people want to change it
<dholbert> emilio: I agree on making it more explicit
<Rossen6> ack emilio
<dholbert> emilio: I think there are some tests that are invalid, e.g. the last one uses :is ...
<dholbert> emilio: I think some of the tests that you landed may need tweaks. I think current behavior from WebKit and Gecko is correct, on :is:first-child etc
<dholbert> emilio: because it was resolved that we'd preserve the inner element serialization [?]
<dholbert> dbaron: there are some tests that Chromium fails due to serialization
<dholbert> emilio: we can sort this out async. generally I support this
<dholbert> s/this/the test failures/
<dholbert> Rossen6: did you get a chance to check WHATWG discussions to be sure nothing needs to change
<dholbert> dbaron: I read through the more recent discussions. I don't think any of that would change this list
<dholbert> PROPOSED RESOLUTION: Tab's comment from 3 weeks ago https://github.com//issues/10787#issuecomment-2332414419
<dholbert> PROPOSED RESOLUTION: disallow
<dholbert> :has()
<dholbert> :scope
<dholbert> :host, :host(), and :host-context()
<fantasai> +1
<dholbert> RESOLVED: disallow :has() :scope :host, :host(), and :host-context()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Friday afternoon
Development

No branches or pull requests

6 participants