Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This introduces a new feature whereby push messages conforming to a certain JSON format directly create an end user notification and show it (possibly preceded by an enhanced push event). In addition to showing a notification, the app badge can be updated as well. This builds on whatwg/notifications#213 which adds URL members to notifications. Exposing PushManager outside of service workers is handled by #393.
saschanaz
left a comment
There was a problem hiding this comment.
My first round of review. Looks mostly fine but with some questions.
| </li> | ||
| <li>If |subscription| is null, then resolve |promise| with null. | ||
| </li> | ||
| <li>If there is an error with |subscription|, reject |promise| with a {{DOMException}} |
There was a problem hiding this comment.
(Same about error, might be nicer if we throw in the obtaining steps or just skip this. It's unclear what a caller should do if this function throws, so maybe implementations should just pretend that no subscription exists if it's corrupted or something. If there's a good reason to throw error, it would be nice to have a note about that.)
There was a problem hiding this comment.
This is just trying to preserve existing text.
This comment was marked as resolved.
This comment was marked as resolved.
This introduces a new feature whereby push messages conforming to a certain JSON format directly create an end user notification and show it (possibly preceded by an enhanced push event). In addition to showing a notification, the app badge can be updated as well. This builds on whatwg/notifications#213 which adds URL members to notifications. Exposing PushManager outside of service workers is handled by #393.
This introduces a new feature whereby push messages conforming to a certain JSON format directly create an end user notification and show it (possibly preceded by an enhanced push event). In addition to showing a notification, the app badge can be updated as well. This builds on whatwg/notifications#213 which adds URL members to notifications. Exposing PushManager outside of service workers is handled by #393.
This is part of the Declarative Web Push initiative (see #360). This makes window.pushManager work by making push subscriptions tied to a scope rather than a service worker registration. Most often push subscriptions remain 1:1 with service worker registrations, but the scope whose serialized path is "/" is treated specially from now on and can exist independently.
6e463a7 to
895dd36
Compare
|
A significant portion of this PR can be part of an editorial PR. I'll file a new one for them and merge it myself, for easier review here. |
saschanaz
left a comment
There was a problem hiding this comment.
I don't see a blocker, just a few editorial things. I'm fine with dealing with them in separate issues.
| [=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=]. |
|
Section 4 about security consideration has this:
We probably want to change that. (Either remove that or make it non-normative? Not sure why it should be defined again in a security section.) Edit: Maybe the Subscription deactivation section should say MUST for deactivation after unregistration. |
This is part of the Declarative Web Push initiative (see #360) and mainly makes sense when that is supported, although could be independently supported in theory.
This makes window.pushManager work by making push subscriptions tied to a scope rather than a service worker registration. Most often push subscriptions remain 1:1 with service worker registrations, but the scope whose serialized path is "/" is treated specially from now on and can exist independently.
This obsoletes #368.
The following tasks have been completed:
Implementer support:
Preview | Diff