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

Add way to update total and displayItems before .show() #639

Closed
jenan-stripe opened this issue Oct 6, 2017 · 6 comments
Closed

Add way to update total and displayItems before .show() #639

jenan-stripe opened this issue Oct 6, 2017 · 6 comments

Comments

@jenan-stripe
Copy link

We’ve heard from merchants that they’d like a way to update the total of a PaymentRequest before .show() is called. Concrete use case: render a button on page load after being sure that canMakePayment() resolves to true, and then .update() as a user selects between SKUs. Another: a donation site where the user can select how much to donate.

One solution may be to just create new PaymentRequest instances when the user interacts with the page, but canMakePayment() makes network requests and can be slow, so you either end up with a race condition between the user changing the form and clicking the button or uncomfortable disabled+re-enabled UI.

Payment Handlers get passed total when canMakePayment is called, so I could see an argument that canMakePayment() needs to be re-run as totals change, but obviously total can also be updated after .show() via the shipping callbacks.

@rsolomakhin
Copy link
Collaborator

Hi @jenan-stripe,

Thank you for bringing this to our attention. We've been recommending merchants with this requirement to call canMakePayment() once and reuse its result as the shopping cart updates:

https://github.com/w3c/payment-request-info/wiki/CodeExamples#check-whether-user-can-make-payment-before-all-prices-are-known

We don't recommend calling canMakePayment() for every instance of PaymentRequest, if the only differences among them are the total and line items. This is because the browser should not pass the total and the line items into the payment apps during the canMakePayment() call. Chrome used to do this a while ago for Android payment apps, but we've since removed this functionality to improve user privacy.

Is this helpful or do you still see room for improvement in the API?

Cheers,
Rouslan

P.S.: One neat trick in Chrome that not many people know is to "warm up" PaymentRequest by creating an instance as soon as the page loads or shopping cart contents change. Constructing an instance of PaymentRequest will scan the device for installed payment apps that the merchant requested. This scan can take up to 2 seconds on older mobile devices with cold start or as few as 50 milliseconds on newer devices. This period of time is the "warm up" period. You may have noticed that canMakePayment() takes a while to return. This is because canMakePayment() returns after the payment app scan is complete. It is important to note, however, that canMakePayment() does not start the scan, but only waits for the end of the scan, which was started by constructing a PaymentRequest instance. If you call show() during the "warm up" period, Chrome will show a "Loading..." spinner as it completes the scan for apps. If you call show() after the "warm up" period, Chrome will show the payment sheet immediately. There's a lot of nuance involved here, so please don't hesitate to reach out with more questions.

@jenan-stripe
Copy link
Author

@rsolomakhin Ah, fascinating! Thanks for detailed response and Chrome details!

That sounds like a great workaround, but it sort of falls out of some assumptions about the API rather than specific contracts, right?

  1. Right now, it looks like the Payment Handler spec says that handlers receive total (although it's not clear to me how canMakePayment events differ from these -- am I looking in the wrong place?)
  2. The spec is rather light on canMakePayment() implementation details. There doesn't seem to be anything stopping a browser from forcing canMakePayment() being called before show(), right?

@rsolomakhin
Copy link
Collaborator

rsolomakhin commented Oct 6, 2017

It falls out of some assumptions about the API rather than specific contracts.

It's true that the spec does not specify when the scan for the payment apps should happen. There's a fine line between predictability of API behavior and tying the hands of browser manufacturers. I see your point here, though. Perhaps this is good behavior to standardize across browsers, so that web developers can rely on it. @ianbjacobs Can we put this on agenda for the next call or the face-to-face in November?

Right now, it looks like the Payment Handler spec says that handlers receive total.

The PaymentRequestEvent is fired in the selected payment handler when the user clicks the [Pay] button in the browser payment sheet UI. Constructing PaymentRequest, calling canMakePayment(), or calling show() will not trigger the PaymentRequestEvent. So the payment handler does not get the total until after user consent.

I have a proposal in the works to optionally fire CanMakePaymentEvent in the payment handler. This would be fired during PaymentRequest construction while scanning the locally installed payment apps. This event will not contain the total.

In summary, the corresponding events between the merchant and the payment app are:

User Merchant Browser
Visit page. pr = new PaymentRequest(); App scan: If the payment handler supports standardized payment method identifier (e.g., "basic-card"), then the browser compares merchant's request to capabilities of the payment handler and stores matching payment handlers in apps list in memory. If the payment handler supports only URL-based payment method identifiers (e.g., https://android.com/pay), then the browser fires CanMakePaymentEvent event in the payment handler and stores the ones that returned true in the apps list in memory.
No action required. pr.canMakePayment(); Wait until app scan completes, then return !apps.empty().
Click [Buy_Now] button on the page. pr.show(); Show the payment sheet UI with a "Loading..." spinner, wait until the app scan completes, then show the apps in the sheet.
Click [Pay] in the browser payment sheet UI. Fire PaymentRequestEvent in the selected payment handler.

There doesn't seem to be anything stopping a browser from forcing canMakePayment() being called before show().

This can get a little confusing, so let's clarify a bit. According to the spec, calling show() without calling canMakePayment() is OK, but calling canMakePayment() after calling show() on the same instance of PaymentRequest will throw InvalidStateError.

// CASE 1: This is fine:
pr = new PaymentRequest();
pr.canMakePayment();
pr.show();

// CASE 2: This is also fine:
pr = new PaymentRequest();
pr.show();

// CASE 3: This will throw in canMakePayment():
pr = new PaymentRequest();
pr.show();
pr.canMakePayment();  // InvalidStateError

Is it possible, however, that a browser implements the PaymentRequest spec correctly, but does not work in CASE 2 above? The answer is "No". Although this is not explicitly stated in the spec, this is prevented by the definitions of the algorithms for canMakePayment() and show(). The canMakePayment() algorithm does not change any internal state, so there's nothing for the show() algorithm to check. We can enforce this via an explicit web platform test, as well.

Note that the opposite is not true. The show() algorithm sets the internal field request.[[state]] to "interactive" and canMakePayment() throws InvalidStateError with this state. In practice this means that you can't run canMakePayment() after show() was called on the same instance of PaymentRequest, as shown in CASE 3 above.

I hope this information is useful. Please let me know if I can help with any other concerns you might have.

@ianbjacobs
Copy link
Collaborator

@rsolomakhin, I have added the topic to the agenda.

@jenan-stripe
Copy link
Author

@rsolomakhin Ahhh excellent -- so canMakePayment + payment handler interaction isn't actually specced yet. That clears things up. 👍

@jenan-stripe
Copy link
Author

Closing in favor of #645.

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

4 participants