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

[selectors-4] Disallow pseudo-elements inside :has() #7463

Closed
fantasai opened this issue Jul 6, 2022 · 23 comments
Closed

[selectors-4] Disallow pseudo-elements inside :has() #7463

fantasai opened this issue Jul 6, 2022 · 23 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. selectors-4 Current Work

Comments

@fantasai
Copy link
Collaborator

fantasai commented Jul 6, 2022

Selectors like ol:has(li::marker) and section:has(p::first-line) are problematic and we should make all pseudo-elements invalid inside :has().

@jakearchibald
Copy link
Contributor

jakearchibald commented Jul 7, 2022

In #7346 I use the example:

::page-transition(id):not(:has(:>> incoming-image)) :>> outgoing-image {
  /* … */
}

…to mean "select the ::outgoing-images in ::page-transitions that don't have an ::incoming-image".

I guess the problem with ::marker and ::first-line is that their existence and content can be defined by CSS, creating a loop.

Is there a way around this? Maybe some pseudo elements can be marked as "CSS-dynamic" cannot be used in :has, but others can?

@tabatkins
Copy link
Member

Yeah, we left open in the discussion the possibility that we could relax the restriction when needed. If the "when" is "immediately", that's fine. ^_^ So yeah, defining a category of pseudos that can be used here is the way to go - pseudos which, while they may or may not depend on CSS properties for their original creation, do not depend on CSS for their continued existence, so any styles you set won't affect their lifetime.

Question: should the "always exists" pseudo-elements like ::before/after qualify? I lean toward "no" since they're not useful to detect.

@Loirooriol
Copy link
Contributor

Implementations (Blink & WebKit) already disallow pseudo-elements (but :has() is forgiving):

var s = document.createElement("style");
s.textContent = ":has(foo, ::before, bar) { color: blue }";
document.documentElement.append(s);
s.sheet.cssRules[0].selectorText; // ':has(foo, bar)'

@tabatkins
Copy link
Member

That's as good an argument as any for "no". ^_^

@jakearchibald
Copy link
Contributor

Question: should the "always exists" pseudo-elements like ::before/after qualify? I lean toward "no" since they're not useful to detect.

"It's always there, so it should behave as if it's not there" seems a bit odd.

@tabatkins
Copy link
Member

Not "behave as if it's not there", but rather "make the selector invalid", same as the other pseudos that we don't allow.

@jakearchibald
Copy link
Contributor

It isn't useful to do :root:is(:scope) either, but it still matches, because the query :is(:scope) is true.

:has(::before) is similarly true, so it should behave as if it is true.

@khushalsagar
Copy link
Member

:has(::before) probably won't be useful since the answer is always the same but returning true seems more correct.

The pseudo-elements being introduced for page-transition would satisfy this constraint, "pseudos which, while they may or may not depend on CSS properties for their original creation, do not depend on CSS for their continued existence, so any styles you set won't affect their lifetime."

Their creation is dependent on CSS (page-transition-tag) but once the elements are created their existence can not be toggled using CSS for the duration of a transition. There is some subtlety in that tearing down the whole setup relies on checking if these pseudo-elements have an active animation, which itself can be controlled by CSS. But once we detect that there are no active animations, the pseudo-elements are torn down irrespective of if the style changes to add an animation again.

I think it makes sense to add an exception for page-transition pseudo-elements once that has been spec'd.

Are there any existing pseudo elements which would fit in this category, to motivate introducing this concept. ::before/::after don't seem as motivating since they'll always have the same answer.

@khushalsagar
Copy link
Member

I'm also curious to understand how we define that a pseudo-element "exists" for it to satisfy the :has check. For ::before/::after, it looks like Blink conditionally creates nodes in the DOM tree for them if their content property is not "none" (and they would generate a box). But from spec perspective they "always exist". For example, getComputedStyle for it returns the correct computed value. But the same is not true for "::marker".

So for a pseudo-element to always exist, is the idea that they can be targeted in script as-if they have a DOM node?

@khushalsagar
Copy link
Member

But the same is not true for "::marker"

Actually that's not true. Jake and I were using opacity to check if getComputedStyle is correct but opacity on ::marker doesn't apply whether it generates a box or not. color works and is computed correctly irrespective of whether ::marker is generating a box. So I'm not following how ::marker is different from ::before/::after with respect to an "always exist" category.

@Loirooriol
Copy link
Contributor

We have a resolution in #1793 (comment)

marker exists on all elements, on ::before and on ::after but no box unless it's display: list-item

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed disallowing pseudos in :has(), and agreed to the following:

  • RESOLVED: Disallow all current pseudo-elements inside of :has(), allow future pseudo-elements to define that they are valid if useful/possible.
