Re: [whatwg/fetch] Add WebDriver BiDi network request logging (PR #1540)

@annevk commented on this pull request.

Thanks for tackling this. Looks reasonable overall, but I found some issues.

> +    "WEBDRIVER-BIDI": {
+        "authors": [],
+        "href": "https://w3c.github.io/webdriver-bidi/",
+        "publisher": "W3C",
+        "title": "WebDriver BiDi"

Any reason this isn't indexed automatically? Normally this shouldn't be needed.

> +Note: the [=request/request id=] is used by WebDriver-BiDi. It remains constant
+across all requests resultsing from a redirect of an initial request. When a
+request is [=request/cloned=], the created request gets a unique
+[=request/request id=]. [[!WEBDRIVER-BIDI]]

These two statements are contradictory. You probably have to manage this a bit more explicitly throughout the specification.

> @@ -2123,6 +2129,11 @@ Unless stated otherwise, it is false.
 
 <p class=note>This is for exclusive use by HTML's navigate algorithm. [[!HTML]]
 
+<p>A <a for=/>request</a> has an associated <dfn export for=request>navigation id</dfn>.
+Unless stated otherwise, it is null.

You should state the type somewhere. Null or a string?

> +     <!-- the serviceworker spec is responsible for emmitting the WebDriver BiDi
+     request events in this case. That's necessary to ensure that the events are
+     only generated if the serviceworker will handle the fetch, and to get the
+     correct event ordering in the case of network fallback -->

> Service Workers is responsible

and later on

> the service worker will handle

Also please align indentation with similar multiline comments (also applies elsewhere).

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

Message ID: <whatwg/fetch/pull/1540/review/1436208609@github.com>