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

CSSStyleSheetInit dictionary may have unintended usage #105

Open
nordzilla opened this issue Dec 31, 2019 · 6 comments
Open

CSSStyleSheetInit dictionary may have unintended usage #105

nordzilla opened this issue Dec 31, 2019 · 6 comments
Labels
needs resolution Needs consensus/resolution before shipping

Comments

@nordzilla
Copy link
Contributor

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

emilio commented Jan 4, 2020

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

emilio commented Jan 4, 2020

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

@rakina
Copy link
Member

rakina commented Jan 8, 2020

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

emilio commented Jan 8, 2020

It seems a bit weird because title has no effect in regular Shadow DOM stylesheets already, see w3c/csswg-drafts#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.

@rakina rakina added the needs resolution Needs consensus/resolution before shipping label Jan 20, 2020
@css-meeting-bot
Copy link

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

  • RESOLVED: remove title and alternate from constructor
The full IRC log of that discussion <stantonm> topic: CSSStyleSheetInit dictionary may have unintended usage
<stantonm> github: https://github.com//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
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs resolution Needs consensus/resolution before shipping
Projects
None yet
Development

No branches or pull requests

5 participants