w3c / csswg-drafts

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

[cssom-1] circular imports and CSSImportRule #4287

Open bathos opened 5 years ago

bathos commented 5 years ago

I was curious about how circular and repeated imports behave. I checked the definition of @import in CSS Cascading and Inheritance Level 3.

My understanding from reading that is that all valid import atrules cause the contextual stylesheet to behave as if the referenced sheet’s rules appeared in place of the atrule. I checked Chrome and Firefox and they seemed to confirm this and were happy to deal with circular imports in various odd combos. That makes sense — although I’d found no spec text specifically addressing circularity, it seems implicitly permitted because I think the implementation only needs to ‘insert’ them once, in the ‘furthest-down’ place they’re referenced, in order to achieve the specified behavior. Due to the nature of the cascade, inserting them just once in that position is equivalent to actually importing them in all positions — which, if actually done, would potentially lead to endless recursion.

When the same style sheet is imported or linked to a document in multiple places, user agents must process (or act as though they do) each link as though the link were to an independent style sheet.

So styling behavior made sense and was consistent, but things got weird when I started peeking at the CSSOM. It appears Chrome and Firefox don’t agree about what should happen to the representations of CSSImportRules when circular imports are involved.

Here is a minimal example, involving just one stylesheet importing itself:

https://rogue-onion.glitch.me/

Neither FF nor Chrome take the approach of actually implementing a circular graph for this in the CSSOM, but they ‘clip’ the graph in different places. In Chrome, the CSSImportRule has null as its stylesheet value. In Firefox, the CSSImportRule has a stylesheet value, but it is a unique object reference from the parent stylesheet (which is ostensibly the same one? or is it...) and it has an empty CSSRuleList.

Chrome’s behavior violated the Web IDL type definition in which CSSImportRule.prototype.styleSheet is not nullable. In fact there’s even a lil callout about this:

Note: An @import at-rule always has an associated CSS style sheet.

I can’t find anything that specifies the behavior I observed in Firefox, either, though I also haven’t found anything that definitely contradicts it.

Is the behavior of CSSImportRule when there are circular imports underspecified, or specified in a way that doesn’t match web reality?

tabatkins commented 5 years ago

Underspecified, I guess. The current definition just assumes an infinite object graph will be created, but if nobody does that (and their behavior is def reasonable), then we should change this.

Which behavior we use, I don't particularly care about, but I'm inclined towards the Chrome behavior, rather than faking a stylesheet and just making it empty.

bathos commented 5 years ago

I expected the infinite graph, though maybe ‘as though the link were to an independent style sheet’ was considered to preclude that (I think that line is just about the effective cascade behavior, but both Chrome and FF seem to agree that every import === a unique CSSStyleSheet or null).

One difference between the FF and Chrome approaches is that FF does allow disabling the sheet — which is meaningless unless you subsequently remove imports that would make that sheet potentially be seen as having rules again. (I can’t think of any real world use cases where this matters, just noting since it seems to be a functional difference.)

bathos commented 5 years ago

Edge does a third thing:

document.styleSheets[0].cssRules[0].styleSheet.cssRules[0].styleSheet.cssRules[0].styleSheet.cssRules[0]; // CSSImportRule

document.styleSheets[0].cssRules[0].styleSheet.cssRules[0].styleSheet.cssRules[0].styleSheet.cssRules[0].styleSheet; // undefined

It looks like the infinite graph at first, but at a depth of five the styleSheet property returns undefined. :)

tabatkins commented 5 years ago

That's worse than either of the other two options. ^_^

tabatkins commented 5 years ago

@emilio Yo, which behavior do you prefer, Chrome's "first circular stylesheet is null" or Firefox's "first circular stylesheet is empty" behavior? I presume nobody wants old Edge's "sixth circular stylesheet is undefined" behavior. ^_^

emilio commented 5 years ago

Do Blink / WebKit null out the stylesheet for failing loads? Or only for cyclic ones?

I think I'd like to preserve the "Import rule must have a stylesheet, even if empty" behavior, but I'd be ok changing it. Though as far as I can tell, in Gecko it wouldn't be so straight-forward to change only the cyclic case. We treat the "there's a cycle" the same way as "the load failed".

emilio commented 5 years ago

If Chrome / WebKit do keep the stylesheet nulled out for failing loads, then changing Gecko to do the same may be ok (indeed I think we used to do that, and the only reason I changed behavior there was because of that spec sentence).

I think cyclic loads and failing loads should be consistent.

emilio commented 5 years ago

(And sorry for the lag here, was OOO until now :))

tabatkins commented 5 years ago

(And sorry for the lag here, was OOO until now :))

np, that's why i tagged you, so hopefully it would show up higher-priority in your inbox. ^_^

Do Blink / WebKit null out the stylesheet for failing loads? Or only for cyclic ones?

For Blink:

I'd be okay with the behavior of having a stylesheet, but making it self-censor like a failed or cross-origin sheet, yeah.