search

w3c / push-api

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

Progressing spec to CR #334

Open marcoscaceres opened 2 years ago

marcoscaceres commented 2 years ago

@martinthomson, @beverloo, I'm wondering if we can do the final push to move this spec to CR?

Would it be possible for you to triage the last remaining 16 issues and see which are worth addressing? I'm happy to help with any you identify, but would like to know what's blocking us finishing up.

I know a Push server for testing would help, but I think we will have to concede that it's probably not going to happen... but that's ok, we can still test some of the API surface and if we know of any egregious compat issues, we can deal with that in each engine.

marcoscaceres commented 2 years ago

Hi @martinthomson, @beverloo, I've done some triage... this is what I found:

CR blockers:

I'm unsure what to do with these 4 though:

Could use some guidance.

martinthomson commented 2 years ago

303 - Probably worth fixing here if we can. This isn't really a problem that this specification needs to fix, but the IETF documents aren't exactly open for business. I don't know if we can recommend that UA transitively insist that push services accept POST with CORS headers (Access-Control-Allow-* etc...), but I guess we could at least suggest that it might be a good idea.

300 - I think we can close/defer this. It is entirely possible for a push message to get through, with the app unable to access the network in order to do what it thinks is necessary based on that message. That will not result in the SW being woken again, but there are other options (background sync was quoted, but I think that died on the vine; timers are potentially possible also). Ultimately, this is an error condition that apps just need to deal with.

280 - I think that the ship sailed on this. Anne is right to insist on better here, but I don't see this as a barrier to CR.

292 - Defer. This is not a bad idea, but unless browsers are implementing it, I don't see a need for it. In practice, it seems like subscriptions are effectively perpetual, so the need for something like this is somewhat academic. Those cases that really need a change (database compromise, for instance) can probably tolerate a small window where messages might get lost.

beverloo commented 2 years ago

Thanks Marcos!

The userVisible seems to be always required... maybe it should just assume true and it could go away? userVisibleOnly should be standardized to match browser behaviour #313 ... some people argue that they need non-user-visible

It's not unreasonable that we might eventually allow silent push messages in highly privileged environments, but we have no active plans to pursue this. Using userVisibleOnly to signal that feels appropriate, but I don't feel strongly.

Clarify expirationTime Make PushSubscription.expirationTime mandatory #302

I agree in principle, quite strongly even, but we're not ready to act on this. This would be incompatible with much of the existing Web Push API usage, and needs a lot more thought.

303 - I agree that it's probably worth fixing, the idea is sound and fits well into the narrative we've built around these APIs. I'm not aware of any active developer demand however, so happy to defer this to some point after CR too.

300 - I do believe that Background Sync would be a sensible mechanism for this, as it's significant harder to abuse than retrying the push event on error conditions. However, as neither Firefox nor Safari have adopted it, I'm not comfortable pushing for more than a suggestion. The "sorta online" category of error conditions exists everywhere.

280 - Although most of the base64url decoding implementations just substitute characters (& thus work fine with either input, modulo the trailing equal-signs that the forgiving algorithm seems to accomodate for), there is a lot of tooling out there, across client and server, and no way for us to know what would break. We shouldn't change this.

292 - We would like this, but evidently haven't properly prioritised it either. OK with deferring to after CR.

collimarco commented 1 year ago

I also propose this small clarification if you can take a look at it: https://github.com/w3c/push-api/issues/357