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

[cssom-1] Refactor CSSStyleSheet constructor and replaceSync to allow steps to be referenced from HTML spec #6411

Merged
merged 4 commits into from Jul 7, 2021

Conversation

dandclark
Copy link
Contributor

[cssom-1] Refactor CSSStyleSheet constructor and replaceSync to allow steps to be referenced from HTML spec

The CSS module scripts feature is being specified in HTML using several algorithms defined in cssom-1 on the CSSStyleSheet interface. In the review of the CSS module scripts HTML spec PR whatwg/html#4898, there was a concern that referencing the CSSStyleSheet methods directly implies that if these methods were overridden by user script, the UA should call the user script instead.

This change pulls the steps for the CSSStyleSheet constructor and replaceSynce method into separate algorithms so that they can be safely referenced from the HTML spec.

@dandclark
Copy link
Contributor Author

This refactoring doesn't change any behavior, so not sure if a CSSWG resolution is necessary. Happy to discuss in a meeting if necessary.

@mfreed7, FYI. I'd drafted an earlier version of this at WICG/construct-stylesheets#138 but missed the train on #6304. 😊

@mfreed7
Copy link

mfreed7 commented Jul 1, 2021

This change looks good to me! Thanks for cleaning it up in the process, and sorry for the churn on WICG/construct-stylesheets#138.

@mfreed7
Copy link

mfreed7 commented Jul 1, 2021

Gentle ping @emilio and @tabatkins - this PR is blocking the CSS Modules PR.

@emilio
Copy link
Collaborator

emilio commented Jul 5, 2021

Sorry for the lag, I was on vacation until today. Will take a look ASAP, thanks.

@emilio
Copy link
Collaborator

emilio commented Jul 5, 2021

So I'm curious, why do you need the initialization to be separate from construction? That means that we need to define what a constructed but not initialized stylesheet does. Why can't it just use the CSSStyleSheet constructor?

The "synchronously replace the rules of a CSSStyleSheet" bit looks great though.

@dandclark
Copy link
Contributor Author

So I'm curious, why do you need the initialization to be separate from construction? That means that we need to define what a constructed but not initialized stylesheet does. Why can't it just use the CSSStyleSheet constructor?

I don't think it's necessary for them to be separate. I've pushed a change to move the creation and initialization into the same set of steps.

Thanks!

Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that nit, it looks basically good to me, thanks!

cssom-1/Overview.bs Outdated Show resolved Hide resolved
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 this pull request may close these issues.

None yet

3 participants