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

Fix for bikeshed linking error #285

Closed
wants to merge 7 commits into from

Conversation

andypaicu
Copy link
Collaborator

@andypaicu andypaicu commented Jan 12, 2018

@@ -4305,7 +4305,7 @@ <h3 id="strict-dynamic-usage">
</pre>

This will generate a request for `https://cdn.example.com/script.js`, which
will not be blocked because of the matching {{NoncedElement/nonce}} attribute.
will not be blocked because of the matching {{nonce}} attribute.
Copy link
Member

Choose a reason for hiding this comment

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

Hrm. Why do we need to do that? Shouldn't it be finding https://html.spec.whatwg.org/#dom-noncedelement-nonce?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope it does not, I don't think that particular section defines the nonce attribute for NoncedElement.

Can you point me to the source file for that section of the html spec?

Copy link
Member

Choose a reason for hiding this comment

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

Look for <dfn><code data-x="dom-NoncedElement-nonce">nonce</code></dfn> in https://github.com/whatwg/html/blob/master/source. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no idea what data-x actually does, but I suspect that's irrelevant as it's part of the code tag (is it for the section name perhaps?).

The dfn tag doesn't have a for attribute so I don't see how it will become part of the NoncedElement interface. The fix should be to change it to <dfn for="NoncedElement"><code data-x="dom-NoncedElement-nonce">nonce</code></dfn> right?

But then again I might be wrong and it might be supposed to pick up the interface someway else. Most of what bikeshed does is like magic to me anyway.

Copy link
Member

Choose a reason for hiding this comment

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

HTML doesn't use Bikeshed, so we get to add all the attributes Bikeshed autogenerates manually. You'll want something like data-dfn-for="NoncedElement" data-dfn-type="attribute" data-export="", I think. @annevk can confirm.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, @tabatkins can confirm. @annevk can probably confirm too, though. :)

Copy link
Member

Choose a reason for hiding this comment

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

Well, Shepherd (which does the scraping) scrapes the IDL and should therefore know about {{NoncedElement/nonce}}. Perhaps Shepherd isn't updated though and doesn't know about mixins?

(Did you meant to reference the IDL attribute here?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Either the IDL attribute or the next paragraph that details the nonce attribute would be fine on my part.

Copy link
Member

Choose a reason for hiding this comment

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

No, @annevk's right, this should be <{NoncedElement/nonce}> to reference the content attribute, rather than {{NoncedElement/nonce}}. That's still going to require poking at HTML, adding something like data-dfn-for="NoncedElement" data-dfn-type="element-attr" to the definition in the first paragraph of that section. Would you like to poke at it, or shall I?

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