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

Update promise rejection #187

Closed
adamroach opened this issue May 10, 2016 · 10 comments
Closed

Update promise rejection #187

adamroach opened this issue May 10, 2016 · 10 comments

Comments

@adamroach
Copy link
Contributor

The procedure in section 13.2 does not mention any processing steps in the case the update promise d is rejected. In this case, I think that the only sensible thing to do is to revert the change that the user made which triggered the event. This implies that payment apps must retain the state prior to the change that triggered the event so that it can be rolled back to.

@adrianhopebailie
Copy link
Collaborator

@adamroach this processing is intended to happen before the user agent has passed the payment request to the payment app so payment apps are not involved in this processing at all.

However, I think this is still a valid issue for the editors to address:

The procedure in section 13.2 does not mention any processing steps in the case the update promise d is rejected.

Also, the case where d is resolved but is not a valid PaymentDetails dictionary

@adrianba
Copy link
Contributor

@adrianhopebailie, I believe this is all handled in the algorithm. It makes a rejected promise a no-op. It also extracts appropriate data from PaymentDetails and ignores other details.

As I said in mail, I disagree that the only sensible thing to do is to revert the change that the user made which triggered the event.

@adrianhopebailie
Copy link
Collaborator

@adrianba are you asserting that there is no need to describe how the UA should behave in the case of a no-op? i.e. The UI stays as is (with current user selections) and the website is unaware that the browser didn't like the new payment details?

@adrianba
Copy link
Contributor

adrianba commented May 12, 2016

The web site is in control of what happens here. The code running is the web site's code. It is the web site that chooses to reject a promise and deal with the consequences. It is the web site that chooses what data to pass. It is the web site's responsibility to pass in data that satisfies the algorithm's checks. The web site has all the opportunity it needs to be aware of what it is doing.

What I said in mail was, rather than adding some complex recovery logic, we should let the web site be in control and get some implementation experience.

@adamroach
Copy link
Contributor Author

adamroach commented May 12, 2016

Regardless of how we expect this to behave, it needs clear documentation.

I'm a little perplexed by what @adrianba is proposing here, though. Let's imagine that I'm running a website that can only ship to the US and Canada. I get a PaymentRequestUpdateEvent with a US address, and update my shipping methods and total appropriately. Then I get a PaymentRequestUpdateEvent with an address in Finland. In @adrianba's model, my options appear to be:

  1. Ignore the event, and leave the US pricing and shipping methods in front of the user
  2. Call updateWith() and do something odd like removing all shipping options and zeroing out the cost of the shipping line
  3. Call updateWith(), and reject the promise, which has exactly the same effect as (1).
  4. Abort the payment request, which will close the interface the user is interacting with, leaving them befuddled and a little angry.

What I'm proposing is that the optimal user experience here would involve giving the website a means to say "Sorry, but that change doesn't work for me. Go back to how it was, because that was fine." In the model @adrianba is proposing, this simply isn't possible. I'm proposing that (3), above, could easily serve this purpose. It's not clear why having the browser keep track of the previous shipping address or previous shipping method for the duration of event processing would be in any way complicated.

@adrianhopebailie adrianhopebailie added this to the 19 May milestone May 18, 2016
@adrianhopebailie
Copy link
Collaborator

@adrianba and @adamroach - quick reminder that you intended to discuss this offline of the call and see if you could come up with a solution that you both agreed on.

I am dropping from discussion on the calls for now

@adrianhopebailie adrianhopebailie removed this from the 19 May milestone May 24, 2016
@adrianhopebailie
Copy link
Collaborator

@adrianba and @adamroach - PING (can I close)

@adamroach
Copy link
Contributor Author

@adrianhopebailie -- I still hope to engage @adrianba on a conversation around this, but it's been very busy for me recently. If you're okay leaving this on the back burner, I don't think it's blocking anything else at the moment.

@adrianhopebailie
Copy link
Collaborator

@adamroach - np

adrianba added a commit to adrianba/browser-payment-api that referenced this issue Jun 8, 2016
This is a proposal for how to handle w3c#187. It causes the payment request
to be aborted if the promise passed to updateWith() is then rejected.
adrianba added a commit that referenced this issue Jun 9, 2016
This is a proposal for how to handle #187. It causes the payment request
to be aborted if the promise passed to updateWith() is then rejected.
@adrianba
Copy link
Contributor

Closed by #209.

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

3 participants