[go: nahoru, domu]

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-nesting-1] Wrapping parent selectors with :is() introduces a host of problems #9492

Open
LeaVerou opened this issue Oct 18, 2023 · 32 comments
Labels
css-nesting-1 Current Work

Comments

@LeaVerou
Copy link
Member
LeaVerou commented Oct 18, 2023

Opening per action item in today’s telcon ( #8738 (comment) )

Problem

We currently desugar every nested rule using :is(), even when it’s completely redundant. E.g. this:

.foo, #bar {
	& {
		color: red;
	}
}

apparently becomes:

:is(.foo, #bar) {
	color: red;
}

This is not only redundant, it actually has observable side effects: since :is() has a flat specificity equal to the specificity of its argument with the highest specificity, the second rule is not equivalent to the first.

But even worse, because :is() cannot do pseudo-elements, it means this doesn’t work at all:

.foo::before {
	padding: 1em;
	@media (width < 500px) {
		padding: .5em;
	}
}

It would be rewritten like:

.foo::before {
	padding: 1em;
}

@media (width < 500px) {
	:is(.foo::before) {
		padding: .5em;
	}
}

which is invalid.

It also means we cannot fix #8738 by wrapping bare declarations after rules in & {} because in pseudo-elements they could be ignored.

Proposed solution

Edit: I'm not in favor of this any more, see #9492 (comment)

& {} should have no observable side effects. There is never a case where & needs :is() to be rewritten, so adding it not only introduces the problems discussed above, but is entirely redundant.

So a quick fix to the pressing issues could be to simply special case &.

Later down the line, we may want to consider expanding the set of cases that don’t require :is() because there is no danger of combinatorial explosion. From a quick call with @fantasai it seems to us that e.g. cases where there are only simple selectors all the way up would not need :is(), but more research is needed.

@LeaVerou LeaVerou added the css-nesting-1 Current Work label Oct 18, 2023
@Loirooriol
Copy link
Contributor

I just want to note that & is not actually replaced by :is(), it just behaves like it.

The actual definition of & in https://drafts.csswg.org/css-nesting/#nest-selector

the nesting selector represents the elements matched by the parent rule

I guess this could become

the nesting selector represents the elements or pseudo-elements matched by the parent rule

I'm not convinced that the specificity needs to be changed.

Treating a & alone different than when used in a more complex selector seems extremely hacky and a source of confusion to me.

@andruud
Copy link
Member
andruud commented Oct 18, 2023

So a quick fix to the pressing issues could be to simply special case &.

Honest question: what's the pressing issue? Pseudo-elements don't currently work, but it seems fine to have a restriction that we can lift later? I hope we can avoid panic/"quick fixes", and instead accept that this doesn't work until pseudo-elements actually work within :is().

I'm not convinced that the specificity needs to be changed.

Right, if we want to change that, it seems more pressing. It might be too late already.

Treating a & alone different than when used in a more complex selector seems extremely hacky and a source of confusion to me.

+1

Maybe we can imply a new thing around bare declarations instead of &, that behaves as if the parent rule's selector list was used directly (unwrapped), and that also serializes as that selector list?

@LeaVerou
Copy link
Member Author
LeaVerou commented Oct 19, 2023

@Loirooriol

I'm not convinced that the specificity needs to be changed.

I don’t think anybody is arguing for that — probably way too late due to web compat, and also unclear how it could be changed. There’s a reason :is() flattens specificity.

@andruud

So a quick fix to the pressing issues could be to simply special case &.

Honest question: what's the pressing issue? Pseudo-elements don't currently work, but it seems fine to have a restriction that we can lift later?

#8738 is the pressing issue, not pseudo-elements. See resolution & minutes from yesterday. The thing is, #8738 cannot be addressed without fixing the pseudo-element case (see minutes for details).

Treating a & alone different than when used in a more complex selector seems extremely hacky and a source of confusion to me.

+1

Maybe special casing & is hacky, but not wrapping hierarchies of simple selectors seems like a direct improvement, akin to e.g. not using parens when parens are not needed when generating expressions. :is() are basically the parens of CSS selectors.

@andruud
Copy link
Member
andruud commented Oct 20, 2023

OK. Hopefully it's not already too late to carry out #8738.

Maybe special casing & is hacky, but not wrapping hierarchies of simple selectors seems like a direct improvement, akin to e.g. not using parens when parens are not needed when generating expressions. :is() are basically the parens of CSS selectors.

Not only an improvement, it makes &'s behavior inconsistent. Removing parens that do nothing isn't the same as removing &/:is() which does do something.

We should wrap affected declarations in something which isn't &, if we don't want the behavior of &.

