- From: CSS Meeting Bot via GitHub <sysbot+gh@w3.org>
- Date: Wed, 19 Jul 2023 21:41:10 +0000
- To: public-css-archive@w3.org
The CSS Working Group just discussed `[css-nesting] Concern about combinatorial explosion`, and agreed to the following:
* `RESOLVED: limits on nesting is ua-defined`
<details><summary>The full IRC log of that discussion</summary>
<bramus> TabAtkins: depending on your implentation strategy for nesting and possibly all times, there can be an issue with depely nested style rules<br>
<bramus> … particularly if you have selectorlists<br>
<bramus> … oriol posted an example at the end<br>
<bramus> … crashed webkit and makes style resoltuion take 10s on blink<br>
<astearns> https://yarusome.github.io/box-shadow-proposal/<br>
<matthieudubet> q+<br>
<bramus> … we previously talked about a possible limit<br>
<bramus> .. in practice 3 is a reasonable limit. ppl dont go much further than tat<br>
<bramus> … sass did a survey on it too<br>
<astearns> https://github.com/w3c/csswg-drafts/issues/2881#issuecomment-1642450622 for the latest test<br>
<bramus> … we should spec some reasonable minimum to require and to let browsers have an implementation defined nesting depth<br>
<bramus> … webkit has a limit of 128, but as you can see you can trigger problems much earlier<br>
<bramus> … suggestions to support at least 10 limits, to err on the safe side of what ppl use<br>
<bramus> … max can be higher than that<br>
<fremy> q+<br>
<plinss> q+<br>
<Rossen_> ack matthieudubet<br>
<bramus> matthieudubet: issue is not surely with depths but length of selector<br>
<bramus> … depth limit is easier to explain<br>
<bramus> … real issue here is ???<br>
<bramus> s/???/is the lengths of the context selector<br>
<bramus> … if we put limit, we can put limit on the ??? cotext selector<br>
<bramus> … not depth per se<br>
<bramus> … suggesting to put limit of length on selector list and length of context selector<br>
<emilio> q+<br>
<bramus> TabAtkins: and i think limit would not be a particular complex selector, but in product of nested selector lists<br>
<bramus> TabAtkins: example with 256 * 256<br>
<bramus> … product of lengths of sel lists seems fine<br>
<bramus> … whichever seems more reasonable to impls<br>
<bramus> … we can express a limit that is higher than we expect to see<br>
<bramus> matthieudubet: should we define how we would fail?<br>
<bramus> … support until some depth?<br>
<bramus> TabAtkins: sometimes we do and sometimes not<br>
<bramus> … in this case might make sense to ignore reules that were nested too deeply<br>
<fremy> q-<br>
<fremy> matthieudubet addressed all my points, I think<br>
<Rossen_> ack fantasai<br>
<Zakim> fantasai, you wanted to react to matthieudubet<br>
<bramus> fantasai: if we do a limit on depth, we might all want to have the same limit across vendors<br>
<bramus> … we need to have a min depth that is sufficient for authors<br>
<Rossen_> ack plinss<br>
<bramus> plinss: +1 on limit consistency<br>
<bramus> … concerned about when authors hit it: are they gonna know?<br>
<bramus> … als concerned about of performance impact when we have a reasonable limit. limit can be ??? than we have right now<br>
<bramus> … other issue - dont want to derail - but might make explosion problem worse so will keep for later<br>
<bramus> Rossen_: please introduce<br>
<fantasai> Losing a few rules that are just past the limit in some browsers but not others would be a tle problem that could really break things<br>
<bramus> plinss: when the parent rule has commas with multiple lists, the nested rule is prewrapped in :is() and that breaks the cascade<br>
<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<br>
<bramus> astearns: there is an issue for that<br>
<bramus> emilio: i dont understand<br>
<bramus> TabAtkins: :is() doesnt reflect specificity<br>
<Rossen_> q?<br>
<bramus> plinss: I expect two rules that are nested cascade with different specificyt level,. with :is() you dont have that<br>
<bramus> … explosion is worse than we think it is, if we fix that<br>
<Rossen_> ack emilio<br>
<bramus> emilio: im ok with a limit<br>
<bramus> … i dont think a max should be specced<br>
<bramus> Rossen_: that is what tab suggested<br>
<bramus> … we did sth similar for units way back<br>
<bramus> TabAtkins: but elika was saying we should spec a max<br>
<bramus> fantasai: if the limit is sth low, then it is reasonable atuhors will hit that.<br>
<bramus> … some browsers might then drop rules<br>
<bramus> Rossen_: no, this is about a lower limit<br>
<bramus> fantasai: if the upper limit is far enough – e.g. 100 – only some test cases or autogenerated stuff might hit it<br>
<bramus> … this proposal is that min is 10, but that would mean max can also be 10<br>
<bramus> … min should therefore be far enough to not cause interop prob<br>
<bramus> emilio: we dont have limit of length of selectors.<br>
<bramus> … it all depends on what you are nesting<br>
<bramus> … single selectors nesting is probably fine<br>
<fremy> q?<br>
<fremy> q+<br>
<florian> q+<br>
<bramus> … if you nest lists of 100 selectors each, that might blow up soon<br>
<bramus> fremy: nesting limit does not seem the right metric<br>
<bramus> … correct midway is to see how much selectors you end up with<br>
<bramus> … e.,g. you can have up to 256 if it is expanded<br>
<florian> q-<br>
<Rossen_> ack fremy<br>
<bramus> … better to limit total num of selectors<br>
<bramus> TabAtkins: wont work<br>
<bramus> … at least in blink, if you take oriosl example and wrap it in :is(), you still have a probleme<br>
<bramus> … it is combinatorial in some more ephemral notion of complexity that i do not want to define<br>
<bramus> fremy: that is different<br>
<bramus> … if you put in :is() you only have complexity in running the code, not storing it<br>
<bramus> TabAtkins: blinks performance is the same for with as without<br>
<bramus> … reason we have 10s recompute here is based on sth more intrinsic about complexity of the selector, not the length of the list<br>
<bramus> … a expanded selector here is still 125.<br>
<bramus> … complexity here is not sth we can easily define in terms of any selector metric<br>
<bramus> matthieudubet: if you count list inside of parent list, the time is linear to.<br>
<Rossen_> q?<br>
<bramus> … (missed)<br>
<bramus> .. limit i am suggesting should be related to length<br>
<bramus> TabAtkins: webkit will delay for longer than 10s right now. chrome and firefox have similar timings<br>
<bramus> … but also def of 'what things to expand' is already a number of things: :is(), :where(), :nth-child with of<br>
<bramus> matthieudubet: yes, it is more complex that depth<br>
<bramus> … depth limit of 10 might not be enough<br>
<bramus> TabAtkins: 10 is more then enough for virtually all cases (based on sass survey)<br>
<ntim_> q+<br>
<bramus> … somebody might go over, but in ppl tend to stick below that<br>
<Rossen_> ack ntim_<br>
<bramus> ntim_: i can see limit of 10 being too low, e.g. in build systems<br>
<bramus> myles: what are the curernt limits?<br>
<nicole> q+<br>
<bramus> TabAtkins: none, there is no spec limit.<br>
<bramus> florian: in implementations the run out of memory or crash<br>
<bramus> emilio: yeah<br>
<myles> q+<br>
<Rossen_> ack nicole<br>
<bramus> nsull: can we get data from sass (or some other party) about the number of nmesting authors do?<br>
<bkardell_> +1<br>
<bramus> miriam: sass does not track that<br>
<bramus> … people nested really deep at the start, but best practice now is 3 but ppl dont always follow that<br>
<bkardell_> surely we can grep github for scss stuff<br>
<bramus> … web archive might have data on that<br>
<bramus> nsull: intersting. what is average selector length?<br>
<bkardell_> ah yeah, good call...<br>
<bramus> Rossen_: if we dotn have the data, we should explore that offline<br>
<bkardell_> q+<br>
<Rossen_> ack myles<br>
<bramus> myles: seems like the limits are purely mechanical<br>
<bramus> … why is this different than things like total number of nodes or nested iframes?<br>
<bramus> TabAtkins: there is a reason why you would want to limit here<br>
<bramus> … if qty is roughly linear with amount of stuff in the page it might be ok<br>
<bramus> … but if its exponentatial we are concerned about it<br>
<bramus> … we have a limit in variables for example<br>
<bkardell_> q-<br>
<bramus> … to prevent doubling in 30 stages before going OOM<br>
<bramus> … this falls in similar case. you dont want ppl to be able to crash a page by writing a crazy selector<br>
<bramus> myles: I understand this is easier here, than a bunch of nested iframes<br>
<bramus> … reason othe rlimits are not standard is bc ppl will not hit them<br>
<bkardell_> q+<br>
<bramus> … are you, tab, saying that reason that limit is needed here bc we expect ppl to make real pages and hit it?<br>
<bramus> TabAtkins: potentially. 3rd party CSS could DOS your page, which is not ideal<br>
<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<br>
<Rossen_> ack bkardell_<br>
<bramus> bkardell_: 3rd party thing is a little bit tricky<br>
<bramus> … bc 3p stuff could ruin the whole internet<br>
<TabAtkins> i don't mean "a professional company giving 3rd party style", I mean "user-controlled"<br>
<bramus> … 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<br>
<bramus> … am happy to help look up the actual numbers<br>
<Rossen_> q?<br>
<bramus> TabAtkins: we do want to have a min limit so that users have an expectation of what is usuable.<br>
<florian> q+<br>
<bramus> … in case of browser limits get cut low<br>
<fremy> q?<br>
<bramus> … we rarely put a max limit<br>
<fremy> q+ (proposal)<br>
<bramus> … but proposed resolution is to add a min limit<br>
<fremy> q- (proposal)<br>
<fremy> q+<br>
<bramus> … it seems like a moderate depth to support<br>
<bramus> … authors might run into a cutoff after that, and uas should make sure pages remain responsive in that case<br>
<bramus> Rossen_: we seem to be circiling back<br>
<TabAtkins> s/moderate depth/moderately excessive depth/<br>
<bramus> florian: for max limit, the can then choose some metric they want?<br>
<bramus> … are browsers likely to create a limit that uathors might hit?<br>
<bramus> … we can also say a limit is implementation defined<br>
<bramus> TabAtkins: analogous situation is size of grids, where we knew that relatively large grids were larger than the limit we had<br>
<bramus> … i am fine with saying it is impl defined<br>
<astearns> implementation-defined and wait for bugs to come in?<br>
<plinss> q+<br>
<bramus> … authors must in that case not do stupid stuff<br>
<Rossen_> ack florian<br>
<bramus> … so i am happy with close no change<br>
<bramus> … currently there is nothing in the spec about it<br>
<Rossen_> ack fremy<br>
<bramus> fremy: quick idea: in the example you can get to 7mo selectors. can we compute the length of th selectors like string length and compute product of that and cut off at a limit?<br>
<bramus> … to prevent expo scenarios<br>
<florian> s/for max limit, the can then choose some metric they want?/for max limit, the can then choose some metric they want, not necessarily depth/<br>
<bramus> TabAtkins: I dont want to get into that complexity bag, and let implementations figure it out<br>
<bramus> fremy: I am proposing to use the actual string length of a selector<br>
<bramus> .. without considering how complex it is<br>
<bramus> … e.g. 1000 chars<br>
<bramus> TabAtkins: problem case here can be constructed in a few 100 chars<br>
<Rossen_> ack plinss<br>
<bramus> plinss: if we allow implementers to choose a limit, then we must set a minimum and have some advice to auhtors about that<br>
<bramus> … to prevent interop issues<br>
<bramus> Rossen_: so back at the same proposal :)<br>
<florian> s/might hit?/might hit? If not, setting a minimum around 10 isn't going to constrain anyone<br>
<bramus> TabAtkins: 2 proposals:<br>
<bramus> … - close no change<br>
<bramus> … - or a depth specified min<br>
<bramus> myles: and a third to pick 10?<br>
<bramus> TabAtkins: that is option 2<br>
<bramus> Rossen_: we seem to come back to option 2<br>
<SebastianZ> I'd vote for option 2.<br>
<argyle> nesting depth `clamp(0, 10, 🤷♀️)`<br>
<bramus> fremy: based on tabs remarks option 2 makes no sense? with just adding :is() you can get the same complexitiy in a nmormal selector, so why limit depth in nesting?<br>
<bramus> Rossen_: that is separate issue, no?<br>
<bramus> fremy: it is not<br>
<bramus> astearns: but maybe we should<br>
<florian> q?<br>
<bramus> TabAtkins: it is not just is, is, nthchild, where<br>
<bramus> florian: given that this is a case, should we do nothing nowhere?<br>
<bramus> fremy: that is my point. id rather go with option 1 than 2, as you can do it somewhere else<br>
<bkardell_> q+<br>
<fremy> +1 on Rossen_ proposal to add a recommendation for nesting depth<br>
<bramus> Rossen_: (goes over options again) but happy to go with 1<br>
<SebastianZ> q+<br>
<TabAtkins> Interestigly, 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 beahvior, we get substantially faster perf in chrome<br>
<bkardell_> q-<br>
<bramus> plinss: i dont see point in author guidance about exceeding a max, when there is no min in spec<br>
<TabAtkins> (~0.8s on my laptop, vs 13s for the nested version)<br>
<florian> q+<br>
<bramus> Rossen_: (missed)<br>
<bramus> Rossen_: i appreciate the pushback, but dont see how any author guidance for that logic could turn into formal vendor reqs<br>
<bramus> plinss: if we dont have min and give author guidance, that number is ???<br>
<fantasai> +1 plinss<br>
<bramus> Rossen_: perhaps for some browser on some platform<br>
<fantasai> s/???/fiction/<br>
<bramus> plinss: then spec it as min<br>
<Rossen_> q?<br>
<bramus> 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 reasoanble author behavior with some guideline will keep ppl in bright space of good perf.<br>
<bramus> … trying to lay down a strict limit might not give us the guarantuee of good perf<br>
<emilio> q+<br>
<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<br>
<matthieudubet> q+<br>
<bramus> … so “you should think about >” might be a good thing<br>
<fremy> @ TabAtkins, I guess maybe we should investigate a bit more<br>
<nicole> q+<br>
<TabAtkins> right, i'm wondering if it's just something about the length of the nesting entirely<br>
<Rossen_> ack SebastianZ<br>
<bramus> SebastianZ: i think a min limit would be good, but<br>
<bramus> .. it is not limited to nesting but also selector complexity so maybe selector spec should also mention that?<br>
<Rossen_> Zakim, close queue<br>
<Zakim> ok, Rossen_, the speaker queue is closed<br>
<bramus> TabAtkins: yeah<br>
<bramus> SebastianZ: if we can come up with a metric, it should be in the selector spec<br>
<Rossen_> q?<br>
<bramus> emilio: since this is related<br>
<bramus> … same case in variable, and vendors have different limits<br>
<bramus> … string based, token based, …<br>
<bramus> … it dont think limit is easy to define<br>
<bramus> … perf is based on impl details<br>
<bramus> … complexity depends on wether elements match too<br>
<bramus> … i would want to leave this undefined until we have decent understanding of these performance characteristics<br>
<plinss> +1 emilio<br>
<emilio> ack emilio<br>
<bramus> Rossen_: by undefined you mean explicitly undef?<br>
<bramus> emilio: yes<br>
<Rossen_> ack emilio<br>
<Rossen_> ack florian<br>
<nicole> +1 emilio<br>
<bramus> florian: I agree, but also give authors some guidance as well<br>
<fremy> +1<br>
<bramus> matthieudubet: as tab said, it does not depend on depth but length of selector which is easy to define<br>
<bramus> … that are the limits we are hitting. not depth, length of selector<br>
<florian> s/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"<br>
<bramus> TabAtkins: a quick test case shows it is not the case<br>
<Rossen_> ack matthieudubet<br>
<bramus> … e.g. 25 :is() is slow<br>
<florian> s/guidance as well/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"<br>
<bramus> matthieudubet: i agree it might be too complex to put into a spec<br>
<Rossen_> ack nicole<br>
<bramus> nsull: agree with emilio and myles<br>
<bramus> … we dont know how devs will use this<br>
<bramus> … making up guidance now seems too premature<br>
<bramus> … so I support option 1, and see what actually breaks now<br>
<bramus> … and we might revisit guidance later<br>
<bramus> myles: I was not stating, but asking for clarification<br>
<astearns> ack fantasai<br>
<bramus> Rossen_: seems like there is enough support for option 1 to keep it explicitly undefined<br>
<bramus> … it is important that impls not crash though<br>
<bramus> … can we resolve?<br>
<TabAtkins> Also I misremembered, Variables does *not* impose an explicit limit, it just calls this out as an impl issue.<br>
<bramus> plinss: small objection, as option 1 was 'close no change' and wait for data<br>
<bramus> TabAtkins: I am fine with 'close no change' without prejudice<br>
<bramus> … untill we hit a problem in the future<br>
<bramus> ntim_: we need to figure out how nesting is used in the wild<br>
<bramus> Rossen_: so are you signing up?<br>
<bramus> ntim_: no<br>
<bramus> TabAtkins: bug trackers will find the data, and then we can file a new issue based on that<br>
<florian> +1<br>
<bramus> … until we have practical proof, we dont need to hold this open<br>
<bramus> iank_: we had a limit for grid tracks, and we received a bunch of bugs for it. so ppl file them<br>
<bkardell_> Can I ask for one clarification? Is option #1 also put no author guidance/information about preventing problems into the spec?<br>
<bramus> ntim_: to prevent interop issues, we need the data<br>
<bramus> TabAtkins: and the bug reports will surface that<br>
<bkardell_> Rossen_: ^ can we clarify that?<br>
<bramus> Rossen_: lets close here<br>
<TabAtkins> this issue has no bearing on author guidance in the spec<br>
<TabAtkins> I'll put some in.<br>
<bramus> … do we want to resolve on 1: explicitly define that the limit is (pause) undefined<br>
<bkardell_> if tab will put some explanation in, I can +1 on option 1<br>
<bramus> Rossen_: objections?<br>
<bramus> plinss: no<br>
<bramus> Rossen_: explicilty undefined as in “no limit”<br>
<TabAtkins> i'll put in some guidance similar to this https://drafts.csswg.org/css-variables/#long-variables<br>
<bramus> plinss: we will have answer when research is done<br>
<bramus> Rossen_: objections?<br>
<bramus> florian: saying explicitly that there is no limit, you require ???<br>
<bramus> TabAtkins: no, not 'no limit' but undefined<br>
<bramus> … proposed resolution: nesting limit is UA defined<br>
<TabAtkins> proposed resolution: limits on nesting is ua-defined<br>
<bramus> RESOLVED: limits on nesting is ua-defined<br>
</details>
--
GitHub Notification of comment by css-meeting-bot
Please view or discuss this issue at https://github.com/w3c/csswg-drafts/issues/2881#issuecomment-1642793638 using your GitHub account
--
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config
Received on Wednesday, 19 July 2023 21:41:12 UTC