Re: [w3c/manifest] chore: refactor and export scope terms (#882)

@mgiuca commented on this pull request.



>          </li>
-        <li>Otherwise, return <code>false</code>.
+        <li>Return a [=boolean=] indicating if |targetPath| starts with

s/if/whether

>          </li>
       </ol>
       <p>
-        A <a>URL</a> <var>target</var> is said to be <dfn data-lt=
-        "within-scope-manifest">within scope of a manifest</dfn>
-        <var>manifest</var> if <var>target</var> is <a>within scope</a> of the
-        navigation scope of <var>manifest</var>.
+        A [=URL=] |target:URL| is <dfn data-export="" data-dfn-for=
+        "processed manifest" data-lt="within scope">within navigation scope of

Sorry I just ... can't get my head around GitHub's commenting system. It is truly the worst code review system I have ever used (including email). I don't seem to be able to reply to a comment thread above, so starting a new one...

> Looks good, but I still would prefer to call it "within scope of a manifest" rather than "within the navigation scope of a processed manifest" (the "navigation scope" is implied, and the "processed" is explicit later).
>
> And I don't understand why you say "within scope of the processed manifest's scope member" when you could say "within scope of the navigation scope of the processed manifest"; now those are defined as equivalent, so you may as well use the term you defined.

You didn't address this comment. Do you disagree or did you just miss it?

> @@ -496,21 +493,21 @@ <h2>
         unexpected behavior, use a scope ending in a <code>/</code>.
       </div>
       <p>
-        If the <a>application context</a>'s <a>active document</a>'s
-        [=Document/URL=] is not <a data-lt="within-scope-manifest">within
-        scope</a> of the <a>application context</a>'s manifest, the user agent
-        SHOULD show a prominent UI element indicating the [=Document/URL=] or
-        at least its <a>origin</a>, including whether it is served over a
-        secure connection. This UI SHOULD differ from any UI used when the
-        [=Document/URL=] is <a>within scope</a>, in order to make it obvious
-        that the user is navigating off scope.
+        If the [=application context=]'s [=active document=]'s [=Document/URL=]
+        is not [=processed manifest/within scope=] of the [=application
+        context=]'s [=processed manifest=], the user agent SHOULD show a

I noticed that you changed all references to this from "of the application context's manifest" to "of the application context's processed manifest". That has two issues:

1. It's quite verbose to always have to say "processed manifest".
2. It implies that an application context has both an unprocessed and processed manifest applied to it, or that either an unprocessed or a processed manifest can be applied. I don't think it should be necessary to make this distinction.

I think it should be possible to say "within scope of the application context's manifest" and know that that unambiguously means the manifest has been processed. If you don't think it means that, perhaps we can change the definition of "application context" or "applied" to explicitly say that it's a processed manifest that's applied.

> @@ -437,11 +437,11 @@ <h2>
         Navigation scope
       </h2>
       <p data-link-for="WebAppManifest">
-        A <dfn data-export="">navigation scope</dfn> is a <a>URL</a> that
-        represents the set of URLs to which an <a>application context</a> can
-        be navigated while the manifest is <a>applied</a>. The <a>navigation
-        scope</a> of a manifest <var>manifest</var> is
-        <var>manifest</var>["<a>scope</a>"].
+        The {{WebAppManifest/scope}} member of a [=processed manifest=] is the

Actually, just realised that this changes "navigation scope" to be a property of the application context, not the manifest. I think that's a bit backwards, because then you have this twisted structure:

- The `scope` is a property of the processed manifest.
- The "navigation scope" is a property of the application context (referring to the processed manifest applied to the application context).
- A URL can be "within scope" of a URL.
- A URL can be "within scope" of a manifest (referring to the navigation scope of the application context the manifest is applied to).
- Lots of definitions are concerned with whether a URL is "within scope of an application context's manifest" (referring to the processed manifest applied to the application context).

So if something says "if a URL is within scope of the application context's manifest", expanding out all of these definitions require that we:

1. Start with an application context,
2. Get the manifest applied to it (argument to "within scope of a manifest"),
3. Get the application context it is applied to (argument to "navigation scope"),
4. Get the manifest applied to it (to get the `scope` property),
5. Check whether the `scope` is a prefix of URL.

We should be able to cut out steps 3 and 4. I think this can be achieved by navigation scope continuing to be a property of a manifest, not an application context.

We could also define "navigation scope of an application context" and "within scope of an application context", but I think that's overkill.

-- 
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/882#pullrequestreview-424975824