Say how we integrate with PaymentRequest #23
Conversation
This follows w3c/payment-request#382 so that instead of simply defining the dictionaries in this spec, we say how they are used, and how invalid incoming data is processed. (See w3c#20.)
|
I've requested @ianbjacobs grant me access, so I can review and make some minor fixes to this PR. |
|
@domenic, sorry, this fell through the cracks. Will get to it soon. |
| <h3>Processing incoming payment method data</h3> | ||
|
|
||
| <p>When the PaymentRequest API is to pass data to a payment app supporting basic-card as part | ||
| of the <a>paymentRequest.show()</a> algorithm, the payment app must process any non-null |
There was a problem hiding this comment.
Looking at this a bit closer, I don't think .show() is the right place in the algorithm where this matters. This actually matters after show has been called and the user has selected the appropriate payment app. Then the data is transferred. No data is transferred at .show() by default.
There was a problem hiding this comment.
I respectfully disagree, @zkoch . I just hit this in Chrome while writing tests. This needs to happen here.
There was a problem hiding this comment.
Also, it's not just .show(), it's also, "canMakePayment()"
There was a problem hiding this comment.
FWIW, this is the test that Chrome fails with respect to this: https://pastebin.com/PmqNA7JN
There was a problem hiding this comment.
Chatting with @marcoscaceres on the editor call, I think there are two issues here:
-
We should remove any language around payment apps from this section, as payment apps aren't part of the .show algorithm
-
We should throw an exception if we see basic-card and things that aren't supported as a part of that (e.g. 'supportedTypes: ["banana"]')
There was a problem hiding this comment.
We should remove any language around payment apps from this section, as payment apps aren't part of the .show algorithm
+1
We should throw an exception if we see basic-card and things that aren't supported as a part of that (e.g. 'supportedTypes: ["banana"]')
-1 : This implies that the browser will do validation of the data before passing to payment handlers. If this is what we want then we should get consensus from all the browser vendors that this is a good idea AND that they will support new payment methods that come along and define new payment method specific data formats.
There was a problem hiding this comment.
This implies that the browser will do validation of the data before passing to payment handlers.
Correct.
But(!), for non-short strings (i.e., URLs), the .data is just "object", so it gets passed along to the appropriate payment handler after 1) undergoing JSON serialization (to expunge function and other unsafe things), and 2) undergoing conversion to "object" as per WebIDL.
You cannot escape WebIDL conversion, because .data MUST be converted into something that can survive being transferred from JS to C++ and back to JS, possibly crossing multiple thread boundaries (service worker acting as a payment handler). Or, in the case where the payment handler is a Android native app, that might be JS to Java, and then back to JS.
If this is what we want then we should get consensus from all the browser vendors that this is a good idea
Microsoft, Google, Mozilla are having this discussion right here - and I'm sure our friends at Opera, Samsung, and Facebook are watching (Hai friends!). It doesn't get any more "browser vendor consensus" than that... well, unless we somehow drag Apple in here 😛
AND that they will support new payment methods that come along and define new payment method specific data formats.
Sure. Why would anyone not support and define new standardized PMIs where they would benefit users? That's the whole point of standardizing the short strings. Like any other part of the web platform, they require backing from browser vendors before they can actually be standardized.
Let me be absolutely clear: If a short string doesn't have support from implementers (i.e., doesn't ship in 2+ browsers), then it can't ever become a web standard. Same as any other thing in the platform. If you are suggesting minting arbitrary short strings, and then complaining that no one supports them, then you need to check your assumptions. W3C standards are "Recommendations" - no one is obliged to support any of them. However, it's still possible for a browser to support non-standardized PMIs via URLs and the Payment Handler spec.
| <section> | ||
| <h3>Processing incoming payment method data</h3> | ||
|
|
||
| <p>When the PaymentRequest API is to pass data to a payment app supporting basic-card as part |
There was a problem hiding this comment.
Bit of an English problem here it looks like. I think it should be: "When the PaymentRequest API passes data to a payment app..."
| <h3>Processing incoming payment method data</h3> | ||
|
|
||
| <p>When the PaymentRequest API is to pass data to a payment app supporting basic-card as part | ||
| of the <a>paymentRequest.show()</a> algorithm, the payment app must process any non-null |
There was a problem hiding this comment.
Also, it's not just .show(), it's also, "canMakePayment()"
|
|
||
| <p>When the <a>user accepts the payment request algorithm</a> runs in the context of a | ||
| payment app supporting basic-card, the payment app must provide back the relevant data in a | ||
| <code>BasicCardResponse</code> dictionary.</p> |
There was a problem hiding this comment.
Is there a chance that converting from a random app to a BasicCardResponse would fail? If so, should we reject?
| <p>When the PaymentRequest API is to pass data to a payment app supporting basic-card as part | ||
| of the <a>paymentRequest.show()</a> algorithm, the payment app must process any non-null | ||
| incoming data by deserializing it using an algorithm equivalent to that performed by | ||
| <a>JSON.parse</a>, and then converting the resulting object to a <code>BasicCardRequest</code> |
There was a problem hiding this comment.
nit: <a data-cite="!WEBIDL#es-dictionary">converting</a>
| <a>JSON.parse</a>, and then converting the resulting object to a <code>BasicCardRequest</code> | ||
| dictionary.</p> | ||
|
|
||
| <p>If these steps fail, the payment app must report that it does not support the incoming |
There was a problem hiding this comment.
Also, I think we should throw the WebIDL exception here that resulted from the conversion. Then, in .show() rethrow it.
In .canMakePayment(), we can catch it, and report it as a "false" - optionally, informing the developer that they data is invalid.
Note: another mention of payment app here.
|
Superseded by #39 |
This follows w3c/payment-request#382 so that instead of simply defining the dictionaries in this spec, we say how they are used, and how invalid incoming data is processed. (See #20.)