Re: [w3c/manifest] Add id member to manifest (#988)

@marcoscaceres requested changes on this pull request.

Started to implement but hit some issues. 

@philloooo, made some suggested changes to the algorithm. 

I didn't get a chance to feed the table values into it, but they could serve as unit tests for it.

> @@ -808,6 +811,90 @@ <h3>
           </p>
         </section>
       </section>
+      <section>
+        <h3>
+          `id` member
+        </h3>
+        <p>
+          The [=manifest's=] <code><dfn data-export="" data-dfn-for=
+          "manifest" id="identity">id</dfn></code> member is a <a>string</a> that
+          represents the <dfn>identity</dfn> for the application. 

```suggestion
          represents the <dfn data-dfn-for="web-application">identity</dfn> for the application. 
```

> @@ -808,6 +811,90 @@ <h3>
           </p>
         </section>
       </section>
+      <section>
+        <h3>
+          `id` member
+        </h3>
+        <p>
+          The [=manifest's=] <code><dfn data-export="" data-dfn-for=
+          "manifest" id="identity">id</dfn></code> member is a <a>string</a> that
+          represents the <dfn>identity</dfn> for the application. 
+          The |identity| takes the form of a URL, which is same origin as the

You will need to make this change in a few places: 
```suggestion
          The [=web-application/identity=] takes the form of a URL, which is same origin as the
```

> @@ -1210,6 +1300,16 @@ <h3>
             Updating the manifest
           </h3>
           <aside class="issue" data-number="446"></aside>
+          <p>
+            User agents MAY, at any time, re-fetch and update a manifest for an

I'm a bit concerned about this from a privacy/security perspective... I think an update should only happen if the user reopens and is using the application. Basically, we should treat the update in the same way service workers do it (following a similar lifecycle). 

The determination to update then needs to go through all the fetching/processing steps again. 

Additionally, the user should have the ability to inspect the update. In particular, they might need to verify that the icons are not being changed to, for example, "bank.com". See, for example:
https://github.com/w3c/manifest/issues/446#issuecomment-206112421

![image](https://user-images.githubusercontent.com/870154/128294121-60d3153a-69d3-4dc4-ac02-447cd76b8669.png)




> +          </li>
+          <li>If |id| is not same origin as |manifest|["start_url"], return.
+          </li>
+          <li>If |id| is failure, return.
+          </li>
+        </ol>
+        <aside class="example">
+          Below table shows some example cases of the |identity| processing algorithm.
+          <table class="data">
+            <tr>
+              <th>|json|["id"]</th>
+              <th>|manifest|["start_url"]</th>
+              <th>|manifest|["id"]</th>
+            </tr>
+            <tr>
+              <td><i>missing</i></td>

```suggestion
              <td><I>undefined</i></td>
```

> +          <li>Set |manifest|["id"] to |manifest|["start_url"].
+          </li>
+          <li>If the type of |json|["id"] is not [=string=], return.
+          </li>
+          <li>Let |id:URL| be the result of [=URL Parser|parsing=] |json|["id"]
+          with |manifest|["start_ur"]'s origin as the base URL.
+          </li>
+          <li>If |id| is not same origin as |manifest|["start_url"], return.
+          </li>
+          <li>If |id| is failure, return.

hmm... the algorithm currently doesn't quite do as intended... here is the JS equivalent of what's currently specified:

```JS
/**
 * @param {Object} json
 * @param {Map<string, any>} manifest
 */
function processIdMember(json, manifest){
  // Set |manifest|["id"] to |manifest|["start_url"].
  manifest.set("id", manifest.get("start_url"));

  // If the type of |json|["id"] is not [=string=], return.
  if (typeof json.id !== "string") return;

  // Let |id:URL| be the result of [=URL Parser|parsing=] |json|["id"]
  // with |manifest|["start_ur"]'s origin as the base URL.
  const id = new URL(json.id, manifest.get("start_url"));

  // If |id| is not same origin as |manifest|["start_url"], return.
  if(id.origin !== manifest.get("start_url").origin)) return;

  // If |id| is failure, return.
}
```

I think what we want is:

```JS
/**
 * @param {Object} json
 * @param {Map<string, any>} manifest
 */
 function processIdMember(json, manifest) {
  const startURL = manifest.get("start_url");
  const fallbackId = new URL(startURL);
  let id;
  if (typeof json.id === "string") {
    try {
      id = new URL(json.id, startURL);
    } catch {
      // It failed, so use the fallbackId
      id = fallbackId;
    }
  }

  if (id.origin !== startURL.origin) {
    // user agent would warn here...
    id = fallbackId;
  }
  // finally, we have the id...
  manifest.set("id", id);
}
```




-- 
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/988#pullrequestreview-722936198