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

@mgiuca commented on this pull request.



>        </p>
-      <ol>
-        <li>Let <var>scopePath</var> be the [=string/concatenation=] of
-        <var>scopes</var>'s <a data-cite="URL#concept-url-path">path</a>, using
-        U+002F (/).
+      <ol class="algorithm">
+        <li>If |target| and |scope| are not [=same origin=], return `false`.
+        </li>
+        <li>Let |scopePath:string| be the [=string/concatenation=] of

"using" is correct here. See the definition of [concatenation](https://infra.spec.whatwg.org/#string-concatenate). This is supposed to place "/" in between each piece of the path, not at the end of it.

Perhaps you could clarify it by writing "using the separator "/"" or "using "/" as a separator".

>          </li>
-        <li>If <var>target</var> is <a>same origin</a> as <var>scope</var> and
-        <var>targetPath</var> starts with <var>scopePath</var>, return
-        <code>true</code>.
+        <li>If |targetPath| starts with |scopePath:string|, return `true`.

Maybe this and the next line could be combined by saying "Return a Boolean indicating whether |targetPath| starts with |scopePath|." Or even just "Return true if |targetPath| starts with |scopePath|, or false otherwise."

Since if I were writing this in C++, I would write:

```c++
return targetPath.starts_with(scopePath);
```

not

```c++
if (targetPath.starts_with(scopePath))
  return true;
else
  return false;
```

>          </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 said to be <dfn data-export="" data-dfn-for=
+        "manifest" data-lt="within scope">within the navigation scope of a web

Can this term just be "within scope", otherwise it is again too verbose (as it is intended as a way to write a short-hand).

"**within the navigation scope of a web manifest** _manifest_" is longer than "**within scope** of the navigation scope of _manifest_", so there is no point using this short-hand if its name is that long.

>          </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 said to be <dfn data-export="" data-dfn-for=
+        "manifest" data-lt="within scope">within the navigation scope of a web
+        manifest</dfn> {{WebAppManifest}} |manifest:WebAppManifest| if [=URL=]
+        |target:URL| is [=manifest/within scope=] of the |manifest|'s

I think you mean [=URL/within scope=].

>          </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 said to be <dfn data-export="" data-dfn-for=
+        "manifest" data-lt="within scope">within the navigation scope of a web
+        manifest</dfn> {{WebAppManifest}} |manifest:WebAppManifest| if [=URL=]
+        |target:URL| is [=manifest/within scope=] of the |manifest|'s
+        {{WebAppManifest/scope}} member (once resolved).

"once resolved" -- this is a colloquial term (URL "resolution") but I don't think it's defined anywhere.

Can you say "once [=processed=]" (linking to "processing a manifest"), because that's really what you're saying: we're talking about the scope of the manifest after processing.

Better yet, move this to the definition of "navigation scope of a manifest" above. That is:

> The navigation scope of a manifest _manifest_ is _manifest_[scope], once manifest has been processed.

or

> The navigation scope of a **processed manifest** _manifest_ is _manifest_[scope].

Then, here, you should say "of the navigation scope of _manifest_" rather than "of the _manifest_'s scope member".

> @@ -1978,8 +1978,8 @@ <h3>
               URL</var> as the base URL. If the result is failure, <a>issue a
               developer warning</a> and [=iteration/continue=].
               </li>
-              <li>If <var>shortcut</var>["url"] is not <a>within scope</a> of
-              <var>manifest URL</var>, <a>issue a developer warning</a> and
+              <li>If <var>shortcut</var>["url"] is not [=URL/within scope=]

There seem to be changes leaking both ways between this and #886. I think you should not change this here since you have a separate PR to fix this bug.

>            <a>sequence</a>&lt;<a>ShortcutItem</a>&gt; <var>shortcuts</var> and a
-          <a>URL</a> <var>manifest URL</var>. This algorithm returns a
-          <a>sequence</a>&lt;<a>ShortcutItem</a>&gt;.
+          <a>URL</a> |manifest URL:URL|, and [=URL=] <var>scope URL</var>. This

Here too (leave this change for #886).

> @@ -2642,8 +2642,9 @@ <h3>
         </h3>
         <p>
           The <dfn>url</dfn> member of a <a>ShortcutItem</a> is the <a>URL</a>
-          <a data-lt="within scope of a manifest">within the application's
-          scope</a> that opens when the associated shortcut is activated.
+          that opens when the associated shortcut is activated. When resolved,
+          the {{ShortcutItem/url}} must be [=URL/within scope=] of scope URL,
+          it gets ignored.

This sentence is ungrammatical ("it gets ignored"). Also, do not use the keyword "must" in this definition which is not strictly imposing requirements. Just say "is the URL [=manifest/within scope=] of the manifest that opens when the associated shortcut is activated." (It doesn't have to be explicit about everything, since that's covered by algorithms elsewhere.)

> @@ -1140,8 +1138,9 @@ <h3>
           member</a> given <var>manifest</var>["<a>related_applications</a>"].
           </li>
           <li>Set <var>manifest</var>["<a>shortcuts</a>"] to the result of
-          running <a>processing the <code>shortcuts</code> member</a> given
-          <var>manifest</var>["<a>shortcuts</a>"] and <var>manifest URL</var>.
+          running <a>processing the `shortcuts` member</a> given
+          <var>manifest</var>["<a>shortcuts</a>"], <var>manifest URL</var>, and
+          <var>scope URL</var>.

Here too (leave this change for #886).

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