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

Make "total" and "details" optional #912

Open
maxlgu opened this issue May 4, 2020 · 11 comments
Open

Make "total" and "details" optional #912

maxlgu opened this issue May 4, 2020 · 11 comments

Comments

@maxlgu
Copy link

maxlgu commented May 4, 2020

TL;DR

Given that when Digital Goods API is used with PaymentRequest API, the total amount is unnecessary, we propose to make the “total” field optional in PaymentRequest API spec, along with a few consequent changes.

Make “total” optional

The current PaymentRequest API requires the “total” field (of PaymentDetailsInit) to be mandatory, because any payment must come with a total amount. The mandatory “total” field was reasonable when we assume that developers could specify the “total” only with this field. But this assumption is invalid when the PaymentRequest API is used in conjunction with the proposed Digital Goods API, which connects with the digital store backend where the goods (with product ID) and their prices are specified. With the DPM API, merchants could simply provide the product ID in the PaymentRequest without knowing the total amount. Therefore, we propose to make the “total” field optional in the PaymentRequest API.

Note that we are not proposing making "total" generally optional (for all payment methods). Rather, it would become at the discretion of the user agent or payment handler when it is required (i.e., it would no longer be "required" in the WebIDL type, but the handler is allowed to throw an exception if it's not given).

For traditional payment methods, this would still be required (otherwise there is no way to communicate to the payment provider how much money is actually being spent). But for some new payment modes such as the proposed Digital Goods API, the server is the source of truth on what the price of the item is, and requiring the payment amount to be given by the client is redundant.

Make “details” optional

After “total” becomes optional, required by the WebIdl spec, the “details” field would have to become optional with a default value:

"If the type of an argument is a dictionary type or a union type that has a dictionary type as one of its flattened member types, and that dictionary type and its ancestors have no required members, and the argument is either the final argument or is followed only by optional arguments, then the argument must be specified as optional and have a default value provided."

So we propose “details” should become optional and default to “{}”.

Improved developer ergonomics

These changes would improve the developer ergonomics, as developers would be able to write:
new PaymentRequest([{supportedMethods: methodName, data: {productId: ‘abc’}}]);
instead of:
new PaymentRequest([{supportedMethods: methodName, data: {productId: ‘abc’}], {total: {...}});

Asynchronously fail a request missing required “total”

In cases where “total” is required but missing, we propose that canMakePayment(), hasEnrolledInstrument() and show() should return a promise rejected with a "NotAllowedError" DOMException. Before the proposal, missing “total” would cause a TypeError “Missing required member(s): total” in PaymentRequest’s constructor synchronously. Note that after the proposal the failure would become asynchronous.

Why not fail a missing “total” in the constructor any more? Although it’s good for backwards compatibility, we move away from constructor because:

  • only WebIDL type errors should be thrown in the constructor
  • canMakePayment, hasEnrolledInstrument, and show are asynchronous methods that may communicate to the payment handler that may require total, so they are more appropriate for the failure.

This change of failure may break the usages of PaymentRequest API that handle TypeError for a missing total. But we deem this breakage as trivial because it’s an error-handling for a programming error - the merchants should have always provided “total” instead of relying on the TypeError.

Summary

In summary, we propose these changes to the PaymentRequest API spec:

  • make “total” an optional field in PaymentDetailsInit (see the table below).
  • make “details” an optional field that is default to {} in PaymentRequest’s constructor (see the table below).
  • when missing “total” where it’s required, canMakePayment(), hasEnrolledInstrument() and show() should return a promise rejected with a "NotAllowedError" DOMException.
ChangeIDL
Remove “required” from the “total” field

dictionary PaymentDetailsInit : PaymentDetailsBase {
  DOMString id;
  required PaymentItem total;
};
Add “optional” to “details”, and give it a default value {}

[SecureContext, Exposed=Window]
interface PaymentRequest : EventTarget {
  constructor(
    sequence methodData,
    optional PaymentDetailsInit details = {},
    optional PaymentOptions options = {}
  );
  ...
}; 
@ianbjacobs
Copy link
Collaborator

cc @aestes, @marcoscaceres, @romandev

@marcoscaceres
Copy link
Member

marcoscaceres commented May 6, 2020

Why not move productId into PaymentDetails instead? Then we can mandate that if total is missing, then productId must be there (as productId serves as substitute for total)?

@marcoscaceres
Copy link
Member

Underlying question here: would any other payment handler apart from Google Pay benefit from productId?

@mgiuca
Copy link

mgiuca commented May 6, 2020

Note: This isn't just for Google Pay, this is to support any provider using our (as yet woefully under-)proposed Digital Product Management API, which could be any digital purchasing backend. I'm planning to post more details of that in the next few days.

Given that, it may be a bit premature to change the Payment Request spec for a use case that doesn't yet have a firm design. @maxlgu would you be happy to sit on this until that solidifies?

@ianbjacobs
Copy link
Collaborator

Noting support from Visa for this proposal via the WG's mailing list:
https://lists.w3.org/Archives/Public/public-payments-wg/2020May/0004.html

@maxlgu
Copy link
Author

maxlgu commented May 7, 2020

@mgiuca , I would sit on this until DPM API solidifies.

@mgiuca
Copy link

mgiuca commented May 11, 2020

There's some confusion on a few threads about this that we would be relaxing the restriction across the board. @maxlgu I think you should update the proposal to be clearer that the individual payment processors can still mandate the total, but that this allows new payment processors to not require a total.

Here's the text of my reply on the above thread:

We are not proposing making "total" generally optional (for all payment methods). Rather, it would become at the discretion of the user agent or payment handler when it is required (i.e., it would no longer be "required" in the WebIDL type, but the handler is allowed to throw an exception if it's not given).

For traditional payment methods, this would still be required (otherwise there is no way to communicate to the payment provider how much money is actually being spent). But for some new payment modes such as the proposed Digital Product Management API, the server is the source of truth on what the price of the item is, and requiring the payment amount to be given by the client is redundant.

@maxlgu
Copy link
Author

maxlgu commented May 12, 2020

@mgiuca , I've incorporated your words into the proposal. Thanks!

blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue May 19, 2020
Context:
Total is a field in the "new PaymentRequest()" API. It specifies the
amount and currency of the payment request. However, when the merchant
requests for the app-store billing (e.g., Google Play Store billing -
by specifying "https://play.google.com/billing" as the method), the
total field becomes unnecessary. This is because app-stores takes
the total from elsewhere.

Before:
The total field is mandatory for PaymentRequest.

After:
The total field is optional if only app-store methods are requested.
When total field is optional and left out, Chrome would add a total of
amount 0, currency "ZZZ" and label "AppStoreBillingTotalPlaceHolder".

Change:
* Added a RuntimeEnabledFeature: PaymentRequestTotalOptional
* Added an about flag: payment-request-optional-total
* change the optionality of the total field and details field of
PaymentRequest API.

Related Doc:
* Chrome Status: https://www.chromestatus.com/feature/5226111782879232
* Intent to Prototype: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/TJVn0Ps9ugA/3unr2Vo8AgAJ
* W3C explainer: w3c/payment-request#912

Bug: 1066531

Change-Id: Id5ad87b9fc452fd41a1ebef066d981737545a235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2150974
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769914}
@adrianhopebailie
Copy link
Collaborator

adrianhopebailie commented May 20, 2020

It seems unnecessary to use the PR API for Digital Goods Services in this way.

This change would mean the Payment Request API is no longer for payments but rather for purchases.

As I suggested in WICG/digital-goods#5, I recommend adding a purchase method to the new API instead.

If it's important to invoke the payment flow of the digital goods service via PR API then why not simply pass in the amount that was returned from itemService.getDetails?

@rsolomakhin
Copy link
Collaborator

Could you please clarify your understanding of differences between payments and purchases? I think I've been working on the wrong API this whole time :-D

@rsolomakhin
Copy link
Collaborator

If it's important to invoke the payment flow of the digital goods service via PR API then why not simply pass in the amount that was returned from itemService.getDetails?

That would give an impression to the web developer that the amount can be modified and is being passed to the payment handler. That is not the case, however, as these types of payment handlers use the product ID to lookup the price in their source of truth database.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Context:
Total is a field in the "new PaymentRequest()" API. It specifies the
amount and currency of the payment request. However, when the merchant
requests for the app-store billing (e.g., Google Play Store billing -
by specifying "https://play.google.com/billing" as the method), the
total field becomes unnecessary. This is because app-stores takes
the total from elsewhere.

Before:
The total field is mandatory for PaymentRequest.

After:
The total field is optional if only app-store methods are requested.
When total field is optional and left out, Chrome would add a total of
amount 0, currency "ZZZ" and label "AppStoreBillingTotalPlaceHolder".

Change:
* Added a RuntimeEnabledFeature: PaymentRequestTotalOptional
* Added an about flag: payment-request-optional-total
* change the optionality of the total field and details field of
PaymentRequest API.

Related Doc:
* Chrome Status: https://www.chromestatus.com/feature/5226111782879232
* Intent to Prototype: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/TJVn0Ps9ugA/3unr2Vo8AgAJ
* W3C explainer: w3c/payment-request#912

Bug: 1066531

Change-Id: Id5ad87b9fc452fd41a1ebef066d981737545a235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2150974
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#769914}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: f8f56a44e023414f59c86350cea68ae4cf8009aa
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

6 participants