Re: [csswg-drafts] [css-contain] contain: style does not seem useful. (#3280)

The CSS Working Group just discussed `contain: style doesn't seem very suseful`, and agreed to the following:

* `RESOLVED: remove style containment from keywords that aren't 'style'`
* `RESOLVED: mark 'style' at risk`
* `RESOLVED: Republish CR of css-contain once edits are in`

<details><summary>The full IRC log of that discussion</summary>
&lt;fantasai> Topic: contain: style doesn't seem very suseful<br>
&lt;fantasai> s/suseful/useful/<br>
&lt;fantasai> emilio: Very confused what use case contain: style is supposed to solve.<br>
&lt;astearns> github:<br>
&lt;fantasai> emilio: It's hard to implement. Very broken in the only implementation that's shipping.<br>
&lt;fantasai> emilio: I can't see any potential advantage except *maybe* update counters faster in the page?<br>
&lt;futhark> q+<br>
&lt;fantasai> emilio: But that's usually not a perf issue.<br>
&lt;fantasai> TabAtkins: My answer in the issue was that the 'contain' properties in general are to help isolate parts of your document<br>
&lt;fantasai> TabAtkins: perf wins are a benefit, esp layout<br>
&lt;fantasai> TabAtkins: But also helps you isolate parts of your document to make it easier to think about<br>
&lt;fantasai> TabAtkins: It covered the remaining thing that would leak out of a component<br>
&lt;fantasai> TabAtkins: Was a completion exercise in making an isolated component<br>
&lt;fantasai> emilio: Did we decide how counters work in CSS? Are they off DOM tree or flat treee?<br>
&lt;fantasai> TabAtkins: CSS works on flat tree as its "element tree"<br>
&lt;fantasai> TabAtkins: So counters are defined to work on flat tree<br>
&lt;fantasai> TabAtkins:...<br>
&lt;fantasai> emilio: Tests?<br>
&lt;fantasai> TabAtkins: Had some in past, lost them. before wpt<br>
&lt;fantasai> TabAtkins: Might pick up again when working on Lists<br>
&lt;fantasai> dbaron: Counters are part of generated content, thus box tree<br>
&lt;fantasai> TabAtkins: Mental model is that everything is elements, then we turn things into boxes, then we turn things into fragments<br>
&lt;heycam> fantasai: if we had counters working on the not flat tree, then I don't know that list numbering would make much sense, if part of your component was reordering items in a list<br>
&lt;heycam> ... so I think they do need to work on the flat tree<br>
&lt;astearns> q?<br>
&lt;fantasai> TabAtkins: If you take an ol, give it a shadow tree, rearrange items, numbering should reflect final display, not original ol.<br>
&lt;fantasai> fantasai: Right.<br>
&lt;fantasai> futhark: I agree with emilio that there is not much to do with optimization for contain: style<br>
&lt;Rossen> ack futhark<br>
&lt;fantasai> futhark: One useful thing would be if we said that inheritance doesn't happen from 'contain: style' down<br>
&lt;dbaron> At the time we implemented ::before and ::after as part of the box tree, the specs were still pushing towards allowing multiple CSS presentations of a single document.<br>
&lt;fantasai> futhark: Then we can optimize for ?<br>
&lt;fantasai> futhark: If 'contain: style' meant that 'all: initial', then we could optimize getComputedStyle<br>
&lt;fantasai> futhark: Then it wouldn't rely on layout (???)<br>
&lt;fantasai> futhark: Then can do display locking<br>
&lt;fantasai> emilio: ...<br>
&lt;fantasai> TabAtkins: Contain property doesn't block things going into the subtree. It blocks things leaking out.<br>
&lt;fantasai> florian: It's the contain property, not the isolate property<br>
&lt;Rossen> q?<br>
&lt;fantasai> TabAtkins: If it ends up being that we don't want to impelment 'contain: style ' across browsers, we can drop it. It just, it's part of my vision for what 'contain' was about. Last piece necessary to create a seal around the subtree<br>
&lt;fantasai> TabAtkins: There's a bunch of parts of CSS where we have global names etc.<br>
&lt;fantasai> TabAtkins: Maybe we just rely on shadow DOM.<br>
&lt;fantasai> florian: Wanted to ask rego's pov , since he fixed a lot of bugs  on 'contain: style'<br>
&lt;fantasai> emilio: It's very broken to the point that 'contain: style' toggling doesn't do anything dynamically<br>
&lt;rego> there are a few bugs<br>
&lt;fantasai> emilio: Implementing it in Gecko is really hard, and I'd rather not ship something that's broekn.<br>
&lt;fantasai> emilio: My ask is drop it.<br>
&lt;fantasai> Rossen^: So what's your ask, Emilio?<br>
&lt;fantasai> Rossen: So proposal here is to drop 'contain: style' from css-contain<br>
&lt;fantasai> AmeliaBR: Is there any reason this couldn't be added back later, if it turns out to be useful?<br>
&lt;fantasai> florian: Yes and no. It's easy to add stuff. But there's 'contain: strict' which is all of the contains. If we add a new one, then it woudldn't just be an addition but a change.<br>
&lt;fantasai> florian: So we'd have to add a new keyword for all of the contains that includes the new type of containment<br>
&lt;fantasai> chrishtr: If property changes on an element on 'contain: style', and we don't have 'contain: style', is it possible to dirty something else?<br>
&lt;fantasai> TabAtkins: counters and quote property<br>
&lt;fantasai> emilio: The computed style doesn't change.<br>
&lt;fantasai> emilio: The counter values are not stored in computed styles<br>
&lt;fantasai> chrishtr: Does contain: layout say that counters...?<br>
&lt;fantasai> emilio: Style is a bit of misnomer<br>
&lt;fantasai> chrishtr: Regardless of name, need to have this functionality to have complete perf<br>
&lt;fantasai> emilio: My point is that changing counters is not something that is typically a perf bottleneck. Not like changing height, which has lots of effects throughout the doc<br>
&lt;fantasai> emilio: Not soemthing I've seen in a perf profile at all<br>
&lt;fantasai> chrishtr: But if you want to do things like async rendering or display locking<br>
&lt;fantasai> chrishtr: you'd need this, just in case the content changed a counter<br>
&lt;futhark> q+<br>
&lt;fantasai> chrishtr: Whether it should be its own thing, idk<br>
&lt;fantasai> emilio: It's not something that happens frequently. It can have a layout effect across the page.<br>
&lt;fantasai> chrishtr: But unless you lock it, has an effect<br>
&lt;fantasai> emilio: It's same as changing content property<br>
&lt;fantasai> florian: But that doesn't affect outside the element<br>
&lt;fantasai> emilio: ... counters reset can change stuff across document<br>
&lt;fantasai> chrishtr: Point of containment is that you can reason about the relationship of work between stuff in the contained subtree fvs rest of document<br>
&lt;fantasai> chrishtr: It's small but a real footgun?<br>
&lt;fantasai> emilio: That's fair, but we still don't have a great model for counters<br>
&lt;fantasai> emilio: Makes it seem like counters work in the style tree.<br>
&lt;fantasai> emilio: Which isn't true<br>
&lt;fantasai> emilio: We need tests for like shadow DOM, need tests for display: contents, for all these edge cases which change how counters are affected by style containment<br>
&lt;fantasai> chrishtr: Agree we need tests<br>
&lt;fantasai> emilio: and a proper spec<br>
&lt;fantasai> dbaron: We had an interim agreement to add 'counter-set'<br>
&lt;fantasai> dbaron: we never added<br>
&lt;fantasai> TabAtkins: it's in a spec, it's in Lists<br>
&lt;fantasai> TabAtkins: just never past WD<br>
&lt;fantasai> emilio: ... spec, not in implementations<br>
&lt;fantasai> emilio: Even in Blink, renumbering lists<br>
&lt;fantasai> emilio: If element is display: contents; contain: strict; if you add a list item inside of it?<br>
&lt;fantasai> emilio: You have a &lt;ol>&lt;div style="contain: strict">&lt;li>&lt;/div>&lt;/ol><br>
&lt;fantasai> emilio: Right now if you change the div's display, it is buggy<br>
&lt;fantasai> TabAtkins: Our entire counters implementation has issues<br>
&lt;fantasai> eae: Spec is correct, but our impl is incorrect<br>
&lt;fantasai> emilio: Given state of impls, implementing 'contain: style' is not worth it<br>
&lt;fantasai> TabAtkins: Fact that built-in lists might be magical is a separate issues<br>
&lt;fantasai> emilio: If lists are magical, then we have another leak<br>
&lt;fantasai> dbaron: We have a patch to Gecko to use counters for lists, so might fix that<br>
&lt;fantasai> chrishtr: Sounds like all the browsers are sad, and I think we agree that there is a real footgun even if a small one<br>
&lt;fantasai> chrishtr: So I think we should add proper tests and fix things to not be sad<br>
&lt;fantasai> chrishtr: Then if that's the case, come back to you about this.<br>
&lt;fantasai> dholbert: Sounds like use case is lazily rendering the subtree<br>
&lt;fantasai> dholbert: If counters and quotes are the only things that would have an effect, maybe implementations can just add checks for those properties<br>
&lt;fantasai> dholbert: They're quite rarely used<br>
&lt;fantasai> dholbert: Might be easier to just disable perf optimizations in cases where they're used rather than implementing 'contain: style'<br>
&lt;gregwhitworth> q+<br>
&lt;astearns> ack futhark<br>
&lt;fantasai> futhark: I think Chris you had some thoughts... I'm confused about isolation vs containment<br>
&lt;Rossen> ack futhark<br>
&lt;fantasai> futhark: For perf, you probably want isolation<br>
&lt;fantasai> futhark: If you modify DOM outside something that's contaiend<br>
&lt;fantasai> futhark: It can affect sibling selectors that affect the contained subtree<br>
&lt;fantasai> emilio: Which can set 'contain'<br>
&lt;fantasai> TabAtkins: Nothing about contain is about isoaltion<br>
&lt;fantasai> TabAtkins: You can't have layout isolation<br>
&lt;fantasai> TabAtkins: You can't use percentage or auto or anything. It has to be 100% manually-specified or nothing.<br>
&lt;fantasai> TabAtkins: You have a subree that you can ignore when changes happen inside it. That's the use case we're designing for<br>
&lt;fantasai> chrishtr: Points about % sizing and isolation vs something else is a complicated subject, and the more you go twards one end the more you lose use cases  or perf<br>
&lt;fantasai> chrishtr: needs investigation<br>
&lt;fantasai> chrishtr: Not only is 'contain: style' poorly implemented, the other contain modes while implemented corretly don't actually improve perf<br>
&lt;florian> q+<br>
&lt;gregwhitworth> ack me<br>
&lt;fantasai> emilio: Even though containment is designed to ignore a subtree, you can still have e.g. :empty { contain: strict } and that div's contained but you insert a child in that div, and you are no longer contained<br>
&lt;fantasai> emilio: contain can be dependent on DOM content sof an element<br>
&lt;fantasai> emilio: There are sleectors already that depend on contents of the element<br>
&lt;fantasai> chrishtr: ...<br>
&lt;fantasai> TabAtkins: You cannot have selectors that depend on style<br>
&lt;fantasai> TabAtkins: Filing an issue won't solve it.<br>
&lt;fantasai> TabAtkins: We can't do that.<br>
&lt;Rossen> q?<br>
&lt;xfq> q- fr, fl<br>
&lt;Rossen> ack all the florians<br>
&lt;fantasai> florian: Mentioned about testing and things, at the moment we have reasonably extensive testing of normal counters<br>
&lt;fantasai> florian: If lists are fake counters, that's not tested separately<br>
&lt;fantasai> florian: I couldn't find tests for shadow DOM and display: contents<br>
&lt;fantasai> florian: BUt for normal counters, should be well-covered<br>
&lt;Rossen> q?<br>
&lt;fantasai> TabAtkins: Given large number of inconsistencies in lists from when I was testing them awhile back....<br>
&lt;fantasai> florian: Checking for what ti's supposed to do to counters.<br>
&lt;fantasai> florian: If you toggle contain on/off, we don't have tests for that.<br>
&lt;fantasai> florian: But effect of contain on counters, we do have tests<br>
&lt;fantasai> TabAtkins: Thanks for test suites<br>
&lt;fantasai> florian: Thank gtalbot<br>
&lt;fantasai> Rossen: Hearing pushback from Mozilla<br>
&lt;fantasai> Rossen: Not hearing much from other vendors<br>
&lt;fantasai> Rossen: What do we want to do next?<br>
&lt;fantasai> fantasai: Rename it, obviously.<br>
&lt;fantasai> Rossen: Or we can drop it<br>
&lt;fantasai> emilio: Can we drop contain: style and if we decide it's a thing we want<br>
&lt;fantasai> TabAtkins: Drop style now, figure it out later, decide what to do about 'strict' and whether to make it stricter or add a stricter value?<br>
&lt;fantasai> fantasai: Might be able to slip it in, because unlikely to be dependent on leaking counters/quotes<br>
&lt;fantasai> TabAtkins: Quotes definitely rare. Unsure about counters.<br>
&lt;fantasai> emilio: ...<br>
&lt;fantasai> Rossen: We could try dropping style now, and see if counters surface as a prominent use case, then deal with it<br>
&lt;fantasai> TabAtkins: What do you think, Chris<br>
&lt;fantasai> chrishtr: Drop it from the spec, but leave it implemented in one browser?<br>
&lt;fantasai> TabAtkins: No, that'd be a spec violation<br>
&lt;fantasai> gregwhitworth: It's implemented and shipping in a browser, but it doesn't work<br>
&lt;fantasai> TabAtkins: It does for real counters, if you have no dynamic changes<br>
&lt;fantasai> hober: It's up to them what they ship in the browser.<br>
&lt;fantasai> Rossen: So would there be an objection to dropping 'contain: style'?<br>
&lt;hober> s/in the browser/in their browser. It's up to us what we have in the spec./<br>
&lt;fantasai> emilio: My point for dropping it now is that it's the same kind of risky change to change Blink's lists to use real counters as it would be to drop 'style' / implement later<br>
&lt;fantasai> emilio: The case of having 'contain: style' that spans across a list<br>
&lt;fantasai> emilio: or counters inside a 'contain: style' subtree<br>
&lt;fantasai> emilio: Until Lists work that way, you're going to break something<br>
&lt;fantasai> TabAtkins: If our counter impl is already broken and weird, so it probably doesn't matter if we leave the code in?<br>
&lt;fantasai> chrishtr: What would likely happen is that we wouldn't remove the feature, we'd fix it<br>
&lt;fantasai> chrishtr: I'd rather fix it in Chrome and work through the spec issues than drop it<br>
&lt;fantasai> chrishtr: What we do with the spec in teh meantime<br>
&lt;fantasai> eae: It's something we really want to do in Blink, and we do realize that the current state of thing where e.g. Lists don't work that way<br>
&lt;fantasai> eae: So it might make sense to drop it for now<br>
&lt;fantasai> florian: If we drop now and add back later gets us to strict + stricter<br>
&lt;fantasai> florian: I'm not bikeshedding the stricter<br>
&lt;fantasai> emilio: We also want to ship containment<br>
&lt;fantasai> emilio: But we really don't want to implement 'contain: style' under the current state of affairs<br>
&lt;fantasai> emilio: Given it needs more work in specs and implsmentations<br>
&lt;TabAtkins> q+<br>
&lt;fantasai> emilio: It would make sense not to block shipping containment on 'contain: style'<br>
&lt;fantasai> chrishtr: Was there a compat issue?<br>
&lt;fantasai> emilio: No, but we don't want to willfully violate the spec.<br>
&lt;fantasai> emilio: We don't want to parse a value and then not implement it<br>
&lt;fantasai> (That is in fact a violation of the CSS spec)<br>
&lt;fantasai> TabAtkins: Is it just about built-in lists or all counters/<br>
&lt;fantasai> emilio: It's generally hard to implement in our counters implementation<br>
&lt;xfq> ack Tab<br>
&lt;fantasai> TabAtkins: I'm wondering how difficult it is from doing a counter-reset on all possible counters?<br>
&lt;fantasai> dbaron: I think in our counters implementation, the right thing to do is just create a new counter-manager for an element that has 'contain: style'<br>
&lt;fantasai> dbaron: And then it's reasonable<br>
&lt;fantasai> emilio: And then dynamic changes, you'd have to reframe<br>
&lt;fantasai> dbaron: You'd have to reframe anywya<br>
&lt;fantasai> TabAtkins: You can make it as slow as you want to change 'contain'. It's not a use case to flip it dynamically.<br>
&lt;fantasai> florian: Unless I missed something, counters have a definition problem. CSS 'contain: style' doesn't.<br>
&lt;fantasai> florian: There's a dependency, but I think implementation and spec being wrong is an overstatement<br>
&lt;fantasai> TabAtkins: Lists spec having bugs is adequate to say there's a spec problem.<br>
&lt;fantasai> florian: My point is, if there's a problem with the contain spec there needs to be an issue so I can fix it. But there isn't.<br>
&lt;fantasai> TabAtkins: ...<br>
&lt;fantasai> emilio: What would be the best way for Mozilla to ship contain: layout<br>
&lt;fantasai> florian: Could just not ship strict yet, then ...<br>
&lt;fantasai> TabAtkins: People are reasonably encouraged to turn on strict, so technically the right way to do it, but don't think it's good advice<br>
&lt;fantasai> q+<br>
&lt;rego> neither contain:strict neither contain:content (as both include style)<br>
&lt;xfq> ack fantasai<br>
&lt;heycam> fantasai: I think the right way to go is to ship everything except the style value itself<br>
&lt;heycam> ... and then at some future point, incorporate that into strict<br>
&lt;heycam> ... in the majority of cases it's not going to have an effect, so I don't think it's something to be too worried about<br>
&lt;fantasai> emilio: it would be nice if Blink didn't parse 'style' either, and we could do that change together later<br>
&lt;fantasai> florian: When you say don't do style, did you also mean change tehavior of strict later when we add it?<br>
&lt;heycam> fantasai: yeah, we will eventually change the behavior<br>
&lt;fantasai> fantasai: Yes<br>
&lt;fantasai> chrishtr: You said it's not so easy to implement contain: style<br>
&lt;fantasai> chrishtr: How hard is it?<br>
&lt;fantasai> emilio: How hard is it?<br>
&lt;fantasai> chrishtr: Couple months of work?<br>
&lt;fantasai> emilio: It's work, that somebody needs to do<br>
&lt;fantasai> TabAtkins: Given taht I do have a direction for naming in the future<br>
&lt;fantasai> TabAtkins: I'm OK with dropping style for now<br>
&lt;fantasai> TabAtkins: Dropping from definition of strict, dropping as a value<br>
&lt;fantasai> TabAtkins: And then we add 'all' later, that does all 4<br>
&lt;fantasai> TabAtkins: Do that once our implementation is good enough to be meaningful for style<br>
&lt;fantasai> TabAtkins: And Mozilla can also fix<br>
&lt;fantasai> emilio: And add a bunch of tests :)<br>
&lt;fantasai> chrishtr: So what you're suggesting is to refactor spec so that easier to incrementally add in the future<br>
&lt;fantasai> TabAtkins: Right, drop 'style' and add it back in a future level of the spec<br>
&lt;fantasai> chrishtr: What if we do the other things you mention and leave style?<br>
&lt;fantasai> TabAtkins: Drop style containiment from strict and content?<br>
&lt;fantasai> dholbert: That would make us reject 'contain: style' but...<br>
&lt;fantasai> chrishtr: CSS parsers can ignroe keywords they don't understand?<br>
&lt;fantasai> florian: If you as an author writes 'contain: style size" it'll be lost<br>
&lt;Rossen> q?<br>
&lt;fantasai> florian: but if you write 'contain: size; contain: style size' then you just lose the second declaration keep the first<br>
&lt;fantasai> TabAtkins: Most of the time we recommend strict anyway<br>
&lt;fantasai> florian: Seems like least harmful way to do this<br>
&lt;fantasai> chrishtr: And then we'll fix our bugs and allow ppl to opt in<br>
&lt;fantasai> Rossen: Any objections?<br>
&lt;fantasai> RESOLVED: remove style containment from keywords that aren't 'style'<br>
&lt;fantasai> RESOLVED: mark 'style' at risk<br>
&lt;fantasai> RESOLVED: Republish CR of css-contain once edits are in<br>
&lt;fantasai> chris: Ping me when you have a DoC and file a transition request<br>

GitHub Notification of comment by css-meeting-bot
Please view or discuss this issue at using your GitHub account

Received on Tuesday, 26 February 2019 19:55:32 UTC