Re: [w3c/manifest] Rewrite installation process and install prompting logic (#790)

marcoscaceres commented on this pull request.

Looking pretty good @mgiuca. Thanks for cleaning this up. Just one small issue I found with "return result". Rest are mostly just nits. 

>          </p>
+        <ol>
+          <li>Show some user-agent-specific UI, asking the user whether to
+          proceed with installing the app. See <a href=
+          "#installation-sec">privacy and security considerations</a> for
+          recommendations relating to this UI. The <var>result</var> of this
+          choice is either <a data-link-for=
+          "AppBannerPromptOutcome">accepted</a> or <a data-link-for=
+          "AppBannerPromptOutcome">dismissed</a>.
+          </li>
+          <li>If <var>result</var> is <a data-link-for=
+          "AppBannerPromptOutcome">accepted</a>, then in parallel, run the <a>
+            steps to install the web application</a>.
+          </li>
+          <li>Return <var>result</var>.

This is ambiguous: is `result` returned before "in parallel" runs, or after? As it reads (to me), it's returning before, which I think is incorrect, right? 

> @@ -457,9 +479,10 @@ <h2>
           <li>Wait until the {{Document}} of the <a>top-level browsing
           context</a> is <a>completely loaded</a>.
           </li>
-          <li>If there is already an <a>installation process</a> being
-          <a data-lt="present an install prompt">presented</a>, terminate this
-          algorithm.
+          <li>If there is already an <a data-lt=

We probably need need some kind of internal slot to track this, which can be reset as part of task once installation completes (and can hook into manual installation)... we don't need  to do it as part of this PR tho. 

> @@ -287,6 +287,35 @@ <h2>
         and again as an example, the user agent could <a>install</a> the web
         application into a list of bookmarks within the user agent itself.
       </p>
+      <p>
+        A {{Document}} may either be <dfn>installable</dfn> or not. The initial
+        state of a document is non-<a>installable</a>.

```suggestion
        state of a document is not <a>installable</a>.
```

> +      </p>
+      <ol>
+        <li>Let <var>manifest</var> and <var>manifest URL</var> be the result
+        of <a>obtaining the manifest</a>.
+        </li>
+        <li>If <a>obtaining the manifest</a> results in an error, the user
+        agent MAY either:
+          <ol>
+            <li>Fall back to using the <a>top-level browsing context</a>
+            {{Document}}'s metadata to to populate <var>manifest</var> in a
+            user-agent-specific way (e.g., setting
+            <var>manifest</var>.<a data-link-for="WebAppManifest">`name`</a> to
+            the document <a data-cite="HTML#the-title-element">`title`</a>) and
+            considering the document <a>installable</a>, or
+            </li>
+            <li>Consider the document non-<a>installable</a>.


```suggestion
            <li>Or, Consider the document not <a>installable</a>.
```
 

> +        At any time, the user agent MAY perform the <dfn>steps to determine
+        installability of the document</dfn>:
+      </p>
+      <ol>
+        <li>Let <var>manifest</var> and <var>manifest URL</var> be the result
+        of <a>obtaining the manifest</a>.
+        </li>
+        <li>If <a>obtaining the manifest</a> results in an error, the user
+        agent MAY either:
+          <ol>
+            <li>Fall back to using the <a>top-level browsing context</a>
+            {{Document}}'s metadata to to populate <var>manifest</var> in a
+            user-agent-specific way (e.g., setting
+            <var>manifest</var>.<a data-link-for="WebAppManifest">`name`</a> to
+            the document <a data-cite="HTML#the-title-element">`title`</a>) and
+            considering the document <a>installable</a>, or

Too easy to miss the "or" right at the end there... 
```suggestion
            considering the document <a>installable</a>.
```

> +      </p>
+      <p>
+        At any time, the user agent MAY perform the <dfn>steps to determine
+        installability of the document</dfn>:
+      </p>
+      <ol>
+        <li>Let <var>manifest</var> and <var>manifest URL</var> be the result
+        of <a>obtaining the manifest</a>.
+        </li>
+        <li>If <a>obtaining the manifest</a> results in an error, the user
+        agent MAY either:
+          <ol>
+            <li>Fall back to using the <a>top-level browsing context</a>
+            {{Document}}'s metadata to to populate <var>manifest</var> in a
+            user-agent-specific way (e.g., setting
+            <var>manifest</var>.<a data-link-for="WebAppManifest">`name`</a> to

```suggestion
            |manifest|.{{WebAppManifest/name}} to
```

> -                  <var>promise</var>, <var>client</var>, <a>manifest URL</a>,
-                  plus the <var>type</var> and <var>update_via_cache</var>
-                  members of the <var>registration</var>, in which case the
-                  state of the settled <var>promise</var> determines whether
-                  the <a>installation succeeded</a> or not.
-                  </li>
-                </ol>
-              </li>
-              <li>If the <a>installation succeeded</a>, <a>queue a task</a> on
-              the <a>application life-cycle task source</a> to <a>fire an
-              event</a> named <code>appinstalled</code> at the
-              <var>window</var> object.
+              <li>Let <var>client</var> be the <a>top-level browsing
+              context</a> {{Document}}'s <a>relevant settings object</a>, or
+              <code>null</code> if unavailable.
+              </li><!-- Start Register no longer exists in the Service Worker

Do we have some other alternative if Start Register is gone? 

> -          trigger the <a>installation process</a> through the user agent's
-          <abbr>UI</abbr>.
-          </li>
-          <li>The <a>installation process</a> can occur through an
-          <dfn>automated install prompt</dfn>: that is, a <abbr>UI</abbr> that
-          the user agent presents to the user when, for instance, there are
-          sufficient <a>installability signals</a> to warrant
-          <a>installation</a> of the web application.
-          </li>
-          <li>The <a>installation process</a> can occur through a
-          <dfn>site-triggered install prompt</dfn>: the site can
-          programmatically request that the user agent present an install
-          prompt to the user. The user agent MAY restrict the availability of
-          this feature to cases where, for instance, there are sufficient
+          trigger the installation process through the user agent's
+          <abbr>UI</abbr>, directly invoking the steps to <a>present an install

Just a note... `<abbr title="User Interface">UI</abbr>` just needs to be defined once in the document and ReSpec will find all instances of UI and add the right things for you. 

> @@ -457,9 +479,10 @@ <h2>
           <li>Wait until the {{Document}} of the <a>top-level browsing
           context</a> is <a>completely loaded</a>.
           </li>
-          <li>If there is already an <a>installation process</a> being
-          <a data-lt="present an install prompt">presented</a>, terminate this
-          algorithm.
+          <li>If there is already an <a data-lt=
+          "present an install prompt">install prompt being presented</a> or if

note: you shouldn't need the `data-lt` here.   

> @@ -487,14 +510,15 @@ <h3 id="installation-sec">
           Privacy and security considerations
         </h3>
         <p>
-          During the <a>installation process</a>, it is RECOMMENDED that the
-          user agent allow the end-user to inspect the icon, name, <a>start
-          URL</a>, origin, etc. pertaining to a web application. This is to
-          give an end-user an opportunity to make a conscious decision to
-          approve, and possibly modify, the information pertaining to the web
-          application before installing it. This also gives the end-user an
-          opportunity to discern if the web application is spoofing another web
-          application, by, for example, using an unexpected icon or name.
+          During the <a data-lt="present an install prompt">presentation of the

Note, you can put the `data-lt="presentation of the install prompt"` on the `dfn` instead, then you don't need the `data-lt` on the  `a` below. 

-- 
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/790#pullrequestreview-287177839

Received on Thursday, 12 September 2019 03:54:59 UTC