- From: Dael Jackson <daelcss@gmail.com>
- Date: Sun, 10 Sep 2023 11:22:52 -0400
- To: www-style@w3.org
========================================= These are the official CSSWG minutes. Unless you're correcting the minutes, Please respond by starting a new thread with an appropriate subject line. ========================================= CSS Nesting ----------- - The latest test case on #2881 (Concern about combinatorial explosion) showed breakage with too many nested style rules which the group agreed that it should be fixed. - It's unclear what (author-understandable) limits would be appropriate to avoid such out-of-memory/perf problems. - RESOLVED: Limits on nesting is ua-defined (Issue #2881: Concern about combinatorial explosion) - Several people expressed concerns/dislike with the way interleaved rules do not follow source order in the cascade, but get pulled up. However, there was not a clearly workable counter-proposal. The issue will remain open while folks draft up an alternative behavior. (issue #8738: Figure out whether we're fine with "shifting up" bare declarations after nested rules) ===== FULL MEETING MINUTES ====== Agenda: https://github.com/w3c/csswg-drafts/projects/38 Scribe: bramus CSS Nesting =========== Concern about combinatorial explosion ------------------------------------- github: https://github.com/w3c/csswg-drafts/issues/2881 TabAtkins: Depending on your implementation strategy for nesting and possibly all times, there can be an issue with deeply nested style rules TabAtkins: particularly if you have selector lists TabAtkins: oriol posted an example at the end TabAtkins: Crashed webkit and makes style resolution take 10s on blink <astearns> https://yarusome.github.io/box-shadow-proposal/ <astearns> https://github.com/w3c/csswg-drafts/issues/2881#issuecomment-1642450622 for the latest test TabAtkins: We previously talked about a possible limit TabAtkins: In practice 3 is a reasonable limit. People don't go much further than that TabAtkins: sass did a survey on it too TabAtkins: we should spec some reasonable minimum to require and to let browsers have an implementation defined nesting depth TabAtkins: webkit has a limit of 128, but as you can see you can trigger problems much earlier TabAtkins: suggestions to support at least 10 limits, to err on the safe side of what people use TabAtkins: max can be higher than that <emilio> Firefox takes 3.6s on my machine on the test-case fwiw ;) matthieudubet: Issue is not surely with depths but length of selector matthieudubet: depth limit is easier to explain matthieudubet: Real issue here is the lengths of the context selector matthieudubet: If we put limit, we can put limit on the ??? context selector matthieudubet: not depth per se matthieudubet: Suggesting to put limit of length on selector list and length of context selector TabAtkins: And I think limit would not be a particular complex selector, but in product of nested selector lists TabAtkins: example with 256 * 256 TabAktins: product of lengths of selector lists seems fine TabAktins: whichever seems more reasonable to impls TabAktins: we can express a limit that is higher than we expect to see matthieudubet: Should we define how we would fail? matthieudubet: support until some depth? TabAtkins: Sometimes we do and sometimes not TabAtkins: in this case might make sense to ignore rules that were nested too deeply fantasai: If we do a limit on depth, we might all want to have the same limit across vendors fantasai: we need to have a min depth that is sufficient for authors <fantasai> Losing a few rules that are just past the limit in some browsers but not others would be a problem that could really break things <fantasai> So we should have interop on nesting depth support if it's not something so far beyond what authors would use that no one will run into it plinss: +1 on limit consistency plinss: Concerned about when authors hit it: are they gonna know? plinss: Also concerned about performance impact when we have a reasonable limit. Limit can be ??? than we have right now plinss: Other issue - don't want to derail - but might make explosion problem worse so will keep for later Rossen: Please introduce plinss: When the parent rule has commas with multiple lists, the nested rule is prewrapped in :is() and that breaks the cascade astearns: There is an issue for that emilio: I don't understand <bkardell> thought :matches? TabAtkins: :is() doesn't reflect specificity plinss: I expect two rules that are nested cascade with different specificity level, with :is() you don't have that plinss: explosion is worse than we think it is, if we fix that <fremy> I don't have that expectation, personally emilio: I'm ok with a limit emilio: I don't think a max should be specced Rossen: That is what Tab suggested Rossen: We did something similar for units way back TabAtkins: But Elika was saying we should spec a max fantasai: If the limit is something low, then it is reasonable authors will hit that. fantasai: some browsers might then drop rules Rossen: No, this is about a lower limit fantasai: If the upper limit is far enough – e.g. 100 – only some test cases or auto-generated stuff might hit it, then it's ok to not be interoperable fantasai: This proposal is that min is 10, but that would mean max should also be 10 fantasai: Min should therefore be far enough to not cause interop problem emilio: We don't have limit of length of selectors. emilio: It all depends on what you are nesting emilio: Single selectors nesting is probably fine emilio: If you nest lists of 100 selectors each, that might blow up soon fremy: Nesting limit does not seem the right metric fremy: Correct midway is to see how much selectors you end up with fremy: e.g. you can have up to 256 if it is expanded fremy: Better to limit total num of selectors TabAtkins: Won't work TabAtkins: At least in blink, if you take oriol's example and wrap it in :is(), you still have a problem TabAtkins: It is combinatorial in some more ephemeral notion of complexity that I do not want to define fremy: That is different fremy: if you put in :is() you only have complexity in running the code, not storing it TabAtkins: Blinks performance is the same for with as without TabAtkins: reason we have 10s recompute here is based on something more intrinsic about complexity of the selector, not the length of the list TabAtkins: a expanded selector here is still 125. TabAtkins: Complexity here is not something we can easily define in terms of any selector metric matthieudubet: If you count list inside of parent list, the time is linear to. matthieudubet: (missed) matthieudubet: limit I am suggesting should be related to length TabAtkins: webkit will delay for longer than 10s right now. chrome and firefox have similar timings TabAtkins: but also def of 'what things to expand' is already a number of things: :is(), :where(), :nth-child() with 'of' matthieudubet: Yes, it is more complex that depth matthieudubet: depth limit of 10 might not be enough TabAtkins: 10 is more then enough for virtually all cases (based on sass survey) TabAtkins: somebody might go over, but in people tend to stick below that <bkardell> that doesn't really jive with what I have seen, can we confirm it? How? ntim: I can see limit of 10 being too low, e.g. in build systems myles: What are the current limits? TabAtkins: None, there is no spec limit. florian: In implementations, they run out of memory or crash emilio: Yeah nsull: can we get data from sass (or some other party) about the number of nesting authors do? <bkardell> +1 miriam: sass does not track that miriam: people nested really deep at the start, but best practice now is 3 but people don't always follow that <bkardell> surely we can grep github for scss stuff miriam: web archive might have data on that nsull: Interesting. What is average selector length? <bkardell> ah yeah, good call... Rossen: If we don't have the data, we should explore that offline myles: Seems like the limits are purely mechanical myles: Why is this different than things like total number of nodes or nested iframes? TabAtkins: There is a reason why you would want to limit here TabAtkins: If quantity is roughly linear with amount of stuff in the page it might be ok TabAtkins: but if its exponential we are concerned about it TabAtkins: We have a limit in variables for example TabAtkins: to prevent doubling in 30 stages before going OOM TabAtkins: this falls in similar case. You don't want people to be able to crash a page by writing a crazy selector myles: I understand this is easier here, than a bunch of nested iframes myles: reason other limits are not standard is because people will not hit them myles: Are you, Tab, saying that reason that limit is needed here because we expect people to make real pages and hit it? TabAtkins: Potentially. 3rd party CSS could DOS your page, which is not ideal <nicole> myles said there are limits on DOM size or number of nested iframes that aren't standardized because they are high enough authors don't hit them bkardell: 3rd party thing is a little bit tricky bkardell: because 3rd party stuff could ruin the whole internet <TabAtkins> I don't mean "a professional company giving 3rd party style", I mean "user-controlled" bkardell: I have not seen a ton of sass, but can say that in the stuff I have seen like 4-6 depths is what was there bkardell: am happy to help look up the actual numbers TabAtkins: We do want to have a min limit so that users have an expectation of what is usable. TabAtkins: in case of browser limits get cut low TabAtkins: We rarely put a max limit TabAtkins: but proposed resolution is to add a min limit TabAtkins: It seems like a moderately excessive depth to support TabAtkins: authors might run into a cutoff after that, and UAs should make sure pages remain responsive in that case Rossen: We seem to be circling back florian: For max limit, they can then choose some metric they want, not necessarily depth florian: Are browsers likely to create a limit that authors might hit? If not, setting a minimum around 10 isn't going to constrain anyone florian: We can also say a limit is implementation defined TabAtkins: Analogous situation is size of grids, where we knew that relatively large grids were larger than the limit we had TabAtkins: I am fine with saying it is implementation defined <astearns> implementation-defined and wait for bugs to come in? TabAtkins: Authors must in that case not do stupid stuff TabAtkins: so I am happy with close no change TabAtkins: Currently there is nothing in the spec about it fremy: Quick idea: in the example you can get to 7mo selectors. Can we compute the length of the selectors like string length and compute product of that and cut off at a limit? fremy: To prevent exponential scenarios TabAtkins: I don't want to get into that complexity bag, and let implementations figure it out fremy: I am proposing to use the actual string length of a selector fremy: without considering how complex it is fremy: e.g. 1000 chars TabAtkins: Problem case here can be constructed in a few 100 chars plinss: If we allow implementers to choose a limit, then we must set a minimum and have some advice to authors about that plinss: to prevent interop issues TabAtkins: 2 proposals: TabAtkins: - close no change TabAtkins: - or a depth specified min myles: and a third to pick 10? TabAtkins: that is option 2 Rossen: we seem to come back to option 2 <SebastianZ> I'd vote for option 2. fremy: Based on Tab's remarks option 2 makes no sense? with just adding :is() you can get the same complexity in a normal selector, so why limit depth in nesting? Rossen: That is separate issue, no? fremy: It is not astearns: But maybe we should TabAtkins: It is not just :is(), :is(), :nth-child(), :where() florian: Given that this is a case, should we do nothing nowhere? fremy: That is my point. I'd rather go with option 1 than 2, as you can do it somewhere else <fremy> +1 on Rossen proposal to add a recommendation for nesting depth Rossen: (goes over options again) but happy to go with 1 plinss: I don't see point in author guidance about exceeding a max, when there is no min in spec <TabAtkins> Interestingly, if you take Oriol's example, wrap each selector in an :is(), then collapse away the nesting to make it a single selector with the same behavior, we get substantially faster performance in chrome <TabAtkins> (~0.8s on my laptop, vs 13s for the nested version) Rossen: (missed) Rossen: I appreciate the pushback, but don't see how any author guidance for that logic could turn into formal vendor reqs plinss: If we don't have min and give author guidance, that number is fiction <fantasai> +1 plinss Rossen: Perhaps for some browser on some platform plinss: Then spec it as min TabAtkins: Even if we consider that even a small depth might trigger same problem. Not certain if that is case or not. There might not be any reasonable metric to use, but reasonable author behavior with some guideline will keep people in bright space of good perf. TabAtkins: Trying to lay down a strict limit might not give us the guarantee of good perf TabAtkins: so “you should think about >” might be a good thing <fremy> @ TabAtkins, if the :is() is faster, then your original intuition about the slow down root cause might not be correct, so the issue might not be related to the number of selectors <fremy> @ TabAtkins, I guess maybe we should investigate a bit more <TabAtkins> right, I'm wondering if it's just something about the length of the nesting entirely SebastianZ: I think a min limit would be good, but SebastianZ: it is not limited to nesting but also selector complexity so maybe selector spec should also mention that? TabAtkins: Yeah SebastianZ: If we can come up with a metric, it should be in the selector spec emilio: Since this is related emilio: Same case in variable, and vendors have different limits emilio: string based, token based, … emilio: It don't think limit is easy to define emilio: perf is based on impl details emilio: complexity depends on whether elements match too emilio: I would want to leave this undefined until we have decent understanding of these performance characteristics <plinss> +1 emilio Rossen: By undefined you mean explicitly undefined? emilio: Yes <nicole> +1 emilio florian: I agree, but also give authors some guidance as well, but not in terms of max levels, since that's the wrong metric. Rather: "be mindful of combinatorial explosions. For instance {this example} crashes browsers. Don't do this <fremy> +1 matthieudubet: As Tab said, it does not depend on depth but length of selector which is easy to define matthieudubet: that are the limits we are hitting. not depth, length of selector TabAtkins: A quick test case shows it is not the case TabAtkins: e.g. 25 :is() is slow matthieudubet: I agree it might be too complex to put into a spec nsull: Agree with emilio and myles nsull: We don't know how devs will use this nsull: Making up guidance now seems too premature nsull: so I support option 1, and see what actually breaks now nsull: and we might revisit guidance later myles: I was not stating, but asking for clarification <TabAtkins> Also I misremembered, Variables does *not* impose an explicit limit, it just calls this out as an impl issue. Rossen: Seems like there is enough support for option 1 to keep it explicitly undefined Rossen: It is important that impls not crash though Rossen: Can we resolve? plinss: Small objection, as option 1 was 'close no change' and wait for data TabAtkins: I am fine with 'close no change' without prejudice TabAtkins: until we hit a problem in the future ntim: We need to figure out how nesting is used in the wild Rossen: So are you signing up? ntim: No TabAtkins: Bug trackers will find the data, and then we can file a new issue based on that <florian> +1 TabAtkins: Until we have practical proof, we don't need to hold this open iank: We had a limit for grid tracks, and we received a bunch of bugs for it. so people file them ntim: To prevent interop issues, we need the data TabAtkins: And the bug reports will surface that <bkardell> Can I ask for one clarification? Is option #1 also put no author guidance/information about preventing problems into the spec? <bkardell> Rossen: ^ can we clarify that? <TabAtkins> this issue has no bearing on author guidance in the spec <TabAtkins> I'll put some in. Rossen: Let's close here Rossen: Do we want to resolve on 1: explicitly define that the limit is (pause) undefined <bkardell> if tab will put some explanation in, I can +1 on option 1 <TabAtkins> I'll put in some guidance similar to this https://drafts.csswg.org/css-variables/#long-variables Rossen: Objections? plinss: No Rossen: Explicitly undefined as in “no limit” plinss: We will have answer when research is done Rossen: Objections? florian: Saying explicitly that there is no limit, you require ??? TabAtkins: No, not 'no limit' but undefined TabAtkins proposed resolution: nesting limit is UA defined <TabAtkins> proposed resolution: limits on nesting is ua-defined RESOLVED: limits on nesting is ua-defined Is it OK for nesting to reorder declarations? --------------------------------------------- github: https://github.com/w3c/csswg-drafts/issues/8738 emilio: Right now in nesting, when you have mixed declarations and nested rules, the current behavior of nesting (and sass?) is to pull all the declarations up which may feel a bit weird if you don't know this happens emilio: the source order changes emilio: don't know where we ended up in this discussion emilio: but I think it is unfortunate emilio: if people are fine with no change, so am I Rossen: Tab’s last comments were suggesting that TabAtkins: Yes Rossen: Remarks? fremy: Thank you emilio for filing. fremy: Would love if we could fix this but understand that implementations are shipped fantasai: I think this is very confusing. On the plus side it is not common authors will do this, where they have the same specificity as the ??? and they are modifying the same property. So authors will not commonly run into this fantasai: However, when they do, the fact that it applies out of order is very confusing fantasai: We don't have to have this problem fantasai: In terms of options, we can say that when you are interleaving, you have to make additional ampersand rules to have them maintain their position fantasai: Seems straightforward but are concerned that we should fix the cascade because it is confusing TabAtkins: Main argument for no change is that this behavior is the same as what preprocessors do since they supported it TabAtkins: It has not been a problem for any of them as far as we can tell TabAtkins: for sass we have a maintainer confirming that TabAtkins: so we don't need to worry about it I think TabAtkins: It is not an issue in practice, so we should not come up with complicated solution TabAtkins: MQs and non-style rules have a different behavior when nested than style rules are TabAtkins: style rules put all their props in the style as if it were up front TabAtkins: and MQs in similar put all their ?? in nested style rules TabAtkins: We could run into the issue that knowing when nesting has begun depends on non; it is not intuitive what to define well when we have started nested TabAtkins: A rule that is unknown or mis-typed can change that interpretation TabAtkins: in an unpredictable way TabAtkins: to keep consistent behavior lets avoid that problem entirely, and data from preprocessors has shown it is not a problem fremy: The last argument seems valid fremy: The examples emilio gave is not about nesting selectors but nesting an at-media rule, which I can do fremy: I feel example is convincing enough fremy: is implementation of sass/less supporting this use case? fremy: If they don't, then we are making things quite ??? TabAtkins: Yes, I believe they do TabAtkins: throw it into a sass playground and it will output the equivalent compiled code with media shifted out TabAtkins: the two decls will be combined together fremy: In this case I don't feel strongly enough that we should emilio: So fixing interleaved decls inside a grouping rule by wrapping in &-rule is easy emilio: fixing style rules by adding nested style rules is not too hard but … I guess we could fix it emilio: Don't feel too strongly either way <TabAtkins> yup, today emilio's examples compiles to "the div with both properties, followed by the @media holding a div holding the conditional props" Rossen: so then the proposed resolution is to close no change fremy: Did anybody look into Lea Verou’s comment? Rossen: We did jensimmons: We do not like the proposed resolution. It does not make sense for authors right now. They expect for the later styles to apply jensimmons: I agree that a non-complicated thing is a goal, but should not be what we have now jensimmons: sass is a guiding principle, but we should follow how CSS has always worked: source order <ntim> +1 jensimmons: ntim has a proposal <miriam> Here's the example in a Sass playground, for reference (sorry it's a long url): https://sass-lang.com/playground/#MTFkaXYlMjAlN0IlMEElMjAlMjBjb2xvciUzQSUyMGdyZWVuJTNCJTBBJTIwJTIwJTQwbWVkaWElMjAod2lkdGglMjAlM0UlMjAwKSUyMCU3QiUwQSUyMCUyMCUyMCUyMGNvbG9yJTNBJTIwcmVkJTNCJTBBJTIwJTIwJTIwJTIwYmFja2dyb3VuZCUzQSUyMHJlZCUzQiUwQSUyMCUyMCU3RCUwQSUyMCUyMGJhY2tncm91bmQlM0ElMjBncmVlbiUzQiUwQSU3RA== ntim: emilio wrote it in the issue : wrapping in & astearns: Just for my clarification: that solution will allow later rule to override the earlier ones? [multiple]: Yes plinss: I presume for wrapping ??? in another declaration block it becomes another rule in the OM? TabAtkins: Yes plinss: Is the & still required these days? TabAtkins: Yes, you need a selector TabAtkins: In terms of raw css syntax parsing, omitting the selector will give you a rule with empty prelude and that will fail to parse as valid style rule TabAtkins: Further issue as explained in thread. anything we do that distinguished behavior on before/after nested rule means we have to define the switch for 'we are past a nested rule' TabAtkins: Can't be ??? to cause the syntax trigger TabAtkins: We don't need that concept if we don't do this TabAtkins: It will potentially change the cascade emilio: Why would you have problem to just use valid nested rule as trigger? TabAtkins: Because a new rule that does not exist in older browser that understands nesting will throw away that new rule and conclude that nesting hasn't started yet emilio: And the declarations will ?? and that seems fine TabAtkins: And the OM changes emilio: ???? emilio: Like the OM changes, but it also changes when you introduce new rules. emilio: you end up with different length list fantasai: If we have style rule with interleaved rules and declarations. Lets say 3 decls at top, at-rule, and 2 decl at bottom fantasai: old browser will have 5 decls appear in .style of that stylerule fantasai: if we wrap it in an :is() rule then in a new browser you would get first 3 decls in .style. then at-media in .cssRules followed by - followed in it - a new style rule with last 2 decls TabAtkins: Not, but that is closely related. <TabAtkins> specifically, `::before { & {color: red;}}` does *not* apply red to ::before in today's spec <TabAtkins> and causing implicit wrapping would trigger that problem as well scribe: dbaron oriol: About wrapping decls in an &: with two elements, the & can't represent 2 elements. If you have a declaration, garbage, and a declaration -- if later a new feature parses the garbage as a nested rule, then the following declaration would be grabbed and not work. oriol: While I agree the current behavior is not great, and would be a good argument for choosing option 4, I'm leaning more towards what tab is saying -- keeping current behavior TabAtkins: I forgot about that -- I agree that kills it harder. You can't nest if your parent rule has pseudo-elements. That's the behavior of :is(). If you tried to do this and parent selector applied styles to pseudo-elements, the implicit wrapping would just drop these declarations on the floor. That's broken at a fundamental model level right now. plinss: That goes away if we get rid of the :is() behavior, right? TabAtkins: Yes TabAtkins: My proposed resolution is still to close with no change; I think current spec is right. TabAtkins: but we should see if objection from Apple is still standing ntim: I wonder if we can translate to a ? rule without using &, just using the selector text. <ntim> div { color: green; } @media (width > 0) { div { color: red; background: red; } } div { background: green } TabAtkins: No, because that's a relative selector TabAtkins: Those nested rules are "div div"; it's still relative to the parent rule hober: Siblings, not nested myles: There's 3 sibling rules in that example TabAtkins: That would mean treating nesting as a preprocessor directive that rewrites into a different rule structure TabAtkins: We haven't been pursuing that approach since very early on in nesting. I'd like to reject trying to rewrite style things. That makes many things more complicated, like for how @media nesting works in rules. But the more we diverge the OM and the model underneath from the syntax authors write, the worse it is. fantasai: I think I'm not comfortable resolving on no change -- it doesn't sound like we have consensus -- but we don't have a clear counter proposal worked out. I think TabAtkins brought up many useful concerns with emilio's path. We should take some time outside the meeting to review minutes and try to think through the possibilities. Might be down to these 2, but maybe something else. And see if we can address relevant concerns, or at least have a better understanding of what they are <jensimmons> +1 to what Elika just said TabAtkins: If you think it's valuable to more exploration, you're welcome to. I don't think it's going to work, but you're welcome to. fantasai: Sounds like there's an action item for people with concerns about current proposal to come up with a counter proposal. Rossen: No rule that we can't reopen issues. fantasai: I object to resolving when this many people don't like the direction. TabAtkins: As long as it's not a drastic change (like turning it into a rewriting rule), unlikely there will be compat concerns if it changes relatively soon, given that this is a rare code pattern. But not indefinitely, and limitations on how far that reaches. Rossen: So let's move on to the next issue. And once Apple or anyone else has a better proposal, bring it back. <astearns> fwiw I have already seen authoring advice against mixing rules and declarations this way, so if people follow that advice it lessens the compat concerns
Received on Sunday, 10 September 2023 15:23:28 UTC