Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different approaches to opening windows #97

Closed
adamroach opened this issue Jan 23, 2017 · 14 comments
Closed

Different approaches to opening windows #97

adamroach opened this issue Jan 23, 2017 · 14 comments
Assignees
Milestone

Comments

@adamroach
Copy link
Contributor

In the current Payment App Spec, user interaction is performed via a call to ServiceWorkerGlobalScope.clients.openWindow(). In @marcoscaceres' Payment Request Handler document, this is instead handled with a new openClientWindow() method on the PaymentRequestEvent.

There are pros and cons to each approach.

With ServiceWorkerGlobalScope.clients.openWindow(), we re-use the existing machinery and specification of Service Workers, albeit with a need to specify that the web browser make the determination that such a window is being opened in a web payment context and takes steps to render it sensible. This is not difficult, but it is implicit behavior.

With PaymentRequestEvent.openClientWindow(), the fact that this is a different kind of window altogether is clearer, and the binding between the window and its corresponding event is also clearer. On the downside, this approach requires us to respecify a substantial amount of existing handling -- further, by inventing a new affordance where an existing one seems to be adequate doesn't feel like it's very much in the spirit of the extensible web. (Some may recall that my original proposal included a separate mechanism for opening this window, but I decided that re-use was better than re-invention).

So, which approach do we want to use?

@dlongley
Copy link
Contributor

dlongley commented Jan 23, 2017

This seems like it could become a very common pattern for a variety of APIs that need to show UI for specific contexts. In fact, I would argue that there's a generalizable pattern here:

  1. Website triggers event E.
  2. Previously installed ServiceWorkers that are listening for (and have permission to handle) E receive it (with an optional step where the browser mediates which ServiceWorker(s) will receive it via user interaction or previously configured options).
  3. ServiceWorker calls E.openClientWindow() and does something in the context of the event.
  4. Profit ... or whatever.

I agree that we want to keep in the spirit of the extensible Web. However, even though there may be some mechanism for doing this already in a ServiceWorker, it seems that it is insufficient to explicitly cover the above "contextual" use case. Furthermore, this use case is likely to come up in a variety of places in the future -- not just payments, (e.g. credentials, intents, web share, verifiable claims, etc). So whatever we do, it should be done in a way that can be reused by other (future) APIs so that everyone isn't repeating themselves. That would be in keeping with the extensible Web -- it's just that perhaps we actually do need something a bit different here.

@marcoscaceres
Copy link
Member

This seems like it could become a very common pattern for a variety of APIs that need to show UI for specific contexts.... Furthermore, this use case is likely to come up in a variety of places in the future -- not just payments, (e.g. credentials, intents, web share, verifiable claims, etc).

... + Web Authentication...

And exactly. We need to find a solution for this generally and we should work with @mikewest and friends from WebApp Sec to find one.

Credit (blame?) goes to @jakearchibald for the idea behind e.openClientWindow(). I just gave it the terrible name.

@marcoscaceres
Copy link
Member

So, PaymentRequestEvent.openClientWindow() makes this window (client) special/built-for-purpose: binding it to the browser tab/window that initiated the PaymentRequest - and only allowing .openClientWindow() to be called once (it could just resolve with the same client), and for you to always receive, potentially, the same client. This means you can't spin up multiple overlays on a tab - you have to wait till it's closed before you can spin it up again.

Also, because you know that it was a PaymentRequestEvent that initiated it, the browser can provide a more considered UX/UI if need be. But the main thing remains that you can only get one of these - irrespective of the number of service workers, etc... if someone claims this client, then they own it till they release it (somehow... we would need to work out those details).

This frees the ServiceWorkerGlobalScope.clients.openWindow() clients to open normal browser tabs and continue to behave as they do today.

@jakearchibald
Copy link

albeit with a need to specify that the web browser make the determination that such a window is being opened in a web payment context and takes steps to render it sensible

Unfortunately this is towards impossible. In an asynchronous execution environment like JavaScript you can't really link a call to openWindow with a "payment" event unless it's done synchronously. This is one of the reasons we have event.waitUntil on all service worker events, rather than a single global waitUntil.

With PaymentRequestEvent.openClientWindow(), the fact that this is a different kind of window altogether is clearer, and the binding between the window and its corresponding event is also clearer. On the downside, this approach requires us to respecify a substantial amount of existing handling

I think you'd have that problem anyway, since you're wanting to display the client in a particular way, different to how openWindow usually works.

I'm open to refactoring the service worker spec to make these parts more reusable.

From the email I sent to the list:

This still relies on postMessage to communicate with the client. I'm in two minds about it, but there may be higher-level options that would allow the client to produce the response itself. Eg:

addEventListener("paymentrequest", event => {
  if (canHandleWithoutUi) {
    event.respondWith(response);
    return;
  }
  event.respondWith(event.showWindow(url));
});

And the window would have something like navigator.payments.provideResponse(response) which, when called, would close the window, and pass the result back to the service worker. Again, I'm in two minds about it, but it'd remove the need for the clunky postMessage work.

@dlongley
Copy link
Contributor

dlongley commented Jan 24, 2017

@jakearchibald,

And the window would have something like navigator.payments.provideResponse(response) which, when called, would close the window, and pass the result back to the service worker. Again, I'm in two minds about it, but it'd remove the need for the clunky postMessage work.

This is almost exactly how the original API proposal from the Web Payments Community Group worked. The polyfill for it was implemented with postMessage, but the aim was to eliminate those clunky bits in the actual API as you've suggested.

It may be that we can make this response bit more generic as well per the above.

@adamroach
Copy link
Contributor Author

I'm finding the arguments in favor of putting a new openClientWindow() method on the event to be increasingly appealing. I would have a couple of nits to pick about specific points being made, but they don't change the overall conclusions.

@ianbjacobs
Copy link
Contributor

As of 7 Feb 2017, @marcoscaceres has volunteered to work on this. I suggested that he work with @tommythorsen. We will check back in soon. Thanks!

@marcoscaceres
Copy link
Member

@ianbjacobs, could you kindly add me as a collaborator to this project. That will allow me to be assigned explicitly to tasks.

@ianbjacobs
Copy link
Contributor

Done! Thank you @marcoscaceres.

@marcoscaceres marcoscaceres self-assigned this Feb 8, 2017
@marcoscaceres
Copy link
Member

I don't think I'll be able to work on this until the Payment Request spec is done. We are pretty close to finishing that spec tho. If anyone wants to help over there, please do. Quicker we finish off that spec, the more we can focus on this one.

@tommythorsen
Copy link
Member

So, I added PR #106, where I try to put all of the suggestions in this thread into the specification.

@marcoscaceres, @adamroach, @dlongley: Do you think that PR will be enough to close this issue? Are you happy with that PR?

@marcoscaceres
Copy link
Member

Commented there. It's a good start.

@dlongley
Copy link
Contributor

@tommythorsen, I commented on the PR as well. I agree with @marcoscaceres that it's a good start.

@ianbjacobs ianbjacobs added this to the Mark in FPWD milestone Apr 4, 2017
@ianbjacobs
Copy link
Contributor

Closed in favor of issue #115 and we seem to be including our own algorithm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants