w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.42k stars 647 forks source link

[css-cascade] `revert-layer` keyword in style attribute #6743

Closed xiaochengh closed 2 years ago

xiaochengh commented 2 years ago

What should we do when revert-layer is used in the style attribute?

For example, <div style="color: red; color: revert-layer">. I can think of two options:

  1. Treat style attribute as if in its own layer (though importance is handled differently), so we revert color to the highest author layer
  2. Style attribute is not in any layer, so color: revert-layer is an invalid declaration, and color: red wins the cascade

I prefer option 1 because it is simpler to implement (no parse-time special-casing). And other than an obscure way to cancel other declarations in the style attribute, I'm not sure if there is any real use case of putting revert-layer in the style attribute, so I prefer keeping things simple.

@mirisuzanne

xiaochengh commented 2 years ago

Btw, the resolution should be consistent with VTT STYLE blocks since it has a similar role and the same cascade precedence as the style attribute.

This gives a stronger reason to choose option 1, since banning revert-layer from the VTT STYLE block makes things even more complicated.

mirisuzanne commented 2 years ago

I'm happy with that approach. Agenda+ for discussion/resolution.

tabatkins commented 2 years ago

Elika and I agree that since style attributes are now in their own origin, they (implicitly) sit in their own unlayered layer, unrelated to any author-level layers. (And with no way to declare any additional layers.) So yeah, it would just roll back to the previous origin, the author layer, which is the suggested Option 1 behavior.

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed [css-cascade] revert-layer keyword in style attribute, and agreed to the following:

The full IRC log of that discussion <dael_> Topic: [css-cascade] revert-layer keyword in style attribute
<dael_> github: https://github.com/w3c/csswg-drafts/issues/6743
<dael_> miriam: Similar. Question is what does it do in context where layer is not clear. In this case in style attribute. What happens?
<dael_> fantasai: Had recently switched style attribute to be its own origin. Natural fallout is it would revert the style attribute and nothing else.
<dael_> Rossen_: Soudns reasonable
<dael_> chrishtr: Is that option 1?
<dael_> TabAtkins: Yes
<dael_> Rossen_: Other ideas or objections?
<dael_> RESOLVED: it would revert the style attribute and nothing else
<TabAtkins> basically same as doing 'revert-layer' in unlayered author styles
fantasai commented 2 years ago

I think technically all that's needed here is a note, but we should have a note.

mirisuzanne commented 2 years ago

I don't think we technically made element-attached styles into an origin, though maybe the implications are similar (or we should move them into an origin). Currently they have their own stage of the cascade: https://drafts.csswg.org/css-cascade-5/#style-attr

Maybe a note is still enough? Something similar to the note regarding origins?

mirisuzanne commented 2 years ago

@xiaochengh let us know if the edits in a64a842 satisfy the question here.

xiaochengh commented 2 years ago

Not a strong opinion, but I would like to see if an alternative idea may work.

a64a842 says if we have style="foo: revert-layer !important", it will make all important author styles ignored, and land on the normal level of the top author layer. While this follows from the spec text "as if no rules were specified in the current cascade layer—or between its normal and important levels in the cascade", it's a bit unintuitive to me as now the author layers are half-ignored. In contrast, revert-layer in regular style sheets always makes any layer either fully ignored or fully retained; similarly, revert always makes any origin fully ignored or fully retained. It will also make the implementation in Blink a bit more tricky (though still doable).

My original mental model is that revert-layer makes everything in the current layer and all layer above ignored, so style="foo: revert-layer !important" will revert to the important level of the top author layer. I'm not sure how to specify that nicely, though. The closest thing I've come up with is by revising the definition of revert-layer into:

... so that the specified value is calculated as if no rules were specified in the current cascade layer and any succeeding layer in the layer ordering for this property on this element.

The only problem is that the style attribute is not in the layer ordering, and it cannot be simply treated as a layer or origin due to !important...

Anyway, this is not an opposition. I have some concerns with a64a842 and I'll be happier if my original mental model works, but I can still accept a64a842 as the final spec.

mirisuzanne commented 2 years ago

@xiaochengh to diagram, the current spec works like:

* - where revert-layer is defined
x - additional layers reverted
@ - the top layer to be applied
user agent     !important
style          !important       * 
author layer A !important *     x 
author layer B !important x *   x 
author layer C !important x x * x 
animations                x x x x 
style          normal     x x x x 
author layer C normal     x x x @ 
author layer B normal     x x @   
author layer A normal     x @     
user agent     normal     @

And you're proposing a much more limited scope, where only the current layer is reverted, separately for normal and important versions of a layer? Something like:

user agent     !important
style          !important       * 
author layer A !important *     @ 
author layer B !important @ *     
author layer C !important   @ *   
animations                    @   
style          normal         
author layer C normal
author layer B normal
author layer A normal
user agent     normal

To some extent, both of these will be new and possibly unexpected for authors, who have generally not thought about importance-reversal before now. Authors are likely to try reverting styles in a normal layer by using !important as an override. Although it has some additional implications, the current proposal achieves that goal - using the important layer to revert the normal layer.

I don't think it makes sense to revert the normal and important versions of a layer without touching the layers in-between? If authors want to override a normal rule with revert-layer, avoiding those implications, they can do that using specificity instead.

I'd be interested in @jensimmons perspective on this as well.

xiaochengh commented 2 years ago

And you're proposing a much more limited scope, where only the current layer is reverted, separately for normal and important versions of a layer?

No. My mental model works like:

user agent     !important
style          !important       * 
author layer A !important *     @ 
author layer B !important x *    
author layer C !important x x *  
animations                x x x x 
style          normal     x x x x 
author layer C normal     x x x  
author layer B normal     x x @   
author layer A normal     x @     
user agent     normal     @

That's what I meant by "revert-layer makes everything in the current layer and all layers above ignored". There's no behavioral difference to the current spec text unless there's a revert-layer in style.

In this example, we ignore style and animations, but not anything in author layers A/B/C because they are below style (assuming style is a layer above A/B/C, though this does seem a bit ill-defined).

mirisuzanne commented 2 years ago

@xiaochengh aha, I agree that would likely make sense from an author perspective - and is a more direct interpretation of the resolution. Thoughts, @fantasai?

fantasai commented 2 years ago

Yes. Thinking about this more, I'm not sure what's more intuitive as a model, but certainly if you have written some styles into a layer and are expecting your !important rules to override your normal rules, but we remove only the !important rules, things are likely to break. So I think we have to go with @xiaochengh's model.

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed revert-layer keyword in style attr, and agreed to the following:

The full IRC log of that discussion <TabAtkins> Topic: revert-layer keyword in style attr
<TabAtkins> github: https://github.com/w3c/csswg-drafts/issues/6743#issuecomment-971826541
<TabAtkins> miriam: we talked about this a few weeks ago, and resolved that the revert-layer keyword in a style attr should only revert the style attr.
<TabAtkins> miriam: but that becomes complex and unclear when thinking about how style relates to normal and important layers
<TabAtkins> miriam: we've drawn some diagrams in the thread about different ways this could work
<TabAtkins> miriam: main question is: if you use `revert-layer !important` in a style attr, is that still only revert the style attr, and maybe the animations layer, but no author layers? Or is it also reverting important author layers, taking you down to normal author layers?
<Rossen_> q?
<TabAtkins> miriam: I think we meant the former, but there's some confusion there.
<Rossen_> ack BoCupp
<Zakim> BoCupp, you wanted to say in Florian's example state of the range is changed, and if a range intersects a contained element and its state changes its allowed to invalidate its
<Zakim> ... painting
<TabAtkins> miriam: proposal is that setting `revert-layer !important` in the style attr, it reverts the style-attr layer, the animations layer, and nothing else
<TabAtkins> fantasai: looking at the ascii diagram from Xiaocheng, most revert behaviors cut from the important to the normal origin, and remove everything between those two points
<TabAtkins> fantasai: but because style attr is weird, on top of both normal and important, if we similarly cut thru like that, it'll remove important styles from layers but not the normal styles from those layers
<TabAtkins> fantasai: so i think because it's likely to break expectations of someone writing a layer to remove the important part but not the normal part, i think it's better to do this skipping behavior
<TabAtkins> fantasai: a little concerned about how this plays out in impls, but since Xiaocheng wants it, i think we should go with it
<fantasai> TabAtkins: Looked it over before the meeting, and with you all and Xiaocheng agreeing, I'm happy to go with this
<TabAtkins> Rossen_: objections?
<TabAtkins> RESOLVED: Accept suggested behavior, where `revert-layer !important` in style attr only reverts the style-attr origins and the animations origin, ignoring other author origins
tabatkins commented 2 years ago

The spec already reflected this resolution, so closing. ^_^