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

Allow integrity attribute on rel=preload links #4699

Merged
merged 2 commits into from
Jun 21, 2019

Conversation

domfarolino
Copy link
Member

@domfarolino domfarolino commented Jun 14, 2019

This PR "allows" the integrity attribute to be used by <link rel=preload>s. Previously, the integrity attribute would only be considered for <link rel=stylesheet>s, by spec. Link preloads, in this specification, use the default fetch and process the linked resource algorithm, so this should be the only change left.

I am fine with adding support to modulepreload too, but I think that should be a separate PR, since this is tightly coupled with the consensus we have from the preload folks.

Corresponding w3c/preload PR & issue: w3c/preload#127 & w3c/preload#137

/cc @kinu @nyaxt @yutakahirano


/semantics.html ( diff )

@domfarolino domfarolino added needs tests Moving the issue forward requires someone to write tests impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation labels Jun 14, 2019
@domfarolino domfarolino requested review from yoavweiss and annevk June 14, 2019 05:17
@domenic
Copy link
Member

domenic commented Jun 14, 2019

This is a document conformance change, so I don't think we usually require tests for this (although @sideshowbarker has been maintaining conformance checker tests). That said, if there are no tests for integrity="" on rel=preload yet, then we should indeed do those ASAP...

source Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Jun 14, 2019

@domfarolino so yeah, looking at the processing model @domenic is correct and HTML already enforces integrity for everything that uses "default fetch and process the linked resource", probably including modulepreload? It would be good to have that tested.

@domfarolino
Copy link
Member Author

probably including modulepreload? It would be good to have that tested.

Ah, actually modulepreload has its own overload of the "default fetch and process the linked resource" algorithm, but it also honors the integrity attribute as well, so I can include tests for that too I suppose. I'll also just update this PR to allow integrity for rel=modulepreload as well for document conformance, as it's simple enough. Thanks.

@domfarolino
Copy link
Member Author

What are the next steps for getting this merged? Tests are WIP (branch here), and will land before Chrome's implementation, but not currently a blocker for the spec as Domenic and Anne have mentioned. The change seems pretty non-controversial. Currently it is technically blocking Blink's Intent to Implement and Ship, so I'm wondering if there's anything else I can do here?

@domfarolino
Copy link
Member Author

Ah, just realized I never made the modulepreload change I mentioned :) Done!

@domenic
Copy link
Member

domenic commented Jun 21, 2019

I think we were being a little over-strict by using your document conformance change pull request as an opportunity to ask you to do processing-model tests. I agree processing-model tests should not be a document-conformance-PR blocker. But, hey, you did the tests anyway :). The WIP branch is good enough for me.

I'll merge this now!

@domenic domenic merged commit 0ec1925 into master Jun 21, 2019
@domenic domenic deleted the enable-integrity-for-preloads branch June 21, 2019 01:12
@sideshowbarker
Copy link
Contributor

heads-up @whatwg/documentation

@domenic domenic removed the needs tests Moving the issue forward requires someone to write tests label Jun 21, 2019
@chrisdavidmills
Copy link

Documentation need recorded at https://trello.com/c/6YAMGL3B/132-html-link-element-fx-68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document conformance impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation
Development

Successfully merging this pull request may close these issues.

6 participants