The full IRC log of that discussion <TabAtkins> Topic: disallowing pseudos in :has()
<TabAtkins> github: https://github.com//issues/7463
<TabAtkins> fantasai: Proposal was to make selector invalid if a pseudo-element is inside of :has()
<emilio> +1
<fantasai> TabAtkins: object to that as a general thing, because the reason we were doing that was because of conditional pseudo-elements (conditional on propeties)
<fantasai> TabAtkins: but shared element transitions has use case for checking existance of pseudos
<fantasai> TabAtkins: much more awkward API if we don't have ability
<fantasai> TabAtkins: outside of that, I think we should make a list of pseudo-elements that are allowed, and disallow the rest
<fantasai> TabAtkins: so the shared element transition ones, and just because they don't have an effect (because they exist at all times), ::marker/before/after
<fantasai> TabAtkins: but I can go either way on that
<fantasai> TabAtkins: wouldn't do anything
<fantasai> TabAtkins: but need a list for those that don't create cycles
<TabAtkins> fantasai: Prefer to make things invalid rather than valid but meaningless
<TabAtkins> fantasai: If we need to have exception for SET pseudos, fine, but since most pseudos won't make sense we shoudl disallow all others.
<TabAtkins> fantasai: Can alway smake them valid later, less likely to cause problems that way
<astearns> ack dbaron
<TabAtkins> dbaron: I agree with "list of the ones taht work"
<TabAtkins> dbaron: Skeptical about putting before/after/marker on that list.
<TabAtkins> dbaron: Not sure the statement they always exist makes sense.
<TabAtkins> dbaron: Do before and after exist on replaced elements? Does marker exist on non-list items?
<TabAtkins> dbaron: I think the spec says yes to that last one, which I find confusing.
<bramus> `::backdrop` might also be a handy one to allow, could unblock `:top-layer` as that would become `:has(::backdrop)`
<fantasai> bramus, that's just weird
<TabAtkins> TabAtkins: Fine with just disallowing before/after/marker
<faceless> Aren't they all defined to not exist on items with display:none?
<dbaron> s/I think the spec says yes/Not sure what the spec says/
<TabAtkins> astearns: Do we have the allowlist or blocklist?
<TabAtkins> fantasai: Proposal is an allowlist, assume invalid otherwise
<TabAtkins> astearns: What's the allowlist?
<TabAtkins> fantasai: The SET pseudos
<TabAtkins> astearns: what about bramus' suggestion about ::backdrop?
<TabAtkins> fantasai: I think it's a little weird. Not really needed, should do that as a :top-level instead.
<vmpstr> ntim:
<TabAtkins> ???: "Does ::backdrop exist?" is also not clear
<TabAtkins> ntim: Still think it's weird if the state can change
<TabAtkins> astearns: So seems the proposed resolution is that all current pseudo-elements in :has() are invalid, but allow for future pseudo-elements to define that they work
<chris> That sounds good to me
<TabAtkins> ntim: What does "invalid" mean?
<TabAtkins> TabAtkins: Means "invalid" - the selector inside of :has() is dropped.
<TabAtkins> astearns: Objections?
<TabAtkins> RESOLVED: Disallow all current pseudo-elements inside of :has(), allow future pseudo-elements to define that they are valid if useful/possible.
<chris> q+

@tabatkins tabatkins added Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. labels Aug 24, 2022
@Loirooriol
Copy link
Contributor

Note that if e.g. ::foo were defined to be allowed in :has(), :has(::foo) would be like :has(*::foo), i.e. it would match if some descendant of the subject has a ::foo (whatever that would mean). It wouldn't check the subject element at all.

I think this defeats the possibility of "allowing future pseudo-elements to define that they are valid" until we have a :> combinator (#7346).

@tabatkins
Copy link
Member

No, it just means you can't check for the pseudo-element on the subject element. It doesn't prevent the syntax from being useful still, it's just only usable for checking them on descendant elements.

@Loirooriol
Copy link
Contributor

It's just hard for me to image why anybody would want that instead of checking the pseudo-element on the subject element.

@tabatkins
Copy link
Member

Why would anyone want to check anything on a descendant element? It's just as valid as anything else.

But importantly, the behavior I described is what falls out of the syntax as it exists. Doing anything else would be a funky reinterpretation of the syntax, and I'd like to avoid doing that to avoid any weird knock-on effects.

@Loirooriol
Copy link
Contributor

:: is combinator-like so I think it wouldn't be that unreasonable to treat it different in relative selectors as I initially assumed, but fine, it's not a combinator, I'm not pushing for a change. Just wanted to note that :has(::foo) may not behave as one would expect, and checking for pseudo-elements in :has() will have a severe limitation until :> is added.

@AyubM
Copy link

AyubM commented Feb 8, 2023

Is there a plan on when certain pseudo elements will be allowed? I think the referenced webkit article does a good job of explaining how form pseudo classes can be useful with :has.

Otherwise, without those it seems the feature is pretty neutered and might hinder adoption. I for one, after checking my site (which uses :has(input:checked)) and finding them not to work in Firefox, will have to revert to using Javascript.

@Loirooriol
Copy link
Contributor

:checked is a pseudo-class, not a pseudo-element. Your problem is that Firefox hasn't shipped :has().

@SebastianZ
Copy link
Contributor

Your problem is that Firefox hasn't shipped :has().

And if you want to follow Mozilla's implementation of :has(), see https://bugzil.la/418039.

Sebastian

@faceless2
Copy link

faceless2 commented Feb 13, 2023

Now that :has() is unforgiving we need to make a distinction whether pseudo-elements inside are invalid or just don't match. The current text is

Also, unless explicitly defined as a :has-allowed pseudo-element, pseudo-elements are not valid selectors within :has(). (This specification does not define any :has-allowed pseudo-elements, but other specifications may do so.)

If they're not valid that invalidates the whole selector, presumably at parse time. But that's not the current behaviour of Webkit or Chrome, and that doesn't match the relative-selector-list syntax either.

I think this text should be changed from "are not valid selectors" to "do not match"

EDIT: for a non-forgiving selector, it also feels a bit odd that invalid pseudo-elements are allowed: div, :has(::badpseudo) is a valid selector, but div, ::badpseudo is not.

@faceless2
Copy link

Ignore my last comment - "not valid within has()" is unusual but clear enough and matches the impementations.

@tabatkins
Copy link
Member

Right, there are a number of additional conditions on validity that aren't expressed within the grammar itself (such as "must be a recognized pseudo-class name", "namespaces must already be declared", etc), and this is one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. selectors-4 Current Work
Projects
None yet
Development

No branches or pull requests

9 participants