Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
othermaciej opened this issue Feb 4, 2020 · 9 comments · Fixed by w3c/csswg-drafts#6431

Comments

@othermaciej
Copy link

othermaciej commented Feb 4, 2020

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.

@othermaciej othermaciej changed the title Spec requires adopted stylesheets to also be included in styleSheets property, apparently inadverntently Spec requires adopted stylesheets to also be included in styleSheets property, apparently inadvertently Feb 4, 2020
@othermaciej othermaciej changed the title Spec requires adopted stylesheets to also be included in styleSheets property, apparently inadvertently Spec requires adopted stylesheets to be included in styleSheets property but doesn't require them to be used for styling Feb 4, 2020
@rakina
Copy link
Member

rakina commented Feb 4, 2020

cc @tabatkins @emilio

@rakina
Copy link
Member

rakina commented Feb 4, 2020

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

domenic commented Feb 19, 2020

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.)

  • Update https://drafts.csswg.org/cssom/#css-style-sheet-collections so that each DocumentOrShadowRoot has an associated CSS style sheets, which contains all Link-header derived and <link> or <style> derived style sheets.
    • Remove the "document or shadow root CSS style sheets" concept
    • Optionally make the manner in which this list's contents are determined and/or updated more rigorous. Right now it seems to be a mix of declarative (e.g. for Link-header derived ones) and imperative (for anything that goes through "add a CSS style sheet"). But the imperative stuff is pretty vague, given "at the appropriate location". Maybe adding/removing should be imperative but the ordering should be given by a declarative algorithm that operates on the sheet's owner node + something else for Link headers. Or maybe Link headers should be stored in a separate list from the "CSS style sheets" list, since they are immutable throughout the document lifetime?
  • Update Introduce the observable array type (proxy-based) whatwg/webidl#840 so that every observable array extended attribute has a backing list. Then, we can refer to a DocumentOrShadowRoot's adoptedStyleSheets's backing list, to get a real-time updated list of CSSStyleSheet objects.
  • State somewhere (Cascade, or HTML, or CSSOM??) that the stylesheets with contribute to the cascade for a given DocumentOrShadowRoot are the concatenation of the DocumentOrShadowRoot's CSS style sheets plus the DocumentOrShadowRoot's adoptedStyleSheets's backing list.
    • If necessary, make it clear that changes to the contents of either list causes a recomputation of everything relevant.

@tabatkins
Copy link
Contributor

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

@othermaciej
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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 #118 (comment) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants