Re: [whatwg/fetch] Copy client-hints list from env settings object. Align CH processing (#773)

annevk commented on this pull request.



> @@ -996,11 +996,11 @@ to not have to set <a for=/>request</a>'s <a for=request>referrer</a>.
 
 
 <p>A <a for=/>request</a> has an associated
-<dfn export for=request id=concept-request-client-hints-list>client hints list</dfn>,
-which is a <a lt="client hints list" for=client>client-hints list</a>. Unless stated
-otherwise, it is the empty list.
+<dfn export for=request id=concept-request-client-hints-set>client hints set</dfn>,

Let's preserve the original ID. I don't think it matters much that it doesn't match the text anymore.

> @@ -996,11 +996,11 @@ to not have to set <a for=/>request</a>'s <a for=request>referrer</a>.
 
 
 <p>A <a for=/>request</a> has an associated
-<dfn export for=request id=concept-request-client-hints-list>client hints list</dfn>,
-which is a <a lt="client hints list" for=client>client-hints list</a>. Unless stated
-otherwise, it is the empty list.
+<dfn export for=request id=concept-request-client-hints-set>client hints set</dfn>,
+which is a <a lt="client hints set" for=client>client-hints set</a>. Unless stated

A primitive cannot belong to something else. I guess this was originally wrong as well.

> @@ -996,11 +996,11 @@ to not have to set <a for=/>request</a>'s <a for=request>referrer</a>.
 
 
 <p>A <a for=/>request</a> has an associated
-<dfn export for=request id=concept-request-client-hints-list>client hints list</dfn>,
-which is a <a lt="client hints list" for=client>client-hints list</a>. Unless stated
-otherwise, it is the empty list.
+<dfn export for=request id=concept-request-client-hints-set>client hints set</dfn>,
+which is a <a lt="client hints set" for=client>client-hints set</a>. Unless stated

When is the hyphen to be used by the way? I'd rather consistently use or not use it.

> @@ -1825,14 +1825,14 @@ run these steps:
 </ol>
 
 
-<h3 id=client-hints-list>Client hints list</h3>
+<h3 id=client-hints-set>Client hints set</h3>

ID preservation please.

>  
 <p class=note>This section will be integrated into HTTP Client Hints.
 [[!CLIENT-HINTS]]
 <!-- XXX -->
 
-<p>A <dfn export id=concept-client-hints-list for=client>client hints list</dfn> is a
-<a for=/>list</a> of
+<p>A <dfn export id=concept-client-hints-set for=client>client hints set</dfn> is a

I guess we should remove for=client here since it doesn't actually belong to a client, right?

> @@ -2640,6 +2640,10 @@ the request.
    <a for=request>origin</a> to  <var>request</var>'s
    <a for=request>client</a>'s <a for="environment settings object">origin</a>.
 
+   <li><p>Set <var>request</var>'s <a for=request>client-hints set</a> to be a copy of the <a

client-hints set won't link due to the hyphen. Also, instead of copy you want https://infra.spec.whatwg.org/#list-clone.

>      </table>
 
    <li>
-    <p>If <var>request</var> is a <a>subresource request</a>, then:
+    <p>If <var>request</var>'s <a for=request>current url</a>'s
+    <a for=url>origin</a> is <a>same origin</a> with <var>request</var>'s
+    <a for=request>origin</a>, then:

This means that a cross-origin URL inbetween doesn't influence this. I'm not sure that's what we want? Also, can we not obtain this state from something else that has already done the check? Lots of independent same-origin checks seem like a code smell.

>  
       <ol>
-       <li>
+        <li>

One space indentation was correct.

>        <p><a for=list>For each</a> <var>hintName</var> of <var>request</var>'s
-      <a for=client>client hints list</a>:
+      <a for=client>client-hints set</a>:

Hyphen again.

>      </ol>
-  </ol>
+
+    <li><p>If <var>request</var> is a <a>subresource request</a>, then:

The indentation is off here as well.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/773#pullrequestreview-145645439

Received on Monday, 13 August 2018 13:19:14 UTC