@tabatkins
Copy link
Member

Yes, the :is()-like behavior is necessary to avoid inconsistent behavior. &, &.foo, & .foo, and .foo & should all be identical wrt specificity (well, the first will have one class less, obvs). We absolutely should not try to do anything to "fix" this in some of the cases but not others.

The pseudo-element restriction is annoying, but there's already an issue tracking that (#7433).

@tabatkins
Copy link
Member

OK. Hopefully it's not already too late to carry out #8738.

For it to be "too late" it would require authors to (a) be writing decls after blocks, and (b) be depending on the "shift up" behavior differences (specificity and pseudo-elements). I suspect that's extremely unlikely.

@tabatkins
Copy link
Member

Actually, let me make this clearer: I will formally object to treating & {...} differently from & .foo {...}.

Happy to work on the issues via some other mechanism (like Anders says, wrapping them in a different kind of rule instead - let's re-re-revive @nest!), but I'm absolutely not ok with having different behavior based on the exact selector used.

Do we want to repurpose this thread to discussing how to solve the "naked property nesting inherits some of Nesting's weaknesses" issue, or open a fresh issue?

@LeaVerou
Copy link
Member Author
LeaVerou commented Oct 25, 2023

@tabatkins

Yes, the :is()-like behavior is necessary to avoid inconsistent behavior. &, &.foo, & .foo, and .foo & should all be identical wrt specificity (well, the first will have one class less, obvs). We absolutely should not try to do anything to "fix" this in some of the cases but not others.

The way :is() flattens specificity is a wart that was necessary because a pseudo-class needs to have a defined specificity, it cannot depend on what is matched. :is() is necessary for nesting to avoid combinatorial explosion, but I don't see why we should introduce this wart any more than is necessary (and where it is necessary can be statically determined at parse time).

The pseudo-element restriction is annoying, but there's already an issue tracking that (#7433).

That issue is closed. If we want to have an issue for tracking this, it should not be a closed issue.

OK. Hopefully it's not already too late to carry out #8738.

For it to be "too late" it would require authors to (a) be writing decls after blocks, and (b) be depending on the "shift up" behavior differences (specificity and pseudo-elements). I suspect that's extremely unlikely.

As was discussed in the telcon, authors absolutely do write declarations after blocks, though depending on the shifting up behavior is way more rare. FWIW I agree it's not too late to change this.

Actually, let me make this clearer: I will formally object to treating & {...} differently from & .foo {...}.

It may help to explain your reasoning rather than just vetoing things.

FWIW I was seriously considering formally objecting to keeping the shifting up behavior (the closest I ever got to an FO in my 11 years in the WG), but in the end discussion was far more productive and an FO was not necessary. 😊

Happy to work on the issues via some other mechanism (like Anders says, wrapping them in a different kind of rule instead - let's re-re-revive @nest!), but I'm absolutely not ok with having different behavior based on the exact selector used.

Do we want to repurpose this thread to discussing how to solve the "naked property nesting inherits some of Nesting's weaknesses" issue, or open a fresh issue?

I think it's more important to establish why these weaknesses are necessary in the first place, and explore ways to get rid of them before we decide to add even more syntax to CSS to do similar but slightly different things (which is an antipattern from a UX/DX perspective).

@FremyCompany
Copy link
Contributor
FremyCompany commented Oct 25, 2023

I think the issue mentioned on the call (and some people didn't really understand) is that:

article, .article {
    color: red; 
    & * { color: green; }
}

maps to

article, .article { color: red; }
:is(article, .article) * { color: green; }

which has a different specificity from

article, .article { color: red }
article *, .article * { color: green }

and may authors (including me) would have expected the latter one, not the first one

but, as Tab mentioned, this rewriting might cause extremely large lists of selectors in the case of deeply nested selectors, so this might end up expensive.

@FremyCompany
Copy link
Contributor
FremyCompany commented Oct 25, 2023

And just to keep adding to the summary, I think the "explode by specificity" that @LeaVerou proposed would work along these lines:

article, .article, .advert {
    color: red;
    @mixin something;
    color: green;
}

can be rewritten

article, .article {
    color: red;
    @mixin something;
    :where(&):is(article), :where(&):is(.article, .advert) { color: green; }
}

Note that, even though it might be undesirable in the general case (no strong opinion on this), it's possible to do this rewriting only in the case of "promoted" declarations where wrapping & {} would result in wrong specificity for the declaration after promotion.

I would personally be fine with something like this being done only for the rare case of out-of-order declarations. But this begs the question whether this should be the default indeed. I think implementors should give it some thought. It might not be as difficult as it looks like.

If we do it for everything, then simply nesting into a & {} block becomes fine again for out-of-order declaration promotion.

@Loirooriol
Copy link
Contributor

The problem is basically the same as in #1027

#a1, .a2, a3 {
& #b1, & .b2, & b3 {
& #c1, & .c2, & c3 {
& #d1, & .d2, & d3 {
& #e1, & .e2, & e3 {
& #f1, & .f2, & f3 {

Very short, but there are 729 possible different ways to match this. If the specificity can vary, then it has a performance impact, e.g. knowing that the element matches .a2 .b2 .c2 .d2 .e2 .f2 is not enough, because maybe it also matches a3 #b1 .c2 d3 .e2 f3 which has a higher specificity. So checking all/several possible combinations is needed.

I still don't see how the specificity concern is related to #8738. And while I can see it might be confusing, it doesn't seem inconsistent nor anything. As I said in #9492 (comment), I don't see the need to change the current specificity.

@FremyCompany Your idea would imply a combinatorial explosion?

@tabatkins
Copy link
Member

It may help to explain your reasoning rather than just vetoing things.

Yes, I did explain my reasoning, in an immediately preceding comment, and this topic was discussed in older Nesting discussions already so I wasn't sure of how much I should repeat. I posted my veto as a separate comment, rather than just editting my previous ones, to make sure it was expressed in the email record as well. Let me be more explicit, tho.

As expressed in the call again, by me and Oriol, we cannot solve the general case of making specificity match that of fully-unnested rules, because that entails an exponential explosion of possible specificities.

For example, given:

div, .div, #div {
 div, .div, #div {
  div, .div, #div {
   div, .div, #div { ... }
  }
 }
}

