-
Notifications
You must be signed in to change notification settings - Fork 550
update the usage of cryptographic nonce metadata #1177
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some editorial and linking changes, but the content looks OK to me.
single-page.bs
Outdated
@@ -733,6 +734,11 @@ urlPrefix: https://tools.ietf.org/html/ | |||
url: rfc7232#section-2.2; type: http-header; spec: rfc7232; text: last-modified | |||
url: rfc7234#section-5.2; type: http-header; spec: rfc7234; text: cache-control | |||
|
|||
# ************************** INFRA ************************************* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unnecessary, bikeshed already has this reference. (Plus it is unnecessary as noted above)
sections/webappapis.include
Outdated
<li>Let <var>base URL</var> be <var>initiating script</var>'s <a>base URL</a>.</li> | ||
|
||
<li> | ||
<p>Let <var>fetch options</var> be a <a>script fetch options</a> whose <a>cryptographic nonce</a> is <var>initiating script</var>'s <a>fetch options</a>'s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be a script settings options -> be a set of...
sections/webappapis.include
Outdated
|
||
: A <dfn>parser state</dfn> | ||
:: The <a>parser metadata</a> used to fetch imported modules. | ||
: A <dfn>id</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A id -> An id
sections/webappapis.include
Outdated
: A <a>target browsing context</a> | ||
:: Null or a <a>service worker</a> that <a>controls</a> the <a>environment</a>. | ||
|
||
: A <dfn>execution ready flag</dfn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A execution -> An execution
sections/webappapis.include
Outdated
: A <dfn>module record</dfn> | ||
:: A <a>Source Text Module Record</a> representing the parsed module, ready to be evaluated. | ||
: <dfn>Fetch options</dfn> | ||
:: A <a>script fetch options</a>, containing various options related to fetching this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A script fetch options -> A set of...
or
-> A script fetch options object
as "[=parser-inserted=]", and "`not parser-inserted`" otherwise. | ||
17. Let <var>settings</var> be the element's <a>node document</a>'s {{Window}} object's | ||
18. Let options be a <a>script fetch options</a> whose <a>cryptographic nonce metadata</a> is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a script fetch options -> a set of script...
or -> a script fetch options object
@@ -631,26 +631,28 @@ | |||
must run the following steps: | |||
|
|||
|
|||
1. If the <{link/href}> attribute's value is the empty string, then abort these steps. | |||
1. If the <{link/href}> attribute's value is the empty string, then return. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note comment below on consistent terminology in algorithms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to do anything here.
PR updated. Thanks for the review @chaals . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update.
Unfortunately there are a handful more editorial and nitpicky things - it seems I forgot to add many of my comments to the review before, and found some I had missed
Also, at https://w3c.github.io/html/dom.html#global-attributes the nonce attribute doesn't link to a definition, but it should...
sections/changes.include
Outdated
@@ -24,6 +24,8 @@ | |||
<dd>Substantive change. Fixed <a href="https://github.com/w3c/html/issues/1104">issue 1104</a></dd> | |||
<dt><a href="https://github.com/w3c/html/pull/1167">Added <{media/disableRemotePlayback}> to the {{HTMLMediaElement}} interface</a></dt> | |||
<dd>Fixed <a href="https://github.com/w3c/html/issues/1047">issue 1047</a></dd> | |||
<dt><a href="https://github.com/w3c/html/pull/1177">Cleaned up the Fetch sections to apply the <a>cryptographic nonce metadata</a></a> </dt> | |||
<dd>Fixed <a href="https://github.com/w3c/html/issues/198">issue 198</a></dd> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you also added SRI, right?
@@ -631,26 +631,28 @@ | |||
must run the following steps: | |||
|
|||
|
|||
1. If the <{link/href}> attribute's value is the empty string, then abort these steps. | |||
1. If the <{link/href}> attribute's value is the empty string, then return. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to do anything here.
@@ -431,11 +431,11 @@ o............A....e | |||
|
|||
Otherwise, if the <{script}> element has a <{script/type}> attribute, let | |||
<var>the script block's type string</var> for this <{script}> element be the value of that | |||
attribute with any leading or trailing sequences of [=space characters=] removed. | |||
attribute with <a>strip leading and trailing ASCII whitespace</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should read:
attribute after <a>stripping leading and trailing white space</a>.
to pick up the term defined in the spec.
sections/webappapis.include
Outdated
initial fetch (for <a>module scripts</a>) and for fetching any | ||
imported modules (for both <a>module scripts</a> and <a>classic scripts</a>)</p></dd> | ||
|
||
<dt><dfnfor="script">referrer policy</dfn></dt> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dfn for (missing space)
sections/webappapis.include
Outdated
<p>The <dfn>default classic script fetch options</dfn> are a set of<a>script fetch options</a> | ||
whose <a>cryptographic nonce</a> is the empty | ||
string, <a>integrity metadata</a> is the | ||
empty string, <a>parser metadata</span> is "<code>not-parser-inserted</code>", <a>credentials mode</a> is "<code>omit</code>", and <a>referrer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please split the line differently so you don't have a line break inside the link <a>referrer policy</a>
sections/webappapis.include
Outdated
<dl> | ||
<dt><dfn>set up the classic script request</dfn></dt> | ||
<dd><p>Set <var>request</var>'s <a>cryptographic nonce | ||
metadata</a> to <var>options</var>'s <a>cryptographic nonce</a>, its <a>integrity metadata</a> to <var>options</var>'s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the final "s" at the end of the line - should read
...<var>options</var>'
sections/webappapis.include
Outdated
empty string, <a>parser metadata</span> is "<code>not-parser-inserted</code>", <a>credentials mode</a> is "<code>omit</code>", and <a>referrer | ||
policy</a> is the empty string.</p> | ||
|
||
<p>Given a <a>request</a> <var>request</var> and a <a>script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line breaks
sections/webappapis.include
Outdated
:: A <a>URL record</a> that represents the location of the resource with which the | ||
<a>environment</a> is associated. | ||
<p class="note">In the case of an <a>environment settings object</a>, this URL might be | ||
distinct from the <a>environment settings object</a>'s <a>responsible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't split the link over 2 different lines
sections/webappapis.include
Outdated
:: A flag which, if set, means that error information will not be provided for errors in this | ||
script (used to mute errors for cross-origin scripts, since that can leak private information). | ||
: A <dfn>record</dfn> | ||
:: Either a <a>Script Record</a>, for <a>classic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't split the link over 2 different lines
@@ -449,7 +449,7 @@ | |||
The <dfn element-attr for="link"><code>media</code></dfn> attribute says which media the resource | |||
applies to. The value must be a <a>valid media query list</a>. | |||
|
|||
<!-- def <{link/integrity}> --> | |||
The <dfn element-attr for="link"><code>integrity</code></dfn> attribute represents the <a>integrity metadata</a> for requests which this element is responsible for. The value is text. The attribute must not be specified on <{link}> elements that do not have a <{link/rel}> attribute that contains the <{link/stylesheet}> keyword. [[!SRI]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please split this line for easier reading in tools that don't auto-wrap. E.g. < 100 characters / line ...
Fix linking error, note SRI was added.
To fix #198 :