Skip to content
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.

Sign up for GitHub

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

Improve navigate for service worker hooks #1776

Merged
merged 15 commits into from
Oct 24, 2016
Merged

Improve navigate for service worker hooks #1776

Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
eceffd0
Improve navigate for service worker hooks
jungkees Sep 13, 2016
f33307c
Add data-x and fix a wrapping
jungkees Sep 29, 2016
f585688
Destroy the reserved environment
jungkees Sep 29, 2016
d383562
Set reserved client id for workers
jungkees Sep 29, 2016
e5635f9
Address @domenic's review comments https://github.com/whatwg/html/pul…
jungkees Oct 12, 2016
47b2cc2
Move service worker concept to environment
jungkees Oct 12, 2016
7bd8291
Remove reference to request's target browsing context
jungkees Oct 13, 2016
30aa5d3
Resolve Travis CI check failure
jungkees Oct 13, 2016
baa99a3
Fix missed bits: revert variable names
jungkees Oct 13, 2016
db53aff
Change name of environment's service worker to active service worker
jungkees Oct 13, 2016
383e682
Remove re-wrapping and blank-line-removing
jungkees Oct 14, 2016
71eb8fa
Remove set up an environment algorithm
jungkees Oct 17, 2016
250e7a7
Address linking issues
jungkees Oct 18, 2016
a3e7049
Replace destroy language with resetting the id
jungkees Oct 19, 2016
0483230
Update link; remove whitespace; capitalize letter
jungkees Oct 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 165 additions & 49 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,7 @@ a.setAttribute('href', 'http://example.com/'); // change the content attribute d
<li><dfn data-x="concept-fetch" data-x-href="https://fetch.spec.whatwg.org/#concept-fetch">fetch</dfn></li>
<li><dfn data-x-href="https://fetch.spec.whatwg.org/#concept-http-redirect-fetch">HTTP-redirect fetch</dfn></li>
<li><dfn data-x-href="https://fetch.spec.whatwg.org/#ok-status">ok status</dfn></li>
<li><dfn data-x="navigation-request" data-x-href="https://fetch.spec.whatwg.org/#navigation-request">navigation request</dfn></li>
<li><dfn data-x-href="https://fetch.spec.whatwg.org/#concept-network-error">network error</dfn></li>
<li>`<dfn data-dfn-type="http-header" data-x="http-origin" data-x-href="https://fetch.spec.whatwg.org/#http-origin"><code>Origin</code></dfn>` header</li>
<li><dfn data-x-href="https://fetch.spec.whatwg.org/#process-response">process response</dfn></li>
Expand Down Expand Up @@ -2833,7 +2834,8 @@ a.setAttribute('href', 'http://example.com/'); // change the content attribute d
<li><dfn data-x="concept-request-header-list" data-x-href="https://fetch.spec.whatwg.org/#concept-request-header-list">header list</dfn></li>
<li><dfn data-x="concept-request-body" data-x-href="https://fetch.spec.whatwg.org/#concept-request-body">body</dfn></li>
<li><dfn data-x="concept-request-client" data-x-href="https://fetch.spec.whatwg.org/#concept-request-client">client</dfn></li>
<li><dfn data-x="concept-request-target-browsing-context" data-x-href="https://fetch.spec.whatwg.org/#concept-request-target-browsing-context">target browsing context</dfn></li>
<li><dfn data-x="concept-request-reserved-client" data-x-href="https://fetch.spec.whatwg.org/#concept-request-reserved-client">reserved client</dfn></li>
<li><dfn data-x="concept-request-target-client-id" data-x-href="https://fetch.spec.whatwg.org/#concept-request-target-client-id">target client id</dfn></li>
<li><dfn data-x="concept-request-initiator" data-x-href="https://fetch.spec.whatwg.org/#concept-request-initiator">initiator</dfn></li>
<li><dfn data-x="concept-request-type" data-x-href="https://fetch.spec.whatwg.org/#concept-request-type">type</dfn></li>
<li><dfn data-x="concept-request-destination" data-x-href="https://fetch.spec.whatwg.org/#concept-request-destination">destination</dfn></li>
Expand Down Expand Up @@ -3866,10 +3868,14 @@ a.setAttribute('href', 'http://example.com/'); // change the content attribute d
<p>The following terms are defined in <cite>Service Workers</cite>: <ref spec="SW"></p>