There are already 3^4 = 81 unique specificities that the innermost properties could potentially have, depending on what precisely they matched. Adding another level would triple it again, to 243; adding a fourth selector to each level would similarly roughly triple it to 4^4 = 256.

There's no workaround here. An author writing out a selector list with 256 options can easily see there might be a performance issue with that. An author writing four nested rules, each relatively short, might not be aware of that at all. And it's only a short step away from millions or paths, or more.

In the call, you suggested we could do some intelligent coalescing of specificities at each level; if the author writes .foo, .bar, .baz, ... it doesn't matter how long the list is, every option's specificity is [0, 1, 0]. (@FremyCompany also just suggested this, above.) This doesn't solve the general case, however; at best it can slow down the exponential growth, but not prevent it, and the example I wrote above wouldn't be helped by it at all, since every option has a different specificity anyway. So this is similarly unworkable; it's still easy for authors to exceed the compute budget, accidentally or intentionally.


However, while the general case is unsolveable, the specific case of .foo, #bar { @media (...) { color: red; } } wanting to have the same specificity as if it were nested the other way around is solveable.

As I said, I'd object to treating a rule with a lone & differently; this sort of context-sensitivity is confusing for authors and means that seemingly-innocent changes like adding an additional selector to a list can change the meaning of other selectors they didn't touch.

But adding a new construct that explicitly handles this case is fine. I alluded to this above, so let me give the proposal more explicitly:

Add a new @nest rule. It has no prelude, and allows nested properties inside of it. It matches exactly the same elements and/or pseudo-elements as the parent rule, and applies its nested properties to those elements with the same specificity as the parent rule. Use @nest as the wrapping rule for properties after at-rules, or for naked properties in conditional rules (rather than an & {...} rule, as currently specified).

Examples:

div, .foo, #bar::before {
	color: blue;
	@media (...) { color: red; }
	color: green;
}

/* Today, desugars to: */

div, .foo, #bar::before {
	color: blue;
	color: green;
}
@media (...) {
	:is(div, .foo) { color: red; } 
	/* different specificity, and loses the `::before` selector :( */
}

/* With @nest, it'll be treated as (and reflected in the OM as): */

div, .foo, #bar::before {
	color: blue;
	@media (...) { 
		@nest { color: red; } 
	}
	@nest {
		color: green;
	}
}

/* Which desugars to: */
div, .foo, #bar::before {
	color: blue;
}
@media (...) {
	div, .foo, #bar::before { color: red; }
}
div, .foo, #bar::before {
	color: green;
}

@FremyCompany
Copy link
Contributor
FremyCompany commented Oct 25, 2023

@FremyCompany Your idea would imply a combinatorial explosion?

@Loirooriol If this is just to solve the problem of #8738, no, because nothing can be nested inside this rewritten rule. They are created only at parse time for promoted declarations. However, if this is done generally (under the scenes) to solve the general discomfort with & being :is(...), then yes. As I mentioned, I do not have a strong opinion on that, although I'm sympathetic to Lea's effort to try to solve this.

@FremyCompany
Copy link
Contributor

There are already 3^4 = 81 unique specificities that the innermost properties could potentially have, depending on what precisely they matched. Adding another level would triple it again, to 243; adding a fourth selector to each level would similarly roughly triple it to 4^4 = 256.

@tabatkins I don't this this is correct, because specificity commutes ;)

Btw, I'm trying not to take a party in the argument here, I was just trying to restate and rephrase the proposals I heard in the discussion today which I don't think were properly understood by everyone (even though they were by you, as you had spent more time on the issue already).

@LeaVerou
Copy link
Member Author
LeaVerou commented Oct 25, 2023

@FremyCompany Indeed. And while article, .article isn't realistic, things like button, .btn are super common.

A particularly egregious (and yet realistic) example:

file1.css

button.primary {
	background: var(--color-accent);
	color: white;
}

file2.css

button, a.button {
	background: #eee;
}

<button class="primary"> ends up having white text and a light gray background…

The root problem goes way beyond shifting up declarations in nesting or matching pseudo-elements. The way nesting currently works breaks the fundamental assumption that just adding something at the end of a selector list does not affect existing matching.

And what if we just want to one-off style certain elements as buttons without adding class="button" to them?

button, a.button, #nav > li > a {
	background: #eee;
}

Good luck getting any other background on your buttons now!

So I don't think the solution I proposed in the first post would actually work, nor would @nest, since the problem is just as frequently author-triggered. We need to fix the root issue here.

As @emilio mentioned in the call, a first avenue is to dig up and re-examine the reasons why we couldn't do dynamic specificity on :is().

If that is not workable, maybe explore better rewriting algorithms that avoid combinatorial explosion without flattening specificity. @tabatkins mentioned the following example:

#a, .a, a {
	#b, .b, b {
		#c, .c, c {
		}
	}
}

With naïve rewriting, that would produce 3^3 = 27 selectors. In the general case of N selectors per list and M levels nesting, we have N^M selectors, hence the exponential combinatorial explosion that is mentioned.

However, in terms of specificity, there are only 10 different specificities (not 27 as was mentioned in the call):

  1. 3, 0, 0
  2. 2, 1, 0
  3. 2, 0, 1
  4. 1, 2, 0
  5. 1, 1, 1
  6. 1, 0, 2
  7. 0, 3, 0
  8. 0, 2, 1
  9. 0, 1, 2
  10. 0, 0, 3

I think the number of distinct specificities of the combination is O(N * M), i.e. not exponential (and close to linear, since M is usually relatively small), even in the worst case where every item in the selector list has a different specificity.

Now how could we rewrite this to utilize that (and is it even possible)?
I have no idea, but that’s the direction I was saying we could explore.

@Loirooriol

If the specificity can vary, then it has a performance impact, e.g. knowing that the element matches .a2 .b2 .c2 .d2 .e2 .f2 is not enough, because maybe it also matches a3 #b1 .c2 d3 .e2 f3 which has a higher specificity. So checking all/several possible combinations is needed.

If that was the only issue, it's far better to say that the specificity is equal to the first one that actually matches, rather than the max specificity in the list!

Also this becomes much less of a problem if the selector lists that comprise the selector are looked at individually: browsers already do that for regular matching on selector lists. The problem comes up because we're trying to implement nesting by rewriting, like a preprocessor.

@LeaVerou LeaVerou changed the title [css-nesting] Problems with indiscriminately wrapping all parent selectors with :is() [css-nesting-1] Wrapping parent selectors with :is() introduces a host of problems Oct 25, 2023
@andruud
Copy link
Member
andruud commented Oct 25, 2023

+1 to Tab's @nest proposal.

before we decide to add even more syntax to CSS to do similar but slightly different things (which is an antipattern from a UX/DX perspective).

Isn't special-casing & just a different (worse) way to add more stuff that does slightly different things? :-)

Far better to say that the specificity is equal to the first one that actually matches, rather than the max specificity in the list!

No, please keep the specificity calculation simple, and avoid any kind of "dynamic" specificity based on context.

@LeaVerou
Copy link
Member Author
LeaVerou commented Oct 25, 2023

@tabatkins

As I said, I'd object to treating a rule with a lone & differently; this sort of context-sensitivity is confusing for authors

It depends on whether you see :is() as part of the design of this feature, or as an implementation detail.
If you see it as an implementation wart, the less authors are exposed to it, the better.
If you see it as part of the design that you need to teach and explain, then I see your point: it would indeed be confusing for authors iff :is() were actually part of their mental model about how nested selectors match. However, right now I guarantee you it is not. Their mental model is that selectors match as if they were rewritten with the combinatorial explosion method. I'd hypothesize that even authors who do know that :is() is used for the rewrite would still not take it into account most of the time. You can easily verify this by asking authors what they'd expect in any situation where the two produce a different outcome.
It's just that they don't hit the differences between the two often enough that this becomes a pain point. But at least with preprocessors, they can peek at the output and debug — here this is entirely internal.

