Skip to content
This repository has been archived by the owner on Aug 27, 2021. It is now read-only.

Say how we integrate with PaymentRequest #23

Closed
wants to merge 1 commit into from
Closed

Say how we integrate with PaymentRequest #23

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 39 additions & 10 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
links below). All comments are welcome.</p>
</div>
</section>

<section class='informative'>
<h2>Introduction</h2>
<p>
Expand All @@ -118,21 +118,26 @@ <h2>Introduction</h2>
<p class="note">The Web Payments Working Group is also investigating payment methods that offer greater security (e.g., through tokenization).</p>
</section>

<section id="dependencies">
<h2>Dependencies</h2>
<p>
This specification relies on several other underlying specifications.
</p>
<dl>
<section id="dependencies">
<h2>Dependencies</h2>
<p>
This specification relies on several other underlying specifications.
</p>
<dl>
<dt>Payment Request API</dt>
<dd>The terms <dfn>PaymentRequest constructor</dfn>, and <dfn>PaymentAddress</dfn>
<dd>The terms <dfn>PaymentRequest constructor</dfn>, <dfn>PaymentAddress</dfn>, <dfn>user
accepts the payment request algorithm</dfn>, and the <dfn>paymentRequest.show()</dfn> method
are defined by the PaymentRequest API specification [[!PAYMENT-REQUEST-API]].</dd>
<dt>Payment Method Identifiers</dt>
<dd>The term <dfn data-lt="payment method identifier|payment method identifiers">Payment
Method Identifier</dfn> is defined by the Payment Method Identifiers specification
[[!METHOD-IDENTIFIERS]].</dd>
<dt>Web IDL</dt>
<dd>The IDL in this specification is defined by Web IDL [[!WEBIDL]].</dd>
<dt>ECMAScript</dt>
<dd>
The <dfn>JSON.parse</dfn> function is defined by [[!ECMA-262]].
</dd>
</dl>
</section>

Expand Down Expand Up @@ -180,6 +185,22 @@ <h3>BasicCardRequest</h3>
</p>

</section>

<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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of an English problem here it looks like. I think it should be: "When the PaymentRequest API passes data to a payment app..."

of the <a>paymentRequest.show()</a> algorithm, the payment app must process any non-null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I respectfully disagree, @zkoch . I just hit this in Chrome while writing tests. This needs to happen here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it's not just .show(), it's also, "canMakePayment()"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this is the test that Chrome fails with respect to this: https://pastebin.com/PmqNA7JN

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"]')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: <a data-cite="!WEBIDL#es-dictionary">converting</a>

dictionary.</p>

<p>If these steps fail, the payment app must report that it does not support the incoming
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

payment request.</p>

<p>Otherwise, the payment app may use the fields of the resulting dictionary to determine
whether it supports the payment request, and to carry out the payment process.</p>
</section>
</section>

<section id="response">
Expand All @@ -195,8 +216,8 @@ <h3>BasicCardResponse</h3>
required DOMString cardNumber;
DOMString expiryMonth;
DOMString expiryYear;
DOMString cardSecurityCode;
DOMString cardSecurityCode;

PaymentAddress? billingAddress;
};
</pre>
Expand All @@ -222,6 +243,14 @@ <h3>BasicCardResponse</h3>
</dl>

</section>

<section>
<h3>Providing outgoing payment responses</h3>

<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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/<code>/<a>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance that converting from a random app to a BasicCardResponse would fail? If so, should we reject?

</section>
</section>

<section id="security">
Expand Down