Skip to content

Call out to CSP's inline element hooks #274

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

Merged
merged 1 commit into from
Nov 6, 2015
Merged

Call out to CSP's inline element hooks #274

merged 1 commit into from
Nov 6, 2015

Conversation

mikewest
Copy link
Member

Another one. This is based on top of #272 and #273. Sorry for the mess.

@mikewest
Copy link
Member Author

@annevk: And this one too, when you have some time. Note that it depends on #273.

@mikewest
Copy link
Member Author

mikewest commented Nov 5, 2015

Rebased onto ToT.

@@ -13295,6 +13291,10 @@ own thing rather than part of the extended sentence -->
<li><p>If <var>element</var> is not <span>in a <code>Document</code></span>, then abort
these steps.</p></li> <!-- http://software.hixie.ch/utilities/js/live-dom-viewer/saved/2740 -->

<li><p>If the <span>Should node's inline behavior be blocked by Content Security Policy?</span>
Copy link
Member

Choose a reason for hiding this comment

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

Do you want id="style=processing-csp" here? Also, node or element?

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. What would adding id="style=processing-csp" do? I mean, I'm happy to, I just don't understand why. :)
  2. No preference between Node and Element (are there any nodes that aren't elements, in HTML?)

Copy link
Member

Choose a reason for hiding this comment

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

I didn't understand why you added script-processing-csp but maybe that was because of the other steps.

It seems this feature would only ever really apply to elements.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh. I added script-processing-csp because other things in that section had IDs. Just cargo-culting. I'd rather remove that ID than add another here, really, but maybe consistency is important?

Copy link
Member

Choose a reason for hiding this comment

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

It's fine to leave the script one and don't add one here.

@mikewest
Copy link
Member Author

mikewest commented Nov 6, 2015

Changed from node to element. I'll happily defer to your judgement about the IDs, and squish everything together once that's resolved.

content attribute, and the <span>Should node's inline behavior be blocked by Content Security
Policy?</span> algorithm returns "<code data-x="">Blocked</code>" when executed upon the
<code>script</code> element, then the user agent must abort these steps. The script is not
executed. <ref spec="CSP"></p>
Copy link
Member

Choose a reason for hiding this comment

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

The one nitpick I have left is that you use two spaces for indentation here. I think since this <li> only contains a single child we should just inline it as done for <style> and over time align the rest of the <script> algorithm with that style.

@annevk
Copy link
Member

annevk commented Nov 6, 2015

LGTM with the formatting fixed.

Content Security Policy defines a "Should element's inline behavior be
blocked by Content Security Policy?" algorithm in order to handle things
like nonces, hashes, and 'unsafe-inline'. This patch adds those hooks to
the appropriate spots in HTML's 'prepare a script' and 'update a style
block' algorithms.

#271
@mikewest
Copy link
Member Author

mikewest commented Nov 6, 2015

Rebased, formatted, squashed. Thanks!

@annevk annevk merged commit ee3486e into whatwg:master Nov 6, 2015
@annevk
Copy link
Member

annevk commented Nov 6, 2015

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants