WICG / construct-stylesheets

API for constructing CSS stylesheet objects
Other
138 stars 25 forks source link

Spec requires adopted stylesheets to be included in styleSheets property but doesn't require them to be used for styling #118

Open othermaciej opened 4 years ago

othermaciej commented 4 years ago

The spec requires adopted stylesheets to also be included in styleSheets property, apparently inadvertently.

The Constructable Stylesheets spec says:

The user agent must include all style sheets in the DocumentOrShadowRoot's adopted stylesheets inside its document or shadow root CSS style sheets. [1]

The phrase "document or shadow root CSS style sheets" links to the definition of that term in CSSOM (thus effectively monkeypatching CSSOM, since the CSSOM definition is written as if all stylesheets come from <style> or <link rel=stylesheet> elements.

CSSOM in turn defines the styleSheets DOM property thus:

The styleSheets attribute must return a StyleSheetList collection representing the document or shadow root CSS style sheets. [2

This links to the very same definition of document or shadow root CSS style sheets. Therefore, the combination of the specs would require adopted style sheets to also appear in the styleSheets property.

Chrome does not implement this behavior. And per one of the spec authors, this was not intended.

The spec should be updated that the stylesheets will be used, but will not appear in styleSheets. I am not sure how to specify this. In fact, I'm not sure what the spec does now actually requires adopted stylesheets to be used for styling. I don't think the places that require styling even refer to this CSSOM definition. That would likey require monekypatching this spot in CSS Cascading and Inheritence. Best to ask an expert on CSS specs though.

rakina commented 4 years ago

cc @tabatkins @emilio

rakina commented 4 years ago

Thanks for looking into this!

The spec should be updated that the stylesheets will be used, but will not appear in styleSheets. I am not sure how to specify this.

For this, can we just change the styleSheets attribute to contain only stylesheets that are not part of any DocumentOrShadowRoot's adopted style sheets?

In fact, I'm not sure what the spec does now actually requires adopted stylesheets to be used for styling. I don't think the places that require styling even refer to this CSSOM definition. That would likey require monekypatching this spot in CSS Cascading and Inheritence. Best to ask an expert on CSS specs though.

Yeah I'm not totally sure on this one too.. I wonder if that's the only place where we actually talk about all the stylesheets in a document.

tabatkins commented 4 years ago

Hm, Cascade doesn't define which stylesheets are applied to the page, it just dictates their place in the "Order Of Appearance" step. (And adopted sheets count as "sheets independently linked by the originating document", so they're already covered there; the CS spec then covers the relative ordering within that step, under the "as determined by the host document language" caveat.)

For this, can we just change the styleSheets attribute to contain only stylesheets that are not part of any DocumentOrShadowRoot's adopted style sheets?

I think so, yes; CSSOM just needs patching there to open up the concept a little bit. All the more reason to go ahead with our plans to merge this into CSSOM directly. ^_^

othermaciej commented 4 years ago

I think the intent of "sheets independently linked by the original document" is <link rel=stylesheet> and similar constructs. In this case the host document language is HTML and it sort of hooks into this: "The stylesheet keyword may be used with link elements. This keyword creates an external resource link that contributes to the styling processing model."

At the very least CS should have similar language.

But it's kind of a stretch to consider a programmatically constructed and dynamically attached object to be a "style sheet[s] independently linked by the originating document", so it would provide greater clarity to either patch or monkeypatch Cascade to include these in the order.

I don't think "document or shadow root stylesheets" controls what stylesheets are applied. It's only use is for the styleSheets property afaict. So instead of additionally patching styleSheets, a simpler solution would be to not patch "document or shadow root stylesheets".

domenic commented 4 years ago

Here is my proposal for how to make this whole situation more rigorous. (Note: I am not volunteering to spec this, apart from the observable array parts.)

tabatkins commented 4 years ago

Yup, that all sounds right in line with what I thought we'd do too.

othermaciej commented 4 years ago

Probably best not to make Cascade depend on CSSOM (that seems backwards for some reason). Perhaps better if CSSOM can reference a Cascade concept.

A further interesting discovery CSS Cascade does not seem to have any awareness of shadow roots. This likely needs to be fixed.

tabatkins commented 4 years ago

That's because CSS Scoping defines the implications of shadow trees on the Cascade.

Maybe more ideal to centralize that at this point, but the definitions exist at least

othermaciej commented 4 years ago

I am concerned that implementing this spec interoperably would require violating the letter of what the spec says. Would it be helpful to post a PR that temporarily writes this requirement of the spec in looser language, so that it's not nominally requiring the opposite of what is actually intended? (I don't have the bandwidth to do the really rigorous thing proposed in https://github.com/WICG/construct-stylesheets/issues/118#issuecomment-588474978 )