w3c / push-api

Push API
https://w3c.github.io/push-api/
Other
144 stars 40 forks source link

pushsubscriptionchange event doesn't have oldSubscription in it. #325

Open novaknole opened 4 years ago

novaknole commented 4 years ago

Hi...

It's finally assumed that pushsubscriptionchange never fires in chrome. and It's somehow understandable since their subscriptions never expire. Before we decide to start discussing this, I must say that's what information I got here https://bugs.chromium.org/p/chromium/issues/detail?id=1078367&q=Pri%3D%223%22#makechanges.

Question 1)

So I still decided to use this event for mozilla users. and I want to use event.oldSubscription and event.newSubscription in there. The way I triggered this event is simply by blocking and allowing notifications from mozilla's preference's notification's settings.

Question: oldSubscription and newSubscription are both undefined. Why? could this be because of the way I trigger it? and if mozilla itself triggers this, would they NOT be undefined at that stage?

Question 2)

I have seen on the mozilla docs page, that the way it handles pushsubscriptionchange event is that in that event, it tries to subscribe again by using the event.oldSubscription. https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerGlobalScope/pushsubscriptionchange_event .

Question: Is this the standard approach or would the last lines of code on the following website https://pushpad.xyz/service-worker.js be enough?

marcoscaceres commented 4 years ago

@linacambridge, not sure if you remember much fo the details about our implementation?

Or @martinthomson, it almost sound like our implementation might not much the spec anymore?

linabutler commented 4 years ago

Yep, our implementation is super out of date 😭 There's a bug on file for pushsubscriptionchange specifically, and a more general meta bug for bringing Firefox's implementation up to the latest spec.

novaknole commented 4 years ago

@linacambridge Thanks for the heads up.

So to sum up, I will ask a couple of questions.

1) I can't rely on having old and new subscriptions in the event object of the pushsubscriptionchange. Right?

2) pushsubscriptionchange will still get called if mozilla decided to refresh the token. right?

3) If the above (2) is correct, should I have the code to auto subscribe a user again and send the subscription to the server?

4) how do I get the old subscription in there so that I can send old one with the new one to the server so that I can gracefully replace old one with a new one?

5) Is it even worth using this event at all ?

Thanks.

linabutler commented 4 years ago

Hey @novaknole, thanks for following up!

  1. Right.
  2. Right. Firefox push subscriptions don't have a known expiration in advance, but the push server can drop them anytime. Firefox will also trigger pushsubscriptionchange if the user grants permission again after revoking it, like you discovered, or when the origin quota for the number of push messages is refreshed. So...
  3. Yes, it's good to handle the event by resubscribing. In the latest draft of the spec, old and new are supplied automatically, so the browser makes a new subscription behind the scenes, and only passes it to pushsubscriptionchange once it's done. In the old draft that Firefox implements now, you have to call registration.pushManager.subscribe() again yourself. But...
  4. You can't 😭 By the time the pushsubscriptionchange is triggered, the subscription is already gone. You'd need to use a session cookie or token stored in IndexedDB to identify the user, and overwrite their old subscription details with the new one based on that. The Pushpad founder blogged about that limitation a while back.
  5. It's a judgment call. I want to say "yes", since Firefox does trigger it, but I know it's really, really, really annoying to handle now, with all the caveats in (3) and (4). I can't say when Firefox will support the new event, since that work hasn't been prioritized yet—I haven't worked on push in a couple of years, and our wonderful DOM team is at capacity working on other projects.

I wish I had a more satisfying answer for you than "yes, but it's kinda broken now, and you have to use a super clunky workaround to make it work"...but that's the state of things today. Thank you for taking the time to write up all your questions, it's definitely appreciated!

novaknole commented 4 years ago

@linacambridge Wheeew !!! Thanks so much for these answers. Finally, someone out there who just shed all the lights about this fuss.

This is hopefully the last question I am gonna ask !

So, you mention In the old draft that Firefox implements now and In the latest draft of the spec.

1) So what is the old draft ? is it the spec from w3c-push that was written way before?

2) So funny. So on firefox , pushsubscriptionchange event would NEVER,NEVER,NEVER have old and new subscriptions OTHER than NULL .. Right? :D

3) I am 99.9% sure of this, but still, the same behaviour would occur even after I install the app as PWA and open the app from there ?

linabutler commented 4 years ago

@novaknole No problem, I'm really glad that was helpful, and thanks for being so understanding!

  1. Yep, by "old draft", I meant an old version of the Push API spec. oldSubscription and newSubscription got added in PR #234; before that, it was a simple ExtendableEvent with no special properties.
  2. Right. In Firefox, event.oldSubscription === undefined and event.newSubscription === undefined, because it doesn't implement them yet. Here's the code inside of Firefox that creates and triggers pushsubscriptionchange—you can see it's creating a regular ExtendableEvent instead of a PushSubcriptionChangeEvent that the spec says we should create. That's also why Firefox's Web Platform Tests are expected to fail for this feature.
  3. Right, even if you install it as a PWA, oldSubscription and newSubscription will still be undefined.
muradsofi commented 2 years ago

I have a question about that.

If the user disables push notification after subscription and after allow I think user token change. But pushsubscriptionchange not executes is it normal?

marcoscaceres commented 2 years ago

Leaving this open, as we need to verify if the following is clear:

If the user disables push notification after subscription and after allow I think user token change. But pushsubscriptionchange not executes is it normal?

martinthomson commented 2 years ago

There are two things going on here, so maybe we could do a little to extract them:

  1. if a subscription is terminated, does pushsubscriptionchange fire?
  2. if a subscription changes, what information needs to be included in the event?

The original question was about the information in the event (2); but that question is about termination (1). I believe that the purpose of the event was to signal termination (with event.newSubscription == null), but if that has not been implemented. Maybe we need to reconsider that idea in the face of browsers not choosing to follow the spec. No point in a fictional specification.