WICG / construct-stylesheets

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

CSSStyleSheetInit dictionary may have unintended usage #105

Open nordzilla opened 4 years ago

nordzilla commented 4 years ago

The CSSStyleSheetInit dictionary specified here in the spec may lead to some configurations.

For example, setting title = "" and alternate = true is a possible configuration using this dictionary; however an alternate style sheet must have a specified title.

Also, the CSSOM spec notes that media, title, and alternate are specified when the sheet is created, but it does not say this about disabled. Should we be allowed to specify disabled explicitly as one of the constructor options?

At the very least, I think that the constructor should throw if title = "" and alternate = true.

emilio commented 4 years ago

More generally, alternate is only useful to determine the initial disabled state. So if you can specify disabled then it's not great.

There's another quirk of regular sheets that isn't great, which is that if you haven't seen a sheet with title before, the "preferred" title becomes that title, if non-empty. I don't think we necessarily want that for constructable stylesheets, but worth thinking about?

emilio commented 4 years ago

It doesn't help that Blink's / WebKit's handling of alternate stylesheets is so broken though, see https://github.com/whatwg/html/issues/3840.

rakina commented 4 years ago

Hmm, interesting. The MDN page does say an alternate style sheet must have a specified title, but the spec text here doesn't really do anything with stylesheets whose alternate flag is set and an empty title, so I'm not sure if we should throw there. However looking closely I think our current draft spec in this repro isn't hooked up to any of the steps here https://drafts.csswg.org/cssom/#add-a-css-style-sheet (see reply below too)

There's another quirk of regular sheets that isn't great, which is that if you haven't seen a sheet with title before, the "preferred" title becomes that title, if non-empty. I don't think we necessarily want that for constructable stylesheets, but worth thinking about?

I think we can get this for free too if we hook up the construction with "create a CSS style sheet". Is there any reason why we don't want this behavior?

emilio commented 4 years ago

It seems a bit weird because title has no effect in regular Shadow DOM stylesheets already, see https://github.com/w3c/csswg-drafts/issues/2646...

I think title and alternate should just be removed, and we should just leave disabled which sets the final value of the disabled flag.

css-meeting-bot commented 4 years ago

The CSS Working Group just discussed CSSStyleSheetInit dictionary may have unintended usage, and agreed to the following:

The full IRC log of that discussion <stantonm> topic: CSSStyleSheetInit dictionary may have unintended usage
<stantonm> github: https://github.com/WICG/construct-stylesheets/issues/105
<chris> rrsagent, here
<RRSAgent> See https://www.w3.org/2020/01/23-css-irc#T14-21-21
<stantonm> heycam: css stylesheet interface has constructor, allows to set various things on the stylesheet
<stantonm> ... seems like allows combinations of things that are not valid
<stantonm> ... specifically creating stylesheet with empty title
<stantonm> emilio: only use for title and alternate is compute disabled bit
<stantonm> emilio: don't see anything useful
<stantonm> TabAtkins: so your saying we should just remove it?
<stantonm> emilio: yes
<stantonm> chris: don't need it not necessary
<stantonm> emilio: title attribute sets preferred stylesheet list, why it doesn't apply in shadow dom
<stantonm> ... don't know what it means for ransom constructible stylesheet
<stantonm> ... just use .disabled attribute
<stantonm> heycam: remove constructor, and force .disabled?
<stantonm> emilio: title doesn't work in shadow dom
<stantonm> ... reason title is useful is because browsers provide UI for switching
<stantonm> ... .disable is still fine
<stantonm> christ: used to be more visible for, but some functionality moved away
<stantonm> RESOLVED: remove title and alternate from constructor
Randy-Buchholz commented 4 years ago

Since title is used to define Style Sheet Set doesn't removing it from the constructor and keeping it read-only impact the whole concept of sets? A sheet created with new CSSStyleSheet() can never get a title and therefore can't participate in sets. Maybe open up the constructor and specify guards to address invalid conditions/combinations?