Re: [w3c/ServiceWorker] (WIP) Bikeshed cleanup (#1014)

mkruisselbrink commented on this pull request.

Awesome cleanup. Thanks for cleaning up my mess :) I haven't quite made it through the entire thing yet, but it looks good I think. Just some random ideas for possible future improvements.

>  
-  <p>The <a href="#dfn-service-worker">service worker</a> is designed first to redress this balance by providing a Web Worker context, which can be started by a runtime when navigations are about to occur. This event-driven worker is registered against an origin and a path (or pattern), meaning it can be consulted when navigations occur to that location. Events that correspond to network requests are dispatched to the worker and the responses generated by the worker may override default network stack behavior. This puts the <a href="#dfn-service-worker">service worker</a>, conceptually, between the network and a document renderer, allowing the <a href="#dfn-service-worker">service worker</a> to provide content for documents, even while offline.</p>
+  The <a for="/">service worker</a> is designed first to redress this balance by providing a Web Worker context, which can be started by a runtime when navigations are about to occur. This event-driven worker is registered against an origin and a path (or pattern), meaning it can be consulted when navigations occur to that location. Events that correspond to network requests are dispatched to the worker and the responses generated by the worker may override default network stack behavior. This puts the <a for="/">service worker</a>, conceptually, between the network and a document renderer, allowing the <a for="/">service worker</a> to provide content for documents, even while offline.

depending on how shorthand-rather-than-html you want to go, all these `"<a for="/">service worker</a>"` bits could be written as `[=/service worker=]`.

> -          <ol>
-            <li>Create an event <var>e</var> that uses the {{ExtendableMessageEvent}} interface, with the event type {{message!!event}}, which does not bubble and is not cancelable.</li>
-            <li>Let the {{ExtendableMessageEvent/data}} attribute of <var>e</var> be initialized to <var>clonedMessage</var>.</li>
-            <li>Let the {{ExtendableMessageEvent/origin}} attribute of <var>e</var> be initialized to the <a lt="Unicode serialization of an origin">Unicode serialization</a> of <var>incumbentSettings</var>'s <a for="resource">origin</a>.</li>
-            <li>If <var>incumbentGlobal</var> is a {{ServiceWorkerGlobalScope}} object, let the {{ExtendableMessageEvent/source}} attribute of <var>e</var> be initialized to a new {{ServiceWorker}} object that represents <var>incumbentGlobal</var>'s <a for="ServiceWorkerGlobalScope">service worker</a>.</li>
-            <li>Else if <var>incumbentGlobal</var> is a {{Window}} object, let the {{ExtendableMessageEvent/source}} attribute of <var>e</var> be initialized to a new {{WindowClient}} object that represents <var>incumbentGlobal</var>'s <a for="/">browsing context</a>.</li>
-            <li>Else, let it be initialized to a new {{Client}} object that represents the worker associated with <var>incumbentGlobal</var>.</li>
-            <li>Let the {{ExtendableMessageEvent/ports}} attribute of <var>e</var> be initialized to <var>newPorts</var>.</li>
-            <li><a>Dispatch</a> <var>e</var> at <var>destination</var>.</li>
-          </ol>
-          <p>The <a>task</a> <em class="rfc2119" title="MUST">must</em> use the <a>DOM manipulation task source</a>.</p>
-        </li>
-      </ol>
+      The <dfn method for="ServiceWorker"><code>postMessage(|message|, |transfer|)</code></dfn> method *must* run these steps:
+
+        1. If the {{ServiceWorker/state}} attribute value of the <a>context object</a> is "<code>redundant</code>", <a>throw</a> an "{{InvalidStateError}}" exception and abort these steps. 

`"<code>redundant</code>"` could also be written as `{{"redundant"}}` (or `{{ServiceWorkerState/"redundant"}}` to get proper cross-references for enum values.

> -    <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-foreign-fetch-origins">list of foreign fetch origins</dfn> whose element type is a <a for="/">URL</a>. It is initially empty.</p>
+    A <dfn export id="dfn-service-worker" for="">service worker</dfn> is a type of <a>web worker</a>. A <a for="/">service worker</a> executes in the registering <a for="/">service worker client</a>'s <a for="/">origin</a>.
+
+    A <a for="/">service worker</a> has an associated <dfn export id="dfn-state">state</dfn>, which is one of *parsed*, *installing*, *installed*, *activating*, *activated*, and *redundant*. It is initially *parsed*.
+
+    A <a for="/">service worker</a> has an associated <dfn export id="dfn-script-url">script url</dfn> (a <a for="/">URL</a>).
+
+    A <a for="/">service worker</a> has an associated <dfn export id="dfn-type">type</dfn> which is either "<code>classic</code>" or "<code>module</code>". Unless stated otherwise, it is "<code>classic</code>".
+
+    A <a for="/">service worker</a> has an associated <dfn export id="dfn-containing-service-worker-registration" lt="registration|containing service worker registration">containing service worker registration</dfn> (a <a for="/">service worker registration</a>), which contains itself.
+
+    A <a for="/">service worker</a> has an associated <dfn export id="dfn-service-worker-id">id</dfn> (an opaque string), which uniquely identifies itself during the lifetime of its <a>containing service worker registration</a>.
+
+    A <a for="/">service worker</a> has an associated <dfn export id="dfn-service-worker-global-object" for="service worker">global object</dfn> (a {{ServiceWorkerGlobalScope}} object or null).
+    
+    A <a for="/">service worker</a> is dispatched a set of <dfn export id="dfn-lifecycle-events">lifecycle events</dfn>, <a event for="ServiceWorkerGlobalScope">install</a> and <a event for="ServiceWorkerGlobalScope">activate</a>, and <dfn export id="dfn-functional-events">functional events</dfn> including <a event for="ServiceWorkerGlobalScope">fetch</a>.

You're a bit inconsistent with how you link to event types. Here you use the full `<a ...>` syntax, while in other places you would have written this as `{{install!!event}}` (or `{{ServiceWorkerGlobalScope/install!!event}}` if you want to be explicit about which install event you're talking about).