and means that seemingly-innocent changes like adding an additional selector to a list can change the meaning of other selectors they didn't touch.

The current behavior has exactly that effect! See #9492 (comment)

@tabatkins
Copy link
Member

I don't this this is correct, because specificity commutes ;)

Yes, specificity commutes, but that's not particularly relevant in this case. There are 3^4 = 81 possible ways to be matched, and we still need to check all of them to know what the highest specificity among the valid matches are. (And the number of unique specificities is still exponential, just somewhat smaller.) So I slightly misspoke; please substitute the correct term when you read my previous post. ^_^

Far better to say that the specificity is equal to the first one that actually matches, rather than the max specificity in the list!

As I keep saying, this is impossible, as it implies doing exponential work. No browser is willing to do that. Excise it from your possibility space; nothing that achieves this will be acceptable.

It depends on whether you see :is() as part of the design of this feature, or as an implementation detail.

It's absolutely an implementation detail. If we could do the "real" desugaring, we absolutely would; it definitely makes more sense. We just can't. The impossibility of this was well-established in Nesting discussions years ago, and there's no solution to it.

The current behavior has exactly that effect!

Fair, but your suggestion has a significantly larger effect. Currently, adding a new selector to an existing list only changes the behavior if the new selector has higher specificity; if it's the same or lower, there's no behavior change at all. In your suggestion, & {...} would change specificity regardless of what was added to it, and pseudo-elements in the parent list would suddenly stop matching. That's a pretty big change! And, I think, definitely confusing.

@bradkemper
Copy link
Contributor
bradkemper commented Oct 28, 2023

@tabatkins So, my understanding of your proposal is that you couldn't just use @nest in place of & (which would lead to a similar specificity checking explosion), because it could only be used by itself and not combined with other selectors on the same line. And this @nest would be implied in certain cases, such as with at-rules around bare declarations, or with declarations after at-rules. Do I have that right?

So it would help with this:

button, .button{
	color: blue;
	@media (...) { color: red; }
	color: green;
}

But not with this:

button, .button {
   span {
   color: blue;
      @media (...) { 
           i {
               color: red; 
           }
       }
	 color: green;
   }
}

I'm curious where @nest would end up in the desugaring of that. I think I know, but I'm not sure.

@Loirooriol
Copy link
Contributor

I guess it would basically behave like

:is(button, .button) span {
  color: blue;
}
@media (...) {
  :is(button, .button) span i {
    color: red; 
  }
}
:is(button, .button) span {
  color: green;
}

@FremyCompany
Copy link
Contributor
FremyCompany commented Oct 29, 2023

At the risk of getting lynched, have we considered disallowing selector lists in nesting?

That way, people would use :is() or :where() explicitely when they need a list, and it would be more obvious.

Because I admit that it smells bad to me that

a, .link{ * { color:inherit} }

and

a *, .link * { color:inherit}

behave differently.

We could also explicitely allow only one top-level list, do proper expansion for that one, but disallow nested lists and force authors to wrap them in :is() to make them aware of the change in specificity that it implies.

Doing this would make the above example behave properly, and would avoid selector {...} and selector { & {...} } from ever behaving differently, therefore removing the need to reintroduce @nest.

It also becomes possible to relax this later on; even though Tab argues that this is impossible now, who knows what the future holds.

It also enables preprocessors to support it by rewriting without introducing a weird new syntax. People using preprocessors have more agency to notice blowing up issues based on the file size they serve.

@Alohci
Copy link
Contributor
Alohci commented Oct 30, 2023

Much as I like the idea of an @nest type rule for the benefit of CSSOM, could someone explain why the @nest { ... } syntax is necessary? Is there any use for it beyond supporting declarations after nested rules, and if not, can't it always be implied?

@tabatkins
Copy link
Member
tabatkins commented Oct 31, 2023