<ul class="brief">
<li><dfn data-x="dfn-active-worker" data-x-href="https://w3c.github.io/ServiceWorker/#dfn-active-worker">active worker</dfn></li>
<li><dfn data-x="dfn-client-message-queue" data-x-href="https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#dfn-client-message-queue">client message queue</dfn></li>
Copy link
Member

Choose a reason for hiding this comment

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

If we land this PR first before the other PR I wrote where I update links we should also update this link here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

<li><dfn data-x="scope-match-algorithm" data-x-href="https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#scope-match-algorithm">match service worker registration</dfn></li>
<li><dfn data-x="service-worker-container-interface" data-x-href="https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#service-worker-container-interface"><code>ServiceWorkerContainer</code></dfn> interface</li>
<li><dfn data-x="dfn-service-worker-container-interface-client" data-x-href="https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#dfn-service-worker-container-interface-client">service worker client</dfn></li>
<li><dfn data-x="dfn-control" data-x-href="https://w3c.github.io/ServiceWorker/#dfn-control">control</dfn></li>
<li><dfn data-x="on-fetch-request-algorithm" data-x-href="https://w3c.github.io/ServiceWorker/#on-fetch-request-algorithm">handle fetch</dfn></li>
<li><dfn data-x="scope-match-algorithm" data-x-href="https://w3c.github.io/ServiceWorker/#scope-match-algorithm">match service worker registration</dfn></li>
<li><dfn data-x="dfn-service-worker" data-x-href="https://w3c.github.io/ServiceWorker/#dfn-service-worker">service worker</dfn></li>
<li><dfn data-x="dfn-service-worker-container-interface-client" data-x-href="https://w3c.github.io/ServiceWorker/#dfn-service-worker-container-interface-client">service worker client</dfn></li>
<li><dfn data-x="service-worker-container-interface" data-x-href="https://w3c.github.io/ServiceWorker/#service-worker-container-interface"><code>ServiceWorkerContainer</code></dfn> interface</li>
</ul>

</dd>
Expand All @@ -3883,6 +3889,7 @@ a.setAttribute('href', 'http://example.com/'); // change the content attribute d

<ul class="brief">
<li><dfn data-x-href="https://w3c.github.io/webappsec-secure-contexts/#settings-object">Is environment settings object a secure context?</dfn></li>
<li><dfn data-x="secure-context" data-x-href="https://w3c.github.io/webappsec-secure-contexts/#secure-context">secure context</dfn></li>
Copy link
Member

Choose a reason for hiding this comment

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

FYI for terms like "secure context" and "service worker" that are not very ambiguous, there's no need to have data-x. You can just do <dfn data-x-href="...">secure context</dfn> and then at the reference site do <span>service worker</span>. But this way is fine too, no need to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just left it out but wonder if there's any guideline that one can determine whether to use data-x for a definition.

Copy link
Member

Choose a reason for hiding this comment

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

The guideline is basically whether the term is unique enough to stand out on its own. I think here it would be fine to leave out data-x="".

</ul>

</dd>
Expand Down Expand Up @@ -77984,8 +77991,9 @@ console.assert(iframeWindow.frameElement === null);
<h4>Script settings for browsing contexts</h4>

<p>When the user agent is required to <dfn>set up a browsing context environment settings
Copy link
Member

Choose a reason for hiding this comment

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

