Re: [w3c/manifest] Add support for 'serviceworkers' member (#507)

marcoscaceres requested changes on this pull request.



> @@ -2033,6 +2038,71 @@ <h3 id="applying">
           </p>
         </section>
       </section>
+
+      <section>
+        <h3>
+          <code>serviceworker</code> member
+        </h3>
+        <p>
+          The <dfn><code>serviceworker</code> member</dfn> describes a
+          service worker as defined in [[!SERVICE-WORKERS-1]].
+        </p>
+        <p>
+          The <a><code>serviceworker</code> member</a> represents a service worker
+          registration. Other service worker registrations can be done, for instance

Link to registration definition in SW

> +          If they have different script URLs, last one wins.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>serviceworker</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>
+          <var>serviceworker</var> as an argument. This algorithm returns a
+          serviceworker object <var>serviceworker</var>, which can be <code>undefined</code>.
+        </p>
+        <ol>
+          <li>Let <var>src</var> be the result of running the <a>steps
+          for processing the <code>src</code> member of a service worker</a>
+          with <var>serviceworker</var> and <var>manifest URL</var>.
+          </li>
+          <li>If <var>src</var> is <code>undefined</code>, or if the result of
+          running <a href="https://w3c.github.io/webappsec/specs/powerfulfeatures/#is-origin-trustworthy">Is origin potentially trustworthy</a>

Nit: throw in this URL in the dependencies section at the bottom of the document. 

> +          If they have different script URLs, last one wins.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>serviceworker</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>
+          <var>serviceworker</var> as an argument. This algorithm returns a
+          serviceworker object <var>serviceworker</var>, which can be <code>undefined</code>.
+        </p>
+        <ol>
+          <li>Let <var>src</var> be the result of running the <a>steps
+          for processing the <code>src</code> member of a service worker</a>
+          with <var>serviceworker</var> and <var>manifest URL</var>.
+          </li>
+          <li>If <var>src</var> is <code>undefined</code>, or if the result of
+          running <a href="https://w3c.github.io/webappsec/specs/powerfulfeatures/#is-origin-trustworthy">Is origin potentially trustworthy</a>

Won't the service worker registration attempt do this for us? 

> +          registration. Other service worker registrations can be done, for instance
+          by a script; if these have different scopes they will be considered separate
+          registrations. If they have the same scope and script URL, they coalesce.
+          If they have different script URLs, last one wins.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>serviceworker</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>
+          <var>serviceworker</var> as an argument. This algorithm returns a
+          serviceworker object <var>serviceworker</var>, which can be <code>undefined</code>.
+        </p>
+        <ol>
+          <li>Let <var>src</var> be the result of running the <a>steps
+          for processing the <code>src</code> member of a service worker</a>
+          with <var>serviceworker</var> and <var>manifest URL</var>.

Below, you are using `let service worker be` thus trashing this one? I don't think we need to send in a service worker here. Just process the members first, then "attempt to register the service worker"

> +          registration. Other service worker registrations can be done, for instance
+          by a script; if these have different scopes they will be considered separate
+          registrations. If they have the same scope and script URL, they coalesce.
+          If they have different script URLs, last one wins.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>serviceworker</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>
+          <var>serviceworker</var> as an argument. This algorithm returns a
+          serviceworker object <var>serviceworker</var>, which can be <code>undefined</code>.
+        </p>
+        <ol>
+          <li>Let <var>src</var> be the result of running the <a>steps
+          for processing the <code>src</code> member of a service worker</a>
+          with <var>serviceworker</var> and <var>manifest URL</var>.

Sorry, this comment is in the wrong place! 

> +        </h3>
+        <p>
+          The <dfn><code>serviceworker</code> member</dfn> describes a
+          service worker as defined in [[!SERVICE-WORKERS-1]].
+        </p>
+        <p>
+          The <a><code>serviceworker</code> member</a> represents a service worker
+          registration. Other service worker registrations can be done, for instance
+          by a script; if these have different scopes they will be considered separate
+          registrations. If they have the same scope and script URL, they coalesce.
+          If they have different script URLs, last one wins.
+        </p>
+        <p>
+          The <dfn>steps for processing the <code>serviceworker</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>

Below, you are using `let service worker be...` thus trashing this one? I don't think we need to send in a service worker here. Just process the members first, then "attempt to register the service worker"

> +        <p>
+          The <dfn>steps for processing the <code>serviceworker</code>
+          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>
+          <var>serviceworker</var> as an argument. This algorithm returns a
+          serviceworker object <var>serviceworker</var>, which can be <code>undefined</code>.
+        </p>
+        <ol>
+          <li>Let <var>src</var> be the result of running the <a>steps
+          for processing the <code>src</code> member of a service worker</a>
+          with <var>serviceworker</var> and <var>manifest URL</var>.
+          </li>
+          <li>If <var>src</var> is <code>undefined</code>, or if the result of
+          running <a href="https://w3c.github.io/webappsec/specs/powerfulfeatures/#is-origin-trustworthy">Is origin potentially trustworthy</a>
+          with the origin of <var>src</var> is <code>Not Trusted</code>, return
+          <code>undefined</code>.

screwing up the security aspects of the registration is a pretty big error... so we should maybe pass this through and just allow service worker registration to fail (and kill the whole object then)

> +          member</dfn> are given by the following algorithm. The algorithm
+          takes a <a>manifest</a> <var>manifest</var> and <a>serviceworker object</a>
+          <var>serviceworker</var> as an argument. This algorithm returns a
+          serviceworker object <var>serviceworker</var>, which can be <code>undefined</code>.
+        </p>
+        <ol>
+          <li>Let <var>src</var> be the result of running the <a>steps
+          for processing the <code>src</code> member of a service worker</a>
+          with <var>serviceworker</var> and <var>manifest URL</var>.
+          </li>
+          <li>If <var>src</var> is <code>undefined</code>, or if the result of
+          running <a href="https://w3c.github.io/webappsec/specs/powerfulfeatures/#is-origin-trustworthy">Is origin potentially trustworthy</a>
+          with the origin of <var>src</var> is <code>Not Trusted</code>, return
+          <code>undefined</code>.
+          </li>
+          <li>Otherwise, let <var>serviceworker</var> be an object with

nit: ` let <var>serviceworker</var> ` might confuse people into thinking this the actual service worker... maybe "Let `registration properties` be..."

> +          </li>
+          <li>Let <var>type</var> be the result of running the <a>steps
+          for processing the <code>scope</code> member of a service worker</a>
+          passing <var>serviceworker</var>.
+          </li>
+          <li>If <var>scope</var> is not <code>undefined</code>,
+          set<var>serviceworker</var>'s <code>scope</code>
+          property to be <var>scope</var>.
+          </li>
+          <li>Return <var>serviceworker</var>.
+          </li>
+        </ol>
+        <div class="example">
+          In the following example, the web application is listing
+          a service worker for the <code>/foo</code> scope:
+<pre class="">

nit: class=example?

-- 
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/manifest/pull/507#pullrequestreview-12158129