It is always impliable, but it has to be reflected in the OM as some kind of rule, and that means the rule needs to have a serialization. (Otherwise, some OM shapes you can construct won't round-trip: if we serialized this "naked props wrapped in a rule" as just the props, directly, then two of these rules in a row would round-trip as just one, and starting the child list with one of these rules would cause it to disappear.)

But authors shouldn't ever need to actually write this themselves, no.

@andruud
Copy link
Member
andruud commented Nov 1, 2023

FWIW I agree it's not too late to change this.

I added a use-counter to Chrome (120) to detect cases of "trailing" declarations.

Unfortunately it's not obvious how to add a use-counter for the change in specificity that would follow from the @nest proposal. :/

@tabatkins
Copy link
Member

@bradkemper

Right, it wouldn't "help" the @media (...) { i {...}} example; this would continue to act exactly like the i {...} nested directly without the @media. We're just trying to avoid the problem where naked properties are treated differently based on whether they're nested or not.

In your example, the desugaring would be:

button, .button {
   span {
      color: blue;
      @media (...) { 
         i {
            color: red; 
         }
      }
      @nest {
         color: green;
      }
   }
}

(only one @nest added, to capture the trailing color:green)


@FremyCompany

At the risk of getting lynched, have we considered disallowing selector lists in nesting?

That would simply be broken, imo. Look at any Sass project; there's selectors lists at every level of nesting. And being able to use selector lists both in the parent and in the child, depending on whether it's a prefix or suffix being repeated, is a pretty vital bit of the functionality. Nothing about CSS is predicated on a meaningful distinction between stuff at the beginning of a selector vs the end (beyond the concept of the selector subject at all, of course).

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 12, 2024
The CSSWG wants to change how non-leading bare declarations work.
Instead of "shifting up" those declarations to the rule's main
declaration block, we want to treat them as wrapped by an &-rule,
except with parse-time expansion of selectors instead of the normal
behavior for '&' [1][2].

Actually making this change is easier said than done, since we now
have to worry about the compat risk. A naive use-counter added
previously shows that ~0.16% of page loads at some point parse
bare declarations after a nested rule. This is too high, and we
need a more advanced use-counter.

This CL adds the concept of "quiet rules", which are generated
automatically from detecting bare declarations in CSSParserImpl
and mostly treated normally until we reach StyleCascade.
In StyleCascade we cascade with and without the quiet rules,
check if quiet rules made a difference, and trigger a use-counter
based on that.

[1] w3c/csswg-drafts#8738
[2] w3c/csswg-drafts#9492
[3] https://chromestatus.com/metrics/feature/timeline/popularity/4709

Bug: 1517290
Change-Id: Ieef0b47d85ffb5a5e58424457422686e74ea0668
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 25, 2024
The CSSWG wants to change a few things about how "bare declarations"
work. Instead of "shifting up" those declarations to the rule's main
declaration block, we want to treat them as wrapped by an &-rule,
except with parse-time expansion of selectors instead of the normal
behavior for '&' [1][2]. The parse-time expansion part also affects
our existing implicit wrappers generated by bare declarations within
nested grouping rules (e.g. nested @media). Those wrappers rules may
get their specificity lowered.

Actually making these changes is easier said than done, since we now
have to worry about the compat risk. A naive use-counter added
previously [3] shows that ~0.16% of page loads at some point parse
bare declarations after a nested rule. This is too high, and we
need a more advanced use-counter. Also, it doesn't cover
the specificity change for the (existing) implicit wrapper rule.

This CL is the first in a chain which (hopefully) gives us
the tools needed to use-count for these changes.

The "signaling rules" added by this CL, are simply normal style rules
which carry a "signal" that is converted to a use-count (or not)
later (see CSSSelector::Signal for more detail). This alone is not
enough to cover the cases we care about for [1] and [2], so for now
this new capability is not used outside of tests.

[1] w3c/csswg-drafts#8738
[2] w3c/csswg-drafts#9492
[3] https://chromestatus.com/metrics/feature/timeline/popularity/4709

Bug: 1517290
Change-Id: Iad193ff1da07039b9876fadd413f5c9654be08ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5201402
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Steinar H Gunderson <sesse@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1251993}
aarongable pushed a commit to chromium/chromium that referenced this issue Feb 2, 2024
This CL emits invisible rules and signaling rules (which were
introduced by CL:5201402 and follow-ups) during parsing,
in order to use-count two situations:

  1. Instead of "shifting up" bare declarations within nested style
     rules, can we generate wrapping rules "in-place"? [1]

     To figure this out, we generate those would-be in-place
     wrapping rules in ConsumeDeclarationList, except they are
     marked as *invisible*, i.e. they participate until
     the StyleCascade, but are dropped during that process.
     In addition, those invisible rules are marked as *signaling*,
     which means that they trigger a use counter if they have
     some effect on the cascade.

     The hope is that by adding these rules, and proving that
     they don't have any effect, we can conclude that the change
     is safe, because adding the invisible rules gave us the same
     end result as before.

  2. Can we change the effective specificity of the generated
     &-rule wrappers? [2]

     This is only relevant for nested grouping rules, like @media.
     To measure if this is safe, we generate an additional invisible
     wrapping rule (before the regular &-rule) with the new
     specificity characteristics (in ConsumeRuleListOrNested-
     DeclarationList). Additionally, we make the existing &-rule
     signaling. Hopefully, we if we can show that the &-rule
     has no effect on the end result if the (invisible) alternative
     rule is present, we can conclude that it's safe to remove.

