Allow firing pushsubscriptionchange on all permission changes

[antosart]

Closes #236

The following tasks have been completed:

• Modified Web platform tests (link to pull request)
  No, because this allows and does not mandate firing the event. Moreover, WPT support for changing permissions is kind of limited at the moment.

Implementation commitment:

• Chromium (https://issues.chromium.org/issues/407523313)
• Gecko (This is implemented already, see https://bugzilla.mozilla.org/show_bug.cgi?id=1497429)
• WebKit (unknown)

---

Preview | Diff

[antosart]

This PR clarifies behavior that is currently already implemented by Gecko and I believe at least partially already implemented by WebKit (although I couldn't manually test it). Chromium would like to align (https://chromestatus.com/feature/5147683423256576).

Would it be fine to include this wording in the spec? Or would we need to follow any particular process here?

[saschanaz]

No, because this allows and does not mandate firing the event.

Why not mandate though if all browsers are going to do it?

We can still write WPT in that case though. I won't push this too strongly though because no browser can actually make any subscription from the upstream WPT CI. Firefox still can create subscriptions from our own WPT run, not sure Blink has the similar internal setup.

(We should really fix this together, it's painful. Mozilla will create WPT anyway for our own coverage and hopefully for others in the future.)

Moreover, WPT support for changing permissions is kind of limited at the moment.

Both Gecko and Blink support test_driver.set_permission() so this is not really true. (WebKit indeed doesn't support this, but having some test is better than having nothing)

[saschanaz]

This should probably use the revocation hook: https://w3c.github.io/permissions/#dfn-permission-revocation-algorithm

Would be nice if we can decide whether this should be on revocation, regranted, or both.

[antosart]

I added a note below, clarifying why it can make sense to leave this up to the implementor.

[saschanaz]

(This comment is not about the note though)

[saschanaz]

And maybe also worth getting feedback from Permisssion API people about whether it's a good idea to fire at regrant at all. I think probably there's a reason why there's no event for permission change?

cc @Trikolon

[antosart]

Right, this is why I didn't close the thread. I'm not sure how it looks to hook into the revocation algorithm for revocation while leaving the same wording for re-granting, so I would leave this open and wait for others' opinions.

[saschanaz]

(I'm told that @bakulf would know better about permission API, so redirecting the question to him.)

[antosart]

Why not mandate though if all browsers are going to do it?

It's not clear to me that we want to always mandate firing the event. There might be a lot of cases in which the permission state changes (part of this is vendor specific) and I'm not sure that the user agents always want to start a service worker. For example, if the permission is ungranted because of deleting site data (and yes that would also delete the service worker registration, but maybe afterwards) maybe the user agent wouldn't want to fire the event.

Both Gecko and Blink support test_driver.set_permission() so this is not really true. (WebKit indeed doesn't support this, but having some test is better than having nothing)

Right, but in Blink this only sets overridden permissions for testing and does not integrate properly with the permissions implementation. It would be quite some work to fix that, but I agree this is something we should eventually work on.

(I realized this while trying to write WPTs. I can try to finish them up, and test they work at least in Gecko.)

[saschanaz]

It's not clear to me that we want to always mandate firing the event. There might be a lot of cases in which the permission state changes (part of this is vendor specific) and I'm not sure that the user agents always want to start a service worker. For example, if the permission is ungranted because of deleting site data (and yes that would also delete the service worker registration, but maybe afterwards) maybe the user agent wouldn't want to fire the event.

Fair point. I think test_driver.set_permission would map to the "usual" permission change by user though, and then we can write WPT. And maybe it's worth having a note in the spec why it's not mandated.

[saschanaz]

I'll have to check the Firefox behavior again and will be able to access Safari for the same later today.

[saschanaz]

So I briefly revisited our implementation and filed https://bugzilla.mozilla.org/show_bug.cgi?id=1961806, with the following comment:

But the point of having pushsubscriptionchange event is to notify the app servers so that they can learn the given endpoint is now revoked and should not be pinged. It would probably make sense not to fire the event immediately after permission change, but would also be too late to fire it only on regrant.

If the permission is revoked by clearing site data, it would probably make sense to delay the event until the user visits the site next time.

I'm not sure the user would manually regrant the permission in general, one would usually do it from the prompt triggered by the website, so unclear whether firing the event on regrant is worth doing.

[annevk]

If the permission is revoked by clearing site data, it would probably make sense to delay the event until the user visits the site next time.

That sounds like a no-no. If you clear site data the next visit should be as if you visited the site anew. Though generally clearing site data does not revoke permissions, but push subscriptions are an interesting case.

[saschanaz]

If the permission is revoked by clearing site data, it would probably make sense to delay the event until the user visits the site next time.

That sounds like a no-no. If you clear site data the next visit should be as if you visited the site anew. Though generally clearing site data does not revoke permissions, but push subscriptions are an interesting case.

Hmm, another fair point. Given it would be awkward to start up the service worker that is just being cleared, maybe firing it on regrant is the best effort?

[saschanaz]

(Firing on regrant also sorta violates "it should be as if it's anew" but that's probably fine as the regrant requires explicit user interaction.)

Edit: Maybe no if it follows site data clearing, as mentioned below.

[annevk]

Is it? Why would the end user want to reveal they are the same as some prior end user?

[saschanaz]

If one cleared the permission but did not clear the site data, the site will know it anyway, right?

Edit: (I should have made it clear.)
Edit 2: It's a good point though, that means I should check again what Firefox does on site data clearing.

[antosart]

The points coming up in the discussion are exactly the reasons why I think it makes sense for the spec to leave it to the user agent to decide when it makes sense to fire and when it does not, for example depending on whether the permission change was the result of a user action and how that action was presented to the user.

As a concrete example, the behavior that chromium would like to implement is to fire upon re-grant in site settings. That allows users who removed permissions and want to explicitly grant them again to be able to start receiving notifications again. This is also what is already implemented by Gecko, according to what I was able to test.

[antosart]

I'm not sure the user would manually regrant the permission in general, one would usually do it from the prompt triggered by the website, so unclear whether firing the event on regrant is worth doing.

I think this again depends on the user agent and on how settings are presented to users. I believe in some cases this is worth doing.

[antosart]

Note also that the current version of the spec already allows vendors to fire the event when revoking permission (see the Security and Privacy Considerations). So this PR is not really changing anything there. The additional thing that this PR does is explicitly allowing firing the event when regranting permission, which I think is very vaguely already implied by the description of the event, but this PR makes very explicit.

Again, this behavior is already implemented by Firefox.

[saschanaz]

Firing the event when revoking makes sense because we want app servers to know that the endpoint is invalidated. This PR is a bit further than that as this is paired to the blink prototype that wants to do auto resubscription from regrant, which we do not implement.

Maybe this PR should be more about resubscription than regrant. In section 3.3.1:

A user agent or push service MAY choose to refresh a push subscription at any time, for example because it has reached a certain age.

We can say a user agent may choose to revive a subscription at any time, and then we can stop discussing about regrant because resubscription on regrant will be an implementation detail.

[antosart]

the blink prototype that wants to do auto resubscription from regrant

That is not really true. The resubscription would happen after regrant only if the service worker, listening to the pushsubscriptionchange event, actually triggers a new subscription. We thought about creating a new subscription first and then triggering the pushsubscriptionchange event with newSubscription set, but the potential drawback is that creating a subscription has some infrastructural cost that could end up being wasted in a lot of cases, if the service worker doesn't end up actually resubscribing. That's also why the formulation

We can say a user agent may choose to revive a subscription at any time, and then we can stop discussing about regrant because resubscription on regrant will be an implementation detail.

is I believe nonoptimal, because that would force creating a new subscription beforehand.

[saschanaz]

Ah thanks, that definitely was my misunderstanding. I'm sorry.

I feel like it can still boil down to something like "A user agent may fire pushsubscriptionevent at any time when there's a deactivated subscription that hasn't fired pushsubscriptionevent yet"? I think we can generally agree that we want to fire pushsubscriptionchange event eventually, the only question is when, and that can be implementation detail.

BTW, I think we need to revise section 3.3.2 btw, because if we want to fire pushsubscriptionevent any later than when the permission is revoked, this can't always be true for user agent:

When a push subscription is deactivated, both the user agent and the push service MUST delete any stored copies of its details.

The browser would need to store the subscription data to fire the event later. (Alternatively the event can just have null for oldSubscription, but not sure we'd want to force that.)

[antosart]

I feel like it can still boil down to something like "A user agent may fire pushsubscriptionevent at any time when there's a deactivated subscription that hasn't fired pushsubscriptionevent yet"? I think we can generally agree that we want to fire pushsubscriptionchange event eventually, the only question is when, and that can be implementation detail.

I guess that is also fine. I don't even think we need to say "hasn't fired yet". I have the impression that basically the event means "something's changed" and that is also how currently it is interpreted by web developers, who just try to resubscribe upon receiving it. Clarifying some circumstances in which it might make sense to fire the event, as this PR does, also seems meaningful though, no?

[antosart]

Btw, I tried writing WPT, but:

• on Firefox I can't get push subscriptions working in WPTs,
• on Chrome the permission change through the WPT runner is not implemented properly (basically it's just an overlay on top of the real permission states)
• on Safari changing permissions doesn't seem to work in WPT.

There's definitely work to do here, but maybe not for this PR...?

[annevk]

Can you describe your WPT findings in a new issue that we use to track adding WPT coverage at some point? (Probably best against this repository, though a mirroring issue against WPT might be worthwhile.)

[saschanaz]

I guess that is also fine. I don't even think we need to say "hasn't fired yet". I have the impression that basically the event means "something's changed" and that is also how currently it is interpreted by web developers, who just try to resubscribe upon receiving it. Clarifying some circumstances in which it might make sense to fire the event, as this PR does, also seems meaningful though, no?

Yeah, just not sure there can be more than one change for a push subscription after initial subscription - namely unsubscription. And not sure firing it for anything else would be web compatible given existing callers may think all events indicate unsubscription (and proceed to ping the app server to unsubscribe). What other changes do you expect?

[saschanaz]

Can you describe your WPT findings in a new issue that we use to track adding WPT coverage at some point? (Probably best against this repository, though a mirroring issue against WPT might be worthwhile.)

👍🏻, but maybe in #365 ?