Re: [whatwg/fetch] Make status message default the empty byte sequence (#600)

jakearchibald approved this pull request.

Comments are just nits.

> @@ -1320,8 +1320,11 @@ cannot be exposed to script. That would violate <a>atomic HTTP redirect handling
 <a for=/>status</a>. Unless stated otherwise it is <code>200</code>.
 
 <p>A <a for=/>response</a> has an associated
-<dfn export for=response id=concept-response-status-message>status message</dfn>.
-Unless stated otherwise it is `<code>OK</code>`.
+<dfn export for=response id=concept-response-status-message>status message</dfn>. Unless stated
+otherwise it is the empty byte sequence.
+
+<p class=note>This does not have a default value of `<code>OK</code>` as otherwise all HTTP/2
+<a for=/>responses</a> would have that value.

Maybe make it clear that HTTP/2 doesn't support status messages. Something like: "HTTP/2 doesn't support status messages, so this property will be an empty byte sequence for all responses from an HTTP/2 connection"

> -  <a for=header>name</a> is `<code>Content-Type</code>` and
-  <a for=header>value</a> is
-  `<code>text/html;charset=utf-8</code>`,
-  <a for=response>body</a> is the empty byte sequence, and
-  <a for=response>HTTPS state</a> is <var>request</var>'s
-  <a for=request>client</a>'s <a for="environment settings object">HTTPS state</a>
-  if <var>request</var>'s <a for=request>client</a> is non-null.
+  <p>If <var>request</var>'s <a for=request>current url</a>'s <a>cannot-be-a-base-URL flag</a> is
+  set and <a for=url>path</a> contains a single string "<code>blank</code>", then return a new
+  <a for=/>response</a> whose <a for=response>status message</a> is `<code>OK</code>`,
+  <a for=response>header list</a> consist of a single <a for=/>header</a> whose
+  <a for=header>name</a> is `<code>Content-Type</code>` and <a for=header>value</a> is
+  `<code>text/html;charset=utf-8</code>`, <a for=response>body</a> is the empty byte sequence, and
+  <a for=response>HTTPS state</a> is <var>request</var>'s <a for=request>client</a>'s
+  <a for="environment settings object">HTTPS state</a> if <var>request</var>'s
+  <a for=request>client</a> is non-null.

Given the number of properties, dl/ul would be clearer.

> @@ -2985,9 +2983,10 @@ steps:
 
    <li><p>If <var>dataURLStruct</var> is failure, then return a <a>network error</a>.
 
-   <li><p>Return a <a for=/>response</a> whose <a for=response>header list</a> consist of a single
-   <a for=/>header</a> whose <a for=header>name</a> is `<code>Content-Type</code>` and
-   <a for=header>value</a> is <var>dataURLStruct</var>'s <a for="data: URL struct">MIME type</a>,
+   <li><p>Return a <a for=/>response</a> whose <a for=response>status message</a> is
+   `<code>OK</code>`, whose <a for=response>header list</a> consist of a single <a for=/>header</a>
+   whose <a for=header>name</a> is `<code>Content-Type</code>` and <a for=header>value</a> is
+   <var>dataURLStruct</var>'s <a for="data: URL struct">MIME type</a>,

I know it isn't something you changed in this PR, but there's a repetition of "whose" here which is inconsistent with the similar lines above.

Again, dl/ul could be useful here, especially when it comes to the header list.

-- 
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/600#pullrequestreview-114427956

Received on Monday, 23 April 2018 15:25:44 UTC