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

Need to define the interaction of payment API with navigation and browsing context destruction #360

Closed
bzbarsky opened this issue Dec 6, 2016 · 8 comments

Comments

@bzbarsky
Copy link

bzbarsky commented Dec 6, 2016

I don't see anything in the spec that describes what happens if any of the following happen while a payment request is in progress:

  1. Navigation to a different happens in the browsing context which contains the document/window that the payment request was created in.
  2. Navigation happens in some ancestor browsing context.
  3. The browsing context which contains the document/window the payment request was created in is torn down (e.g. iframe removed from the DOM).
  4. Some ancestor browsing context is torn down (e.g. tab which contains an iframe that made a payment request is closed).

Part of the issue here, of course, is that there is a bit of a lack of spec clarity around what happens to event listeners and Promises when these things happen. But even apart from that, it's not clear what should happen to the UA-provided payments UI when these things happen. It's also not clear what happens if these things happen between the user dismissing the browser-provided UI (e.g. accepting the payment) and the "user accepts the payment request algorithm" running.

@rsolomakhin
Copy link
Collaborator

The answer should be "abort all algorithms," IMHO. Does that sound OK? Specs usually don't speak about UI. So Chrome might chose to show a message "transaction cancelled due to navigation" in UI, but that should not be in the spec, so Firefox, for example, can do something different, if they want.

Whether the payment has been processed depends on when navigation or browser context destruction happened:

  1. Some payment apps complete transactions on merchant server side (Basic Card and Android Pay). If the navigation or browser context destruction happened before the PaynmentResponse is sent to the server, transaction is effectively cancelled. Otherwise, the transaction is effectively processed.
  2. Other payment apps complete transactions on payment app side (PayPal and AliPay). If the navigation or browser context destruction happened before the shopping cart and payment app parameters were sent to the payment app, then the transaction is effectively cancelled. Otherwise, the transaction is effectively processed.

@bzbarsky
Copy link
Author

Does that sound OK?

What does that mean in practice? Do the returned promises get rejected? Remain pending forever? If they get rejected, do they trigger their callbacks?

I expect this to be a significant source of non-interop especially if the payment request is triggered from an iframe that is then navigated or removed from the DOM, because browsers tend to disagree on what exactly gets torn down in those situations. :(

Specs usually don't speak about UI.

Sure. This issue is about the web-observable behavior.

Whether the payment has been processed depends on when navigation or browser context destruction happened

Right, this is all racy.

@domenic
Copy link
Collaborator

domenic commented Jan 21, 2017

OK, most of the easy stuff has been fixed or PRed; now time to tackle this one.

First, what are the concrete, web-observable cases we're concerned with here, where one of the things Boris listed could happen? My enumeration is as follows, but please check it:

  • (A) A payment request is being shown. The user has not accepted the payment request. Suddenly, things happen.
  • (B) A payment request has been shown. The user has accepted the payment request, and the browser is still in the process of signaling back to the main thread, when suddenly, things happen.
  • (C) A payment request is being shown. The user manipulates the UI in a way that is supposed to update the main thread (e.g., changing the shipping option or address), and the browser is still in the process of signaling back to the main thread, when suddenly, things happen.

Does this cover it?

If so, I think what we need then is:

  • We insert a blanket "should" clause that if things happen, all payment request-related UI should be hidden. (case A)
    • This means the promise stays pending forever. This seems most consistent since I don't see anything in the spec saying "if the user rejects the payment request, reject the promise". Maybe that's a separate spec bug?
  • The "user accepts a payment request algorithm" needs to do a check that things have not happened before it proceeds to start doing interesting things (i.e. steps 5 onward, where it creates a PaymentResponse). If things have happened, reject request.[[acceptPromise]] with an "AbortError" DOMException. (case B)
    • We may also want to suggest that user agents tell the user "actually, your payment didn't go through" for case B.
  • The tasks queued (post Queue tasks to change developer-visible properties #405) for the shipping option changes need to do a check to see if things have happened. If things have happened, they should do nothing. (case C)

I've been using "things happen" to mean all the things @bzbarsky lists in his original post. @bzbarsky, do you think this captures that, in precise spec terms? Things have happened if "the PaymentRequest's relevant global object's Document is no longer fully active."

@marcoscaceres
Copy link
Member

@bzbarsky ping...

@bzbarsky
Copy link
Author

I think there is one more case:

  • The page has asked for the payment request to be shown, but it hasn't been yet. Suddenly things happen. For example, the page asks for a payment request, then navigates.

As far as defining "things happen"... the problem is that it's not clear to me that the "fully active" concept captures everything. In particular, it's not clear to me whether removing an iframe from the document means the last document that was in it is no longer "fully active", and similar for the tab closing case...

@zkoch
Copy link
Contributor

zkoch commented Aug 2, 2017

@domenic do you think you have everything that you need here to make a decision on how to move forward?

@domenic
Copy link
Collaborator

domenic commented Aug 4, 2017

Yes, I do. Sorry, I just have been swamped with other things :-/

@domenic
Copy link
Collaborator

domenic commented Aug 9, 2017

It turns out much of this is unnecessary as per the event loop spec tasks on non-fully active documents are not executed. So now that we are properly queuing tasks to do observable things, (B) and (C) from #360 (comment) are not a concern.

(A) is still a potential concern though, so I will fix it. Also, in that comment I said we'd just leave the promise pending, but now I think just rejecting with an "AbortError" DOMException makes more sense.

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

No branches or pull requests

5 participants