Re: [w3c/push-api] Expose pushManager on Window (PR #393)

@saschanaz approved this pull request.

I don't see a blocker, just a few editorial things. I'm fine with dealing with them in separate issues.

> -          <a>push subscription</a> is associated with a <a>service worker registration</a> and a
-          <a>service worker registration</a> has at most one <a>push subscription</a>.
+          <a>user agent</a> and the <a>push service</a> on behalf of a web application.
+        </p>
+        <p>
+          A [=push subscription=] has an associated <dfn data-dfn-for=
+          "push subscription">scope</dfn>, which is a [=/URL=].
+        </p>
+        <p>
+          A [=push subscription=] is considered to have a <dfn data-dfn-for=
+          "push subscription">window-accessible scope</dfn> when its [=push subscription/scope=]'s
+          [=url/path=] is a [=/list=] of [=list/size=] 1 and [=push subscription/scope=]'s
+          [=url/path=][0] is the empty string.
+        </p>
+        <p class="note">
+          I.e., the [=url/path=] component of the [=/URL=] serializes as "`/`".

Can we just say this instead of checking the raw value? This is more readable and implementations can freely do the other way for optimization.

> @@ -721,8 +732,9 @@ <h2>
           creating the <a>push subscription</a>.
         </p>
         <p>
-          If the [=user agent=] has to change the keys for any reason, it MUST [=refresh=] the
-          [=push subscription=].
+          If the <a>user agent</a> has to change the keys of a [=push subscription=] for any reason
+          and the [=push subscription=]'s [=associated service worker registration=] is non-null,
+          it MUST [=refresh=] the [=push subscription=].

(Might want to mention #401 in a note)

>            </aside>
           <ol>
+            <li>If |scope| is failure or is a [=/URL=] whose [=url/scheme=] is not "`https`", then

I wonder we can reject earlier, but maybe it's more consistent to do it here. But then I think the scope getting step can be here rather than above.

> @@ -1103,6 +1157,27 @@ <h3>
         </li>
         <li>Let |global| be [=this=]'s [=relevant global object=].
         </li>
+        <li>Let |scope| be null.
+        </li>
+        <li>Let |registration| be null.
+        </li>
+        <li>If [=this=]'s [=PushManager/service worker registration=] is null:
+          <ol>
+            <li>Set |scope| to the result of running the [=basic URL parser=] given "`/`" and
+            |global|'s <a>associated <code>Document</code></a>'s [=Document/URL=].
+            </li>
+            <!-- Should the scope be validated here or do we trust the permission story to take care of that? Hmm. -->

Can't think of a specific concern, can you?

>              </li>
-            <li>When the request has been completed, [=queue a global task=] on the [=networking
-            task source=] using |global| to [=resolve=] |promise| with |subscription| and terminate
-            these steps.
+            <li>If |permission| is {{PermissionState/"denied"}}, [=queue a global task=] on the
+            [=user interaction task source=] using |global| to [=reject=] |promise| with a
+            {{"NotAllowedError"}} {{DOMException}} and terminate these steps.
+            </li>
+            <li>If |subscription| is non-null:
+              <ol>
+                <li>If there is an error with |subscription|, then [=queue a global task=] on the

Because throwing it earlier changes the order? Maybe that's a valid concern, although I think the order already is not compatible between implementations and we'll have to modify that part.

Maybe we can move the ["partial database corruption" part in `PushSubscriptionChangeEventInit`](https://w3c.github.io/push-api/#dom-pushsubscriptionchangeeventinit-oldsubscription) into PushSubscription to define what this is about, because with this change it's less clear what this error is about. (to be fair it was already unclear though.)

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/push-api/pull/393#pullrequestreview-3185154991
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/push-api/pull/393/review/3185154991@github.com>