>  
-      <p class="example">In the example in section 3.1.1, the value of <code>registration.scope</code>, obtained from <code>navigator.serviceWorker.ready.then(function(registration) { console.log(registration.scope); })</code> for example, will be "<code>https://example.com/</code>".</p>
+      <div class="example">
+        In the example in section 3.1.1, the value of <code>registration.scope</code>, obtained from <code>navigator.serviceWorker.ready.then(registration => console.log(registration.scope))</code> for example, will be "<code>https://example.com/</code>".

Maybe change `section 3.1.1` to `[[#service-worker-url]]` to get auto-linking to that section? Although not sure if having the section title in the resulting link will make it more or less readable here.

> -                <li>If the <a for="resource">origin</a> of the result of <a lt="URL parser">parsing</a> <var>entry</var>.\[[key]] is the <a lt="same origin">same</a> as <var>client</var>'s <a for="resource">origin</a>, and <var>entry</var>.\[[value]]'s <a href="#dfn-uninstalling-flag">uninstalling flag</a> is unset, add the {{ServiceWorkerRegistration}} object associated with <var>entry</var>.\[[value]] to the <var>array</var>.</li>
-              </ol>
-            </li>
-            <li>Resolve <var>promise</var> with <var>array</var>.</li>
-          </ol>
-        </li>
-        <li>Return <var>promise</var>.</li>
-      </ol>
+      <dfn method for="ServiceWorkerContainer"><code>getRegistrations()</code></dfn> method *must* run these steps:
+
+        1. Let |client| be the <a>context object</a>'s <a for="ServiceWorkerContainer">service worker client</a>.
+        1. Let |promise| be a new <a>promise</a>.
+        1. Run the following substeps <a>in parallel</a>:
+            1. Let |array| be an empty array.
+            1. <a for="map">For each</a> |key| → |value| of <a>scope to registration map</a>:
+                1. If the <a for="environment settings object">origin</a> of the result of <a lt="URL parser">parsing</a> |key| is the <a lt="same origin">same</a> as |client|'s <a for="environment settings object">origin</a>, and |value|'s <a>uninstalling flag</a> is unset, add the {{ServiceWorkerRegistration}} object associated with |value| to the |array|.

The first origin here should be `<a for="url">` as it's referring to the origin of a url rather than that of a settings object (I know, I made quite a mess about linking to the correct origin definition whenever an origin was used).

> @@ -765,7 +716,7 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
         <tr>
           <td><dfn event id="service-worker-container-controllerchange-event"><code>controllerchange</code></dfn></td>
           <td>{{Event}}</td>
-          <td>The <a for="ServiceWorkerContainer">service worker client</a>'s <a>active service worker</a> changes. (See step 9.2 of the <a href="#activation-algorithm">Activate</a> algorithm. The <a href="#dfn-skip-waiting-flag">skip waiting flag</a> of a <a href="#dfn-service-worker">service worker</a> causes <a href="#activation-algorithm">activation</a> of the <a href="#dfn-service-worker-registration">service worker registration</a> to occur while <a href="#dfn-service-worker-client">service worker clients</a> are <a href="#dfn-use">using</a> the <a href="#dfn-service-worker-registration">service worker registration</a>, {{ServiceWorkerContainer/controller|navigator.serviceWorker.controller}} immediately reflects the <a href="#dfn-active-worker">active worker</a> as the <a href="#dfn-service-worker">service worker</a> that <a href="#dfn-control">controls</a> the <a href="#dfn-service-worker-client">service worker client</a>.)</td>
+          <td>The <a for="ServiceWorkerContainer">service worker client</a>'s <a>active service worker</a> changes. (See step 9.2 of the <a>Activate</a> algorithm. The <a>skip waiting flag</a> of a <a for="/">service worker</a> causes <a lt="activate">activation</a> of the <a for="/">service worker registration</a> to occur while <a for="/">service worker clients</a> are <a>using</a> the <a for="/">service worker registration</a>, {{ServiceWorkerContainer/controller|navigator.serviceWorker.controller}} immediately reflects the <a>active worker</a> as the <a for="/">service worker</a> that <a>controls</a> the <a for="/">service worker client</a>.)</td>

@tabatkins hmm, would be nice if bikeshed provided a way to link to specific steps in an algorithm that would get rendered as "step 9.2", but wouldn't require hard-coding and manually keeping up to date the numbering (like it already sort of supports for sections).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1014#pullrequestreview-10611619