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

Remove duplicate script-blocking style sheet requirement #8759

Merged
merged 4 commits into from
Jan 24, 2024

Conversation

domfarolino
Copy link
Member

Per #3103 (comment), this PR removes a reference to HTML's "script-blocking style sheet" concept from https://drafts.csswg.org/cssom/#requirements-on-user-agents-implementing-the-xml-stylesheet-processing-instruction.

PTAL @annevk. Btw do you have any opinions on what we should do about the same reference to the same term at the bottom of https://drafts.csswg.org/cssom/#requirements-on-user-agents-implementing-the-http-link-header? These days HTML defines the Link header processing model, and as per https://html.spec.whatwg.org/#table-link-relations we only support preconnect and preload, not what CSSOM-1 defines here (and this is noted by the section's "Note yet widely implemented" call-out). IMO it seems best to expand this PR to also remove (at the very least, scoped to this PR) that reference from that section and let HTML work it out if it chooses to.

@annevk
Copy link
Member

annevk commented Apr 26, 2023

We cannot remove this text without equivalent text that style and <link rel=stylesheet> have (which also seems like it could be abstracted more, but I haven't delved into it too deeply):

If element contributes a script-blocking style sheet, append element to its node document's script-blocking style sheet set.

I think the Link header part can be removed in its entirety. I would do that in a separate PR that references whatwg/html#8741.

@domfarolino
Copy link
Member Author

Sure, that sounds good. Do you have a preference as to whether that text should be inside the algorithm steps (I guess close to the end like in HTML) or as this spec had it, outside the algorithm in a normative block below?

@annevk
Copy link
Member

annevk commented May 2, 2023

I don't really. I kinda hope it can be abstracted more at some point so we don't need to state this for each type.

@domfarolino
Copy link
Member Author

That sounds ideal. If it can happen outside of this PR (if that's what you're suggesting), then would you mind reviewing this for now? (I can't formally add anyone as a reviewer I guess...)

@annevk
Copy link
Member

annevk commented May 3, 2023

As clarified via chat this is blocked on the new changes being pushed.

@@ -71,6 +71,9 @@ urlPrefix: https://html.spec.whatwg.org/multipage/
urlPrefix: semantics.html
type: dfn
text: a style sheet that is blocking scripts
Copy link
Member Author

Choose a reason for hiding this comment

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

This is still used by the Link section, so we can probably remove it once we address #8759 (comment).

cssom-1/Overview.bs Show resolved Hide resolved
@domfarolino
Copy link
Member Author

OK, all set for review @annevk.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This doesn't look great I'd say, but I guess it's an improvement.

@domfarolino
Copy link
Member Author

Yeah, I kinda agree. Mostly my priority here is closing out #3103 since it seems bad to have duplicate out-of-sync definitions of the same criteria for script-blocking-ness. Is there anything you'd prefer me to do in this PR to make this all more satisfiable? I guess it's probably better to modify the processing model directly to append the style sheet to the script-blocking style sheet set before we fetch, and the in the response handler remove it from the set etc., but I'm not sure if that matches what you think would be the ideal long-term state for all of this anyways given #8759 (comment).

@annevk
Copy link
Member

annevk commented May 10, 2023

Looking at everyone that invokes "create a CSS style sheet" it seems to me this could be added to "add a CSS style sheet" and removed from all the callers of "create a CSS style sheet".

Those algorithms could also use a lot of cleanup in terms of making their arguments more explicit. And not having this weird "environment encoding" callback setup.

Also, https://html.spec.whatwg.org/#contributes-a-script-blocking-style-sheet has to be cleaned up as it doesn't account for <?xml-stylesheet?> at the moment.

@domfarolino
Copy link
Member Author

I've added the check to "add a CSS style sheet". If you think this looks good I'll make PRs to HTML and https://w3c.github.io/webvtt/#ref-for-create-a-css-style-sheet to remove the reference to the "contributes ..." condition since it appears inside "add", which indeed is called by "create".

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Yeah this looks okay.

cssom-1/Overview.bs Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Sep 18, 2023

I can merge this, but a current editor should probably do a final review. @emilio?

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.

Looks good, thanks!

@svgeesus svgeesus merged commit bdebe3e into w3c:main Jan 24, 2024
1 check passed
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

4 participants