The two approaches in (1) and (2) kind of mirror each other:

 - In (1), we add an invisible rule *after* the main rule,
   hoping that the quiet rule has no effect.
 - In (2), we add an invisible rule *before* the main rule,
   hoping that the *main rule* has no effect.

Note that invisible/signaling rules are only emitted when parsing
bare declarations: the rule *parsed* by a call to insertRule
can contain invisible/signaling rules, but nothing special happens
for the insertion itself. In other words, inserting a rule with
CSSOM does not create invisible/signaling sibling rules.

Introducing signaling/invisible rules adds quite a bit of complexity
to the code, and negatively impacts performance. All code introduced
by CL:5201402 and its follow-ups are therefore planned to be removed
again some time during 2024.

StyleCalcPerfTest for this CL:

Initial style (µs)     Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce                8099      8174   -0.9%  [ -1.3%,  -0.6%]
Encyclopedia            72995     73268   -0.4%  [ -0.7%,  -0.0%]
Extension               84010     84599   -0.7%  [ -1.1%,  -0.3%]
News                    33645     33942   -0.9%  [ -1.3%,  -0.5%]
Search                  10059     10133   -0.7%  [ -1.1%,  -0.3%]
Social1                 18862     19007   -0.8%  [ -1.1%,  -0.5%]
Social2                 12782     12885   -0.8%  [ -1.3%,  -0.4%]
Sports                  41048     41146   -0.2%  [ -0.5%,  +0.0%]
Video                   25840     25953   -0.4%  [ -0.6%,  -0.2%]
Geometric mean                            -0.6%  [ -0.9%,  -0.4%]

Parse (µs)             Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce                1401      1435   -2.4%  [ -3.0%,  -1.9%]
Encyclopedia             7794      7935   -1.8%  [ -2.1%,  -1.5%]
Extension                1374      1391   -1.2%  [ -1.5%,  -0.8%]
News                     8372      8579   -2.4%  [ -2.7%,  -2.1%]
Search                   5244      5374   -2.4%  [ -2.8%,  -2.1%]
Social1                 15499     15935   -2.7%  [ -3.2%,  -2.4%]
Social2                   635       653   -2.7%  [ -3.2%,  -2.3%]
Sports                  59797     61250   -2.4%  [ -2.7%,  -2.1%]
Video                   37212     38083   -2.3%  [ -2.6%,  -2.0%]
Geometric mean                            -2.3%  [ -2.5%,  -2.0%]

Recalc style (µs)      Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce                4274      4330   -1.3%  [ -1.7%,  -0.8%]
Encyclopedia            53741     54462   -1.3%  [ -1.8%,  -0.9%]
Extension               67906     69119   -1.8%  [ -2.3%,  -1.3%]
News                    24823     25113   -1.2%  [ -1.8%,  -0.6%]
Search                   4307      4343   -0.8%  [ -1.3%,  -0.3%]
Social1                 10078     10195   -1.2%  [ -1.5%,  -0.8%]
Social2                  7881      7965   -1.1%  [ -1.6%,  -0.6%]
Sports                  18426     18608   -1.0%  [ -1.5%,  -0.6%]
Video                   11884     12000   -1.0%  [ -1.2%,  -0.7%]
Geometric mean                            -1.2%  [ -1.4%,  -0.9%]

Bug: 1517290

[1] w3c/csswg-drafts#8738
[2] w3c/csswg-drafts#9492

Change-Id: Icb12186f02f4c17070f9ef34907136a6d56e7745
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5217094
Reviewed-by: Steinar H Gunderson <sesse@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1255444}
@trusktr
Copy link
trusktr commented Apr 10, 2024

I've been starting to use CSS Nesting and ran into this problem.

Here's what I tried, that didn't work:

sl-card {
    &::part(base) {
        @media (prefers-color-scheme: light) {
            background: rgb(255 255 255 / 0.4);
            backdrop-filter: saturate(1.3) contrast(1.5) blur(33px);
        }
        @media (prefers-color-scheme: dark) {
            background: hsl(240 5.9% 11% / 0.65);
            backdrop-filter: blur(33px);
        }
    }
}

I had to change it to this for it to work:

sl-card {
    @media (prefers-color-scheme: light) {
        &::part(base) {
            background: rgb(255 255 255 / 0.4);
            backdrop-filter: saturate(1.3) contrast(1.5) blur(33px);
        }
    }
    @media (prefers-color-scheme: dark) {
        &::part(base) {
            background: hsl(240 5.9% 11% / 0.65);
            backdrop-filter: blur(33px);
        }
    }
}

I thought CSS Nesting was not working as it should.; I thought Nesting was merely syntax sugar. Then someone pointed me to this issue.

Note that the second code snippet means duplication of the selectors (I wanted to avoid that).

In the first snippet, if we had named media queries, co-location of logic would be better DX 👌 . But even without named media queries, to me co-location is still more desirable than spreading the same selectors across top level media queries.

Something like this would be nice if it worked:

@mediavalue --foo (...);
@mediavalue --bar screen and (...);

.foo {
  .bar::pseudo {
    @media --foo {
      ...
    }
    @media --bar {
      ...
    }
  }
  .baz::pseudo {
    @media --foo {
      ...
    }
    @media --bar {
      ...
    }
  }
  .lorem::pseudo {
    @media --foo {
      ...
    }
    @media --bar {
      ...
    }
  }
}

This would be less repetitive (assuming some syntax for reuse of media query expressions), and IDE tools would be able to autocomplete the media variables.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 11, 2024
…el,desktop-theme-reviewers,dao

Use the non-native theme number input rather than rebuilding one.

D205379 actually broke the hover effect because nesting with a
pseudo-element doesn't work as you expect, see
w3c/csswg-drafts#9492

Differential Revision: https://phabricator.services.mozilla.com/D207178
@trusktr
Copy link
trusktr commented Apr 12, 2024

I noticed another problem, that I think might be related to :is() handling (if not a Chrome browser bug?). First of all, this doesn't work, because chained ::parts are unfortunately not allowed, and no styling is applied:

.some-el::part(foo)::part(bar) {
  color: red;
  outline: 1px solid blue;
  background: green;
}

However! If I change it to this:

.some-el {
  &::part(foo)::part(bar) {
    color: red;
    outline: 1px solid blue;
    background: green;
  }
}

then some elements on the page get styled with spacing differences (from some other style somewhere else??). I see that nothing in my app gets colored red, blue, or green, but only the layout changes.

Reproduction:

https://codepen.io/trusktr/pen/GRLGVag/0766d3f4ac618b4e483434912b2db46d

Uncomment the part marked with AAA and nothing will change in the output. Comment the AAA part back out, and uncomment the part marked with BBB, and the layout changes!

I'm not sure what it is yet, because in my mind, both of them should be exactly the same (and we know that :is() behavior (especially with pseudos) can break expectations), but maybe its a bug this time.

Can someone explain what the difference is between those two exactly?

@romainmenke
Copy link
Member
romainmenke commented Apr 12, 2024

@trusktr Might be better to file individual issues for separate concepts and questions :)
I am curious about your issue, but it doesn't contain a runnable example. I don't want to take this thread off topic with a debugging session.

If it eventually turns out to be closely related you can always link back here.

@trusktr
Copy link
trusktr commented Apr 12, 2024

I was able to make a reproduction thankfully (updated my previous comment). Is it better as a crbug?

@romainmenke
Copy link
Member

Is it better as a crbug?

yes :)

Here is a minimal repro : https://codepen.io/romainmenke/pen/poBZzjd

This is unrelated to this thread.

@trusktr
Copy link
trusktr commented Apr 15, 2024

This is unrelated to this thread.

Ah ok. As a new end user of the Nesting feature, "syntax not working as expected" is what brought me here (not specifically the :is() behavior implementation detail). It is now apparent from above that I've been bitten by the "syntax not working as expected" in more than one way (although one might just be a browser bug).

ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this issue Apr 15, 2024
…el,desktop-theme-reviewers,dao

Use the non-native theme number input rather than rebuilding one.

D205379 actually broke the hover effect because nesting with a
pseudo-element doesn't work as you expect, see
w3c/csswg-drafts#9492

Differential Revision: https://phabricator.services.mozilla.com/D207178
@yisibl
Copy link
Contributor
yisibl commented Jun 20, 2024

This limitation is sure to confuse more and more authors, even developers of browser engines, see https://issues.chromium.org/issues/40278599

I extracted the style from the issue:

dialog, dialog::backdrop {
  transition: opacity 1s, display 1s allow-discrete, overlay 1s allow-discrete;
  opacity: 0;
}

dialog:modal, dialog:modal::backdrop {
  @starting-style {
    opacity: 0;
  }
  opacity: 1;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-nesting-1 Current Work
Projects
None yet
Development

No branches or pull requests

10 participants