As discussed, this should move. I think a good place is right after the definition of environment (in 8.1.3.1). Also, the argument should be "an environment environment", not "an environment settings object environmentSettings".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed "set up an environment" algorithm as you commented below.

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, it might be good to split this into two algorithms: "set up a simple environment" which takes an environment and a URL, and "set up an environment from another environment" which takes an environment. Then no call sites would have to pass null explicitly, and there would not be an optional ID parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed "set up an environment" algorithm as you commented below. Indeed, "set up an environment from another environment" would have had only one call site, so it seems nicer to explicitly set those fields without defining an algorithm for it.

object</dfn>, given a <span>JavaScript execution context</span> <var>execution context</var>, it
must run the following steps:</p>
object</dfn>, given a <span>JavaScript execution context</span> <var>execution context</var> and
an optional <span>environment</span> <var>reserved environment</var>, it must run the following
steps:</p>

<ol>
<li><p>Let <var>realm</var> be the value of <var>execution context</var>'s Realm
Expand All @@ -78011,7 +78019,6 @@ console.assert(iframeWindow.frameElement === null);

</dd>


<dt>The <span data-x="concept-settings-object-module-map">module map</span></dt>
<dd>

Expand Down Expand Up @@ -78070,13 +78077,6 @@ console.assert(iframeWindow.frameElement === null);

</dd>

<dt>The <span>creation URL</span></dt>
<dd>

<p>Return <var>url</var>.</p>

</dd>

<dt>The <span>HTTPS state</span></dt>
<dd>

Expand Down Expand Up @@ -78105,6 +78105,34 @@ console.assert(iframeWindow.frameElement === null);
</dl>
</li>

<li>
<p>If <var>reserved environment</var> is given, then:</p>

<ol>
<li><p>Set <var>settings object</var>'s <span data-x="concept-environment-id">id</span> to
<var>reserved environment</var>'s <span data-x="concept-environment-id">id</span>,
<var>settings object</var>'s <span data-x="concept-environment-creation-url">creation
URL</span> to <var>reserved environment</var>'s <span
data-x="concept-environment-creation-url">creation URL</span>, <var>settings object</var>'s
<span data-x="concept-environment-target-browsing-context">target browsing context</span> to
<var>reserved environment</var>'s <span
data-x="concept-environment-target-browsing-context">target browsing context</span>, and
<var>settings object</var>'s <span data-x="concept-environment-active-service-worker">active
service worker</span> to <var>reserved environment</var>'s <span
data-x="concept-environment-active-service-worker">active service worker</span>.</p></li>

<li><p>Destroy <var>reserved environment</var>.</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to say this (we can just rely on magical specification garbage collection). So you can probably inline the step before rather than use a list here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was worrying about the situation where we get two objects (both the environment and the environment settings object) from a single id, which can happen if the environment is not destroyed properly right after this step. Or should I just reset the id of the environment to the empty string here?

Copy link
Member

Choose a reason for hiding this comment

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

I see. Setting the id and adding a note about how it's observable would be good. "Destroy" doesn't really have any meaning, so I'd rather not use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Will do.

</ol>
</li>

<li><p>Otherwise, set <var>settings object</var>'s <span
data-x="concept-environment-id">id</span> to a new unique opaque string, <var>settings
Copy link
Member

Choose a reason for hiding this comment

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

Do we expose these strings somewhere? Perhaps we should define the "opaque" format in that case. Not having that defined turned out to be a problem before (with blob URLs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we do. FetchEvent's clientId/reservedClientId/targetClientId and Client's id should return the values of this type. We used the UUID at the beginning but changed to not mandate the UUID type. Here's a discussion on this: w3c/ServiceWorker#647 And also I think we're now doing what you filed here: w3c/ServiceWorker#643

By the way, is there any reference that I can base the work on?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so. What might be problematic is if folks start assuming the string is UTF-8-safe or even ASCII-safe, depending on what user agents do. Is it an option to return something other than a string that can only be used to check for object identity? E.g., a symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only be used to check for object identity

FWIW, I think this is the only purpose of the id itself. I don't think I'm familiar with what you meant by a symbol, though. Could you explain more? Would it be shown as just a string to JS?

Copy link
Member

Choose a reason for hiding this comment

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

As a Symbol: https://tc39.github.io/ecma262/#sec-symbol-objects. They're somewhat more opaque than strings so might be more suitable here. Not a 100% sure though and it also depends a bit on what happens with the ID.

Copy link
Member

Choose a reason for hiding this comment

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

Symbol doesn't work great across realms, without basically giving it a string name (like "Symbol.iterator") in which case we're back to strings. String is probably the best we'll be able to do.

Copy link
Member

Choose a reason for hiding this comment

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

If these IDs are meant to work across realms, that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, IDs are values that can be shared across realms. Symbol.for("stringkey") wouldn't help much here.

object</var>'s <span data-x="concept-environment-creation-url">creation URL</span> to
<var>url</var>, <var>settings object</var>'s <span
data-x="concept-environment-target-browsing-context">target browsing context</span> to null, and
<var>settings object</var>'s <span data-x="concept-environment-active-service-worker">active
service worker</span> to null.</p></li>

<li><p>Set <var>realm</var>'s [[HostDefined]] field to <var>settings object</var>.</p></li>

<li><p>Return <var>settings object</var>.</p></li>
Expand Down Expand Up @@ -82105,14 +82133,14 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O

<li><p>Set <var>request</var>'s <span data-x="concept-request-client">client</span> to
<var>sourceBrowsingContext</var>'s <span>active document</span>'s <span>relevant settings
object</span>, <span data-x="concept-request-target-browsing-context">target browsing
context</span> to <var>browsingContext</var>, <span
data-x="concept-request-destination">destination</span> to "<code data-x="">document</code>",
<span data-x="concept-request-mode">mode</span> to "<code data-x="">navigate</code>", <span
data-x="concept-request-credentials-mode">credentials mode</span> to "<code
data-x="">include</code>", <span>use-URL-credentials flag</span>, and <span
data-x="concept-request-redirect-mode">redirect mode</span> to "<code
data-x="">manual</code>".</p></li>
object</span>, <span data-x="concept-request-destination">destination</span> to "<code
data-x="">document</code>", <span data-x="concept-request-mode">mode</span> to "<code
data-x="">navigate</code>", <span data-x="concept-request-credentials-mode">credentials
mode</span> to "<code data-x="">include</code>", <span>use-URL-credentials flag</span>, <span
data-x="concept-request-redirect-mode">redirect mode</span> to "<code data-x="">manual</code>",
and <span data-x="concept-request-target-client-id">target client id</span> to
<var>browsingContext</var>'s <span>active document</span>'s <span>relevant settings
object</span>'s <span data-x="concept-environment-id">id</span>.</p></li>

<li><p>Set <var>request</var>'s <span>omit-<code>Origin</code>-header flag</span>.

Expand All @@ -82128,6 +82156,23 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
data-x="concept-request-origin">origin</span> to that <span>browsing context scope origin</span>
and unset <var>request</var>'s <span>omit-<code>Origin</code>-header flag</span>.</p></li>

<li>
<p>Create a new <span>environment</span> <var>reservedEnvironment</var>, and set its <span
data-x="concept-environment-id">id</span> to a new unique opaque string, its <span
Copy link
Member

Choose a reason for hiding this comment

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

creation URL is not linking correctly; needs a data-x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

data-x="concept-environment-creation-url">creation URL</span> to <var>request</var>'s
<span>url</span>, and its <span data-x="concept-environment-target-browsing-context">target
browsing context</span> to <var>browsingContext</var>.</p>

<p class="note">The created environment's <span
Copy link
Member

Choose a reason for hiding this comment

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

Hmm now I am questioning whether you need the "set up an environment" algorithm(s) at all. You can just create them and set the fields like you do here. That seems pretty nice too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed this comment: set the fields in the relevant steps instead of defining/calling an algorithm.

Copy link
Member

Choose a reason for hiding this comment

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

Link active service worker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

data-x="concept-environment-active-service-worker">active service worker</span> is set in the
<span data-x="on-fetch-request-algorithm">handle fetch</span> algorithm during the subsequent
Copy link
Member

Choose a reason for hiding this comment

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

Link creation URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

fetch flow if its <span data-x="concept-environment-creation-url">creation URL</span> matches a
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what subsequent fetch flow is. Do you mean "during the fetch"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I meant "during the fetch". Will change it to "during the fetch".

service worker registration. <ref spec="SW"></p>
</li>

<li><p>Set <var>request</var>'s <span data-x="concept-request-reserved-client">reserved
client</span> to <var>reservedEnvironment</var>.</p></li>

<li>
<p>If the <span>Should navigation request of type from source in target be blocked by Content
Security Policy?</span> algorithm returns "<code data-x="">Blocked</code>" when executed upon
Expand Down Expand Up @@ -82240,15 +82285,15 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
</li>

<li><p>Run <span>process a navigate response</span> given <var>request</var>,
<var>response</var>, <var>navigationType</var>, the <span>source browsing context</span>, and
<var>browsingContext</var>.</p></li>
<var>response</var>, <var>navigationType</var>, the <span>source browsing context</span>,
<var>browsingContext</var>, and <var>reservedEnvironment</var>.</p></li>
</ol>

<p>To <dfn data-export="">process a navigate response</dfn>, given a <span
data-x="concept-request">request</span> <var>request</var>, a <span
data-x="concept-response">response</span> <var>response</var>, a string <var>type</var>, and two
<span>browsing contexts</span> <var>source</var> and <var>browsingContext</var>, run these
steps:</p>
data-x="concept-response">response</span> <var>response</var>, a string <var>type</var>, two
<span>browsing contexts</span> <var>source</var> and <var>browsingContext</var>, and an optional
<span>environment</span> <var>reservedEnvironment</var>, run these steps:</p>

<ol>
<li>
Expand Down Expand Up @@ -82374,12 +82419,15 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
<ol>
<li><p>Let <var>window</var> be null.</p></li>

<li><p>Let <var>settingsObject</var> be null.</p></li>

<li><p>If <var>browsingContext</var>'s only entry in its <span>session history</span> is the
<code>about:blank</code> <code>Document</code> that was added when <var>browsingContext</var>
was <span data-x="creating a new browsing context">created</span>, and navigation is occurring
with <span>replacement enabled</span>, and that <code>Document</code> has the <span>same
origin</span> as the new <code>Document</code>, then set <var>window</var> to the
<code>Window</code> object of that <code>Document</code>.</p></li>
<code>Window</code> object of that <code>Document</code>, and set <var>settingsObject</var> to
<var>window</var>'s <span>relevant settings object</span>.</p></li>

<li>
<p>Otherwise,</p>
Expand All @@ -82403,7 +82451,8 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
</li>

<li><p><span>Set up a browsing context environment settings object</span> with <var>realm
execution context</var>.</p></li>
execution context</var> and <var>reservedEnvironment</var>, if present, and set
<var>settingsObject</var> to the result.</p></li>
</ol>
</li>

Expand Down Expand Up @@ -82443,6 +82492,27 @@ State: &lt;OUTPUT NAME=I>1&lt;/OUTPUT> &lt;INPUT VALUE="Increment" TYPE=BUTTON O
</li>

<li><p><span>Implement the sandboxing</span> for the <code>Document</code>.</p></li>

<li>
<p>If <var>settingsObject</var>'s <span
data-x="concept-environment-active-service-worker">active service worker</span> is not null,
then:</p>

<ol>
<li><p>If <var>settingsObject</var> is a <span data-x="secure-context">secure context</span>,
then:</p></li>

<ol>
<li><p>If <var>browsingContext</var> has an <span>opener browsing context</span> and
Copy link
Member

Choose a reason for hiding this comment

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

Need a data-x to link the disown. Also it looks like the span is not closed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I think the span was closed but the pattern it being used was different from that of other occurrences.

<var>request</var>'s <span>client</span> is a <span>non-secure context</span>, then <span
data-x="disowned its opener">disown <var>browsingContext</var>'s opener</span>.</p></li>
</ol>

<li><p>Otherwise, set <var>settingsObject</var>'s <span
data-x="concept-environment-active-service-worker">active service worker</span> to
null.</p></li>
</ol>
</li>
</ol>

</li>
Expand Down Expand Up @@ -86262,8 +86332,60 @@ interface <dfn>NavigatorOnLine</dfn> {

<hr>
Copy link
Member

Choose a reason for hiding this comment

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

I think the phrasing "the settings of a current or potential execution environment" is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks.


<p>An <dfn data-export="">environment settings object</dfn> specifies algorithms for obtaining the
following:</p>
<p>An <dfn data-export="">environment</dfn> is an object that identifies the settings of a
current or potential execution environment. An <span>environment</span> has the following
fields:</p>

<dl>
Copy link
Member

Choose a reason for hiding this comment

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

I think this would end up looking a lot clearer as a <dl> like environment settings object. The only difference is that instead of "specifies algorithms for obtaining the following," you'd say something like "has the following fields"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


<dt>An <dfn data-x="concept-environment-id" data-export=""
data-dfn-for="environment">id</dfn></dt>

<dd>
Copy link
Member

Choose a reason for hiding this comment

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

s/itself/the environment/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


Copy link
Member

Choose a reason for hiding this comment

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

Notes should go in <p class="note"> (and not include the words "Note that" inside such paragraphs). I guess that was note the case before but it would be nice to fix while you're here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<p>An opaque string that uniquely identifies the <span>environment</span>.</p>

Copy link
Member

Choose a reason for hiding this comment

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

Since this is a new <dl> I feel like we should remove a whole bunch of whitespace. E.g., when <dd> only contains a single <p> it can be all together. No newline between <dt> and <dd>, only between <dd> and <dt>. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Referencing the contributing.md,

If a "block" element contains a single "block" element, do not put it on a newline. Do not indent for anything except a new "block" element.

I think that makes sense for new works and changed them as such.

</dd>

<dt>A <dfn data-x="concept-environment-creation-url" data-export=""
data-dfn-for="environment">creation URL</dfn></dt>

<dd>

<p>An <span>absolute URL</span> that represents the location of the resource with which the
<span>environment</span> is associated.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Instead of stating that some things are initially null, I would add a sentence saying when these fields are set. E.g. "These fields are all set by the set up an environment algorithm" or "These fields are all set when the environment is created". Or just leave it out I guess, since that's kind of obvious :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These fields are set during the setup of an environment or an environment settings object. So, I just left it out as you commented.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm can we prefix this with "In the case of an environment settings object, this URL might be distinct..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's more accurate with the phrase. Done.


<p class="note">In the case of an <span>environment settings object</span>, this URL might be
distinct from the <span>environment settings object</span>'s <span>responsible
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "additionally specifies algorithms for"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

document</span>'s <span data-x="concept-document-url">URL</span>, due to mechanisms such as
<code data-x="dom-history-pushstate">history.pushState()</code>.</p>

</dd>

<dt>A <dfn data-x="concept-environment-target-browsing-context" data-export=""
data-dfn-for="environment">target browsing context</dfn></dt>

Copy link
Member

Choose a reason for hiding this comment

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

Over 100 chars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<dd>

<p>null or a target <span>browsing context</span> for a <span
data-x="navigation-request">navigation request</span>.</p>

</dd>

<dt>An <dfn data-x="concept-environment-active-service-worker" data-export=""
data-dfn-for="environment">active service worker</dfn></dt>

Copy link
Member

Choose a reason for hiding this comment

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

s/it/the environment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<dd>

Copy link
Member

Choose a reason for hiding this comment

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

This should say "Null" I think. We don't start with a lowercase letter in these <dd>s anywhere else either. Same for below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<p>null or a <span data-x="dfn-service-worker">service worker</span> that <span
data-x="dfn-control">controls</span> the <span>environment</span>.</p>

</dd>

</dl>

<p>An <dfn data-export="">environment settings object</dfn> is an <span>environment</span> that
additionally specifies algorithms for:</p>

<dl>

Expand Down Expand Up @@ -86360,15 +86482,6 @@ interface <dfn>NavigatorOnLine</dfn> {

</dd>

<dt>A <dfn data-export="" data-dfn-for="environment settings object">creation URL</dfn></dt>

<dd>
<p>An <span>absolute URL</span> representing the location of the resource with which the
<span>environment settings object</span> is associated. Note that this URL might be distinct
from the <span>responsible document</span>'s <span data-x="concept-document-url">URL</span>, due
to mechanisms such as <code data-x="dom-history-pushstate">history.pushState()</code>.</p>
</dd>

<dt>An <dfn data-export="" data-dfn-for="environment settings object">HTTPS state</dfn></dt>

<dd><p>An <span>HTTPS state value</span> representing the security properties of the network
Expand Down Expand Up @@ -96762,6 +96875,9 @@ interface <dfn>SharedWorkerGlobalScope</dfn> : <span>WorkerGlobalScope</span> {
data-x="fetching-scripts-is-top-level">is top-level</var> flag is set:</p>

<ol>
<li>Set <var>request</var>'s <span data-x="concept-request-reserved-client">reserved
client</span> to <var>inside settings</var>.</li>

<li><p><span data-x="concept-fetch">Fetch</span> <var>request</var>, and asynchronously wait
to run the remaining steps as part of fetch's <span>process response</span> for the <span
data-x="concept-response">response</span> <var>response</var>.</p></li>
Expand Down Expand Up @@ -97132,14 +97248,6 @@ interface <dfn>AbstractWorker</dfn> {

</dd>

<dt>The <span>creation URL</span></dt>
<dd>

<p>Return <var>worker global scope</var>'s <span
data-x="concept-WorkerGlobalScope-url">url</span>.</p>

</dd>

<dt>The <span>HTTPS state</span></dt>
<dd>

Expand All @@ -97160,6 +97268,14 @@ interface <dfn>AbstractWorker</dfn> {

</li>

<li><p>Set <var>settings object</var>'s <span data-x="concept-environment-id">id</span> to a new
unique opaque string, <var>settings object</var>'s <span
data-x="concept-environment-creation-url">creation URL</span> to <var>worker global
scope</var>'s <span>url</span>, <var>settings object</var>'s <span
data-x="concept-environment-target-browsing-context">target browsing context</span> to null, and
<var>settings object</var>'s <span data-x="concept-environment-active-service-worker">active
service worker</span> to null.</p></li>

<li><p>Set <var>realm</var>'s [[HostDefined]] field to <var>settings object</var>.</p></li>

<li><p>Return <var>settings object</var>.</p></li>
Expand Down Expand Up @@ -119320,7 +119436,7 @@ INSERT INTERFACES HERE
<dd>(Non-normative) <cite><a href="https://svgwg.org/svg2-draft/">Scalable Vector Graphics (SVG) 2</a></cite>, N. Andronikos, R. Atanassov, T. Bah, A. Bellamy-Royds, B. Birtles, B. Brinza, C. Concolato, E. Dahlström, C. Lilley, C. McCormack, D. Schepers, D. Schulze, R. Schwerdtfeger, S. Takagi, J. Watt. W3C.</dd>

<dt id="refsSW">[SW]</dt>
<dd><cite><a href="https://slightlyoff.github.io/ServiceWorker/spec/service_worker/">Service Workers</a></cite>, A. Russell, J. Song, J. Archibald. W3C.</dd>
<dd><cite><a href="https://w3c.github.io/ServiceWorker/">Service Workers</a></cite>, A. Russell, J. Song, J. Archibald. W3C.</dd>

<dt id="refsTOR">[TOR]</dt>
<dd>(Non-normative) <cite><a href="https://www.torproject.org/">Tor</a></cite>.</dd>
Expand Down