WICG / web-app-launch

Web App Launch Handler
Other
75 stars 28 forks source link

Fix Jake's Feedback #16

Closed fallaciousreasoning closed 5 years ago

fallaciousreasoning commented 5 years ago

This responds to some of the feedback in #15

fallaciousreasoning commented 5 years ago

Ah, interesting. So calling event.respondWith stops any more fetch handlers from seeing the fetch event?

fallaciousreasoning commented 5 years ago

So I think there are three problems here:

1. The case when there are multiple handlers.

We need to synchronously take ownership of handling an event, so that two launch events don't attempt to handle the same launch (potentially resulting in multiple windows being opened).

My preference here is that we have a method on LaunchEvent, analogous to respondWith on fetch (perhaps handleLaunch, launchWith or launchVia).

2. The case where there are multiple events.

If multiple events are triggered in quick succession there is a possible race condition (imagine an app that intends to always focus an existing window. The first event opens a new window, the second event also opens a new window, as the first window wasn't created before it started handling the event).

There are two options here. We can expect apps to handle this, or we could only allow a ServiceWorker to be processing one launch event at a time. My preference here is to only have one launch event active at a time (though I could see this being a performance concern).

3. How the app signals it wants the default behavior for whatever caused the navigation.

It needs to be possible to do this asynchronously. Initially, we planned on doing the default unless the application explicitly called preventDefault. However, more recently we have been thinking that this should be implicit (i.e. if you don't open/focus a window or show a navigation, we do the default). I feel like this should be less error prone.


What do you think @jakearchibald? I don't have much background here, so it's entirely possible I'm overlooking something or planning on reinventing the wheel. Does this sound reasonable?

jakearchibald commented 5 years ago

My preference here is that we have a method on LaunchEvent, analogous to respondWith on fetch (perhaps handleLaunch, launchWith or launchVia).

It might be difficult to justify if this is just a shortcut for preventDefault + stopImmediatePropogation.

The case where there are multiple events.

Ohh, I hadn't thought of this. Good catch.

or we could only allow a ServiceWorker to be processing one launch event at a time.

This could be ok. Multiple launch events at once seems unlikely, right?

Are we defining 'active' in terms of waitUntil?

we planned on doing the default unless the application explicitly called preventDefault. However, more recently we have been thinking that this should be implicit (i.e. if you don't open/focus a window or show a notification, we do the default).

I think implicit would be really hard to track. Imagine a launch event and a push event landed at roughly the same time. showNotification is called. Was the launch handled?

fallaciousreasoning commented 5 years ago

It might be difficult to justify if this is just a shortcut for preventDefault + stopImmediatePropogation

I was picturing it more like respondWith.

addEventListener('launch', e => {
    e.launchVia(async () => {
        // TODO: Async launch stuff
    });
});

So it would be shorthand for waitUntil, preventDefault and stopPropogation, which I think makes it slightly more acceptable. WDYT? If the launch handler didn't open/focus/notify before the promise passed to launchVia, then we would do the default launch action.

This could be ok. Multiple launch events at once seems unlikely, right?

It definitely shouldn't be common. I was thinking of the case where a user rapidly opens a number of files by double clicking on each one.

Are we defining 'active' in terms of waitUntil?

Yup, in terms of waitUntil or launchVia, if we had that.

I think implicit would be really hard to track

Hmm, that's a good point. I was assuming we have some kind of infrastructure for this already in place. Isn't it a requirement that push notifications show a notification? (I just had a look and couldn't find anything to back this up, maybe I just invented it).

I think even if we required developers to call preventDefault we'd need some way of detecting whether the application opened/focused/notified, so we can stop apps from silently swallowing launches :/ Any ideas how we could go about this?

jakearchibald commented 5 years ago

Isn't it a requirement that push notifications show a notification?

Yeah. The reason is we don't want the app to use up resources in the background without the user being reminded that the app has those permissions. So in this case it doesn't really matter if the notification was shown in relation to a different event that happened at the same time, the important thing is a notification is shown.

Whereas here, it seems like you want to guarantee that the developer has responded to that particular launch event.

Any ideas how we could go about this?

I'll have a think, but right now I can only see two directions.