Re: [w3c/push-api] Properly define the pushsubscriptionchange event (#234)

beverloo commented on this pull request.

Thank you for the feedback. Let's add this and build upon it.

> +          A <a>push subscription</a> has internal slots for a P-256 <a>ECDH</a> key pair and an
+          authentication secret in accordance with [[!WEBPUSH-ENCRYPTION]]. These slots MUST be
+          populated when creating the <a>push subscription</a>.
+        </p>
+        <p>
+          If the <a>user agent</a> has to change the keys for any reason,
+          it MUST <a>fire the pushsubscriptionchange event</a> with the
+          <a>service worker registration</a> associated with the <a>push subscription</a> as
+          <var>registration</var>, a <a>PushSubscription</a> instance representing the
+          <a>push subscription</a> having the old keys as <var>oldSubscription</var> and a
+          <a>PushSubscription</a> instance representing the <a>push subscription</a> having the new
+          keys as <var>newSubscription</var>.
+        </p>
+        <section>
+          <h2>
+            Subscription refreshes

Done

> +          <h2>
+            Subscription refreshes
+          </h2>
+          <p>
+            A <a>user agent</a> or <a>push service</a> MAY choose to <dfn>refresh</dfn> a
+            <a>push subscription</a> at any time, for example because it has reached a certain age.
+          </p>
+          <p>
+            When this happens, the <a>user agent</a> MUST create a new <a>push subscription</a>
+            with the <a>push service</a> on behalf of the application, using the
+            <a>PushSubscriptionOptions</a> that were provided for creating the current
+            <a>push subscription</a>. The new <a>push subscription</a> MUST have a key pair that's
+            different from the original subscription.
+          </p>
+          <p>
+            When successful, <a>user agent</a> then MUST <a>fire the pushsubscriptionchange event</a>

I added the following paragraph:

> If the user agent is not able to refresh the push subscription, it should periodically retry the refresh. When the push subscription can no longer be used, for example because it has expired, the user agent must fire the pushsubscriptionchange event with the service worker registration associated with the push subscription as registration, a PushSubscription instance representing the deactivating push subscription as oldSubscription and null as the newSubscription.

I like Kit's suggestion of setting `newSubscription` to `null` since it gives the developer _something_.

> +            <a>PushSubscriptionOptions</a> that were provided for creating the current
+            <a>push subscription</a>. The new <a>push subscription</a> MUST have a key pair that's
+            different from the original subscription.
+          </p>
+          <p>
+            When successful, <a>user agent</a> then MUST <a>fire the pushsubscriptionchange event</a>
+            with the <a>service worker registration</a> associated with the <a>push subscription</a>
+            as <var>registration</var>, a <a>PushSubscription</a> instance representing the initial
+            <a>push subscription</a> as <var>oldSubscription</var> and a <a>PushSubscription</a>
+            instance representing the new <a>push subscription</a> as <var>newSubscription</var>.
+          </p>
+          <p>
+            <a>User agents</a> MAY continue to accept messages for the old <a>push subscription</a>
+            for a brief amount of time, but MUST stop doing so once the first message for the
+            refreshed <a>push subscription</a> has been received, as this indicates that the
+            <a>application server</a> has propagated the subscription change.

Thanks! Adopted.

> +            with the <a>service worker registration</a> associated with the <a>push subscription</a>
+            as <var>registration</var>, a <a>PushSubscription</a> instance representing the initial
+            <a>push subscription</a> as <var>oldSubscription</var> and a <a>PushSubscription</a>
+            instance representing the new <a>push subscription</a> as <var>newSubscription</var>.
+          </p>
+          <p>
+            <a>User agents</a> MAY continue to accept messages for the old <a>push subscription</a>
+            for a brief amount of time, but MUST stop doing so once the first message for the
+            refreshed <a>push subscription</a> has been received, as this indicates that the
+            <a>application server</a> has propagated the subscription change.
+          </p>
+
+          <p class="issue">
+            The steps for creating a subscription should be separated from the
+            <a lt="PushManager.subscribe">subscribe</a> method into an algorithm of its own so that
+            it can be referenced here. Similarly, the <a>PushSubscriptionOptions</a> provided to the

https://github.com/w3c/push-api/issues/235. Removed here.

> @@ -423,8 +473,13 @@
         acquiring permission or determining the permission status.
       </p>
       <p>
-        When a permission is revoked, all <a>push subscriptions</a> created with that permission
-        MUST be <a>deactivated</a>.
+        When a permission is revoked, the <a>user agent</a> MAY

I actually think we found a case where we need to do this too. Let me file a more elaborate issue on it in a bit.

> @@ -423,8 +473,13 @@
         acquiring permission or determining the permission status.
       </p>
       <p>
-        When a permission is revoked, all <a>push subscriptions</a> created with that permission
-        MUST be <a>deactivated</a>.
+        When a permission is revoked, the <a>user agent</a> MAY
+        <a>fire the pushsubscriptionchange event</a> for subscriptions created with that permission,
+        with the <a>servce worker registration</a> associated with the <a>push subscription</a> as

Fixed.

> @@ -423,8 +473,13 @@
         acquiring permission or determining the permission status.
       </p>
       <p>
-        When a permission is revoked, all <a>push subscriptions</a> created with that permission
-        MUST be <a>deactivated</a>.
+        When a permission is revoked, the <a>user agent</a> MAY
+        <a>fire the pushsubscriptionchange event</a> for subscriptions created with that permission,
+        with the <a>servce worker registration</a> associated with the <a>push subscription</a> as
+        <var>registration</var>, a <a>PushSubscription</a> instance representing the
+        <a>push subscription</a> as <var>oldSubscription</var>, and <code>null</code> as
+        <var>newSubscription</var>. The <a>user agent</a> MUST, in parallel, <a>deactivate</a>

Done.

>            </li>
-          <li>Invoke the <a>Handle Functional Event</a> algorithm with a <a>service worker
-          registration</a> of <var>registration</var> and <var>callbackSteps</var> set to the
-          following steps:
+          <li>Set the <code>oldSubscription</code> attribute of <var>event</var> to
+          <var>oldSubscription</var>.
+          </li>
+          <li>Invoke the <a>Handle Functional Event</a> algorithm with <var>event</var>,
+          <var>registration</var> and <var>callbackSteps</var> set to the following steps:

Slightly rephrased.

> +          </pre>
+          <p>
+            The <dfn>newSubscription</dfn> attribute contains the details of the
+            <a>push subscription</a> that is valid per invocation of the <a>pushsubscriptionchange</a>
+            event. The value will be <code>null</code> when no new <a>push subscription</a> could be
+            established, for example because the <a>webapp</a> has lost <a>express permission</a>.
+          </p>
+          <p>
+            The <dfn>oldSubscription</dfn> attribute contains the details of the
+            <a>push subscription</a> that SHOULD NOT be used anymore. The value will be
+            <code>null</code> when the <a>user agent</a> is not able to provide the full set of
+            details, for example because of partial database corruption.
+          </p>
+          <p class="issue">
+            Should we include an attribute that indicates whether the <code>oldSubscription</code>
+            has been decisively invalidated? How would developers use this information?

Agreed. Thanks - removed.

-- 
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/push-api/pull/234#pullrequestreview-22526095

Received on Friday, 17 February 2017 16:25:07 UTC