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

@mgiuca approved this pull request.

Great! I've dropped a bunch of comments that are mostly nits. Happy with how this ended up. Please take into account my comments, then merge.

I do suspect my previous review was based on an old version. :| sorry.

It can't be that you forgot to push. pr-preview hasn't updated since June 11.

>          </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-dfn-for="manifest" data-lt=
+        "within scope" data-export="">within scope</dfn> of a |manifest| if the

Could `|manifest|` get a type: `|manifest:processed manifest|` so when you mouse-over it, it says "processed manifest"?

"processed manifest" is a sort of informal type though, I'm not sure if this needs to be a formal WebIDL or infra type.

>          </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-dfn-for="manifest" data-lt=
+        "within scope" data-export="">within scope</dfn> of a |manifest| if the
+        |target| is [=URL/within scope=] of the [=manifest/navigation scope=]

s/the/_manifest_'s

>          </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-dfn-for="manifest" data-lt=
+        "within scope" data-export="">within scope</dfn> of a |manifest| if the
+        |target| is [=URL/within scope=] of the [=manifest/navigation scope=]
+        (i.e., [=URL/within scope=] of the {{WebAppManifest/scope}} member).

s/the/_manifest_'s

or

_manifest_.`scope`

> @@ -496,21 +493,20 @@ <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=]

Neither "active document" nor "URL" is linking for me. Can you check these refs?

> @@ -496,21 +493,20 @@ <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 [=manifest/within scope=] of the [=application context=]'s
+        [=processed manifest=], the user agent SHOULD show a prominent UI
+        element indicating the [=Document/URL=] or at least its [=origin=],
+        including whether it is served over a secure connection. This UI SHOULD
+        differ from any UI used when the [=Document/URL=] is [=manifest/within
+        scope=] of the [=application context=]'s [=processed manifest=], in

Echoing a previous comment: feel free to disagree here, it's a very minor nit: I would prefer to not say "processed manifest" everywhere, since it's implied by "within scope" that the manifest be processed.

I think it would read more cleanly to just say "application context's manifest". Similarly down below for deep links, and shortcut's `url` item, you could say "manifest" instead of "processed manifest".

-- 
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-436391340