-
Notifications
You must be signed in to change notification settings - Fork 135
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
Detecting Payment Method Availability #316
Conversation
merging
Merging from upstream
Following up from our previous discussion regarding "Detecting Payment Method Availability" (https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md) proposal, this is draft of adding a new API canMakeActivePayment() to PaymentRequest API.
I suggest user agents reject the promise only when the query quota has been exceeded. For example, if the merchant website is querying payment methods one-by-one ("visa", "mastercard", "amex"...) for fingerprinting the user. Think about this function if it was synchronous. It would either return true/false in normal operation or throw an exception if something was amiss. It's the same deal here.
By the way, Chromium has an implementation in review. I also have a test website that shows how to use this function: request = new PaymentRequest(supportedInstruments, details);
if (request.canMakeActivePayment) {
request.canMakeActivePayment().then(function(result) {
info(result ? "Can make active payment" : "Cannot make active payment");
}).catch(function(err) {
error(err);
});
} |
<code>canMakeActivePayment()</code> method | ||
</h2> | ||
<p> | ||
The <dfn>canMakeActivePayment</dfn> method is called when the page wants to know if the user has a payment method available to use for payment before calling <a data-lt="PaymentRequest.show">show</a>. The <a>canMakeActivePayment</a> method returns a <a>Promise</a> that will be resolved when the <a>user agent</a> has determined if at least one method is available from <a>supportedMethods</a> data. In order to prevent the page from probing different payment methods supported by user, <a>canMakeActivePayment</a> can only be called once per top-level domain. Multiple calls to <a>canMakeActivePayment</a> will result in cached response from previous call. To reduce privacy risks, user agents MAY limit calls to <a>canMakeActivePayment</a> for a certain time before invalidating the cached response per top-level domain. Developers can call <a>canMakeActivePayment</a> multiple times with same set of <a>supportedMethods</a> per top-level domain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we need to link things properly here, but we need to get the text right first.
</p> | ||
<ol> | ||
<li> | ||
Let <var>request</var> be the <a>PaymentRequest</a> object on which the method is called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I know elsewhere the algos are written to use "request", but being a method, you can just refer to "this" (being the object bound to the method).
Store <var>acceptPromise</var> in <em>request</em>@[[\acceptPromise]]. | ||
</li> | ||
<li> | ||
Return <var>acceptPromise</var> and asynchronously perform the remaining steps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, we should use HTML's "in parallel" definition instead of asynchronously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcoscaceres any pointers to "in parallel" definition Marcos? I could do another PR to add "in parallel" definition through out the spec instead.
Return <var>acceptPromise</var> and asynchronously perform the remaining steps. | ||
</li> | ||
<li> | ||
Let <var>topLevelDomain</var> be the URL of the top level page. Let <var>cachedResponse</var> be cached result of previous call to <a>canMakeActivePayment</a>. If is <var>cachedResponse</var> present, resolve <var>acceptPromise</var> promise with <var>cachedResponse</var>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to follow the web's security model. I expect discussions of "origin", not "domain" here.
Let <var>topLevelDomain</var> be the URL of the top level page. Let <var>cachedResponse</var> be cached result of previous call to <a>canMakeActivePayment</a>. If is <var>cachedResponse</var> present, resolve <var>acceptPromise</var> promise with <var>cachedResponse</var>. | ||
</li> | ||
<li> | ||
Let <var>cacheInvalidateTimer</var> be the time <a>user agent</a> recorded for previous call to <a>canMakeActivePayment</a> for <var>topLevelDomain</var>. Set <a>DateTime</a> to <var>cacheInvalidateTimer</var> if it's not set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cacheInvalidateTimer should have been initialized before being used (at least initially set to null). Also, it's not clear where this variable lives.
<li> | ||
Let <var>request</var> be the <a>PaymentRequest</a> object on which the method is called. | ||
</li> | ||
<li>If the value of <var>request</var>@[[\state]] is not "created", then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject the promise with the InvalidStateError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, you can't have a promise-returning method throw an exception. It must always return a promise. You can return a rejected Promise with an InvalidStateError here and stop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair WebIDL takes care of converting any exceptions that are thrown in the steps describing a method returning a promise into a rejected promise. But of course still clearer to not rely and that part of WebIDL and just explicitly return a rejected promise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, but like you said, it's confusing to anyone not familiar with WebIDL's behavior... we need clean this up also throughout the spec to adhere more closely to: https://www.w3.org/2001/tag/doc/promises-guide.
If <var>cacheInvalidateTimer</var> is set and is still active, resolve <var>acceptPromise</var> promise with <var>cached</var> response. | ||
</li> | ||
<li> | ||
Let <var>supportedMethods</var> be the union of all the <a>supportedMethods</a> sequences from each |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to actually perform the union algorithmically here and derive the value into a <var>
.
</li> | ||
<li> | ||
Let <var>acceptedMethods</var> be <var>supportedMethods</var> with all identifiers removed that the | ||
<a>user agent</a> does not accept and method supports active payment instrument for payment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something not quite right with the second clause of this sentence. Can you please check your grammar here.
<a>user agent</a> does not accept and method supports active payment instrument for payment. | ||
</li> | ||
<li> | ||
If the length of <var>acceptedMethods</var> is zero, then resolve <var>acceptPromise</var> with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to resolve with false. Rejections can only happen for errors, not for expected values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(i.e., only reject with actual exceptions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't resolve this with false
- the promise returns a PaymentResponse
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adrianba, the proposed function signature is Promise<bool> canMakeActivePayment()
. This promise should return a bool
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you shouldn't use acceptPromise, which is used a global name throughout the rest of the spec to indicate the pending return from show().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Let's use canMakeActivePaymentPromise
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm
<code>false</code>, otherwise resolve <var>acceptPromise</var> with <code>true</code>. | ||
</li> | ||
<li> | ||
Cache the response in <var>cachedResponse</var> and set <var>cacheInvalidateTimer</var> to certain <a>DateTime</a> for the <var>topLevelDomain</var>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I don't think DateTime is defined in the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cant recall another spec that has "time" constrain, any pointers to the right class we should use here?
|
The throttling happens per top-level frame, so iframe based libraries should not be affected, unless multiple iframes embedded on the same page query different payment methods at the same time. Let's encourage a single embedded iframe approach to avoid this situation. As for sharing |
Thanks for the response @rsolomakhin! That's great about the top-level frame. In Squarespace's case that solution may be viable, if clunky, but not all payment pages control pre-payment-page code, or they cannot make the decision about which payment methods to query ahead of time. For example, |
@@ -174,6 +174,7 @@ | |||
interface PaymentRequest : EventTarget { | |||
Promise<PaymentResponse> show(); | |||
Promise<void> abort(); | |||
Promise<Boolean> canMakeActivePayment(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct WebIDL type for a boolean is "boolean" with a lowercase b.
My name was brought up in the call today, I'm collating feedback and will hopefully be able to share soon. |
<code>canMakeActivePayment()</code> method | ||
</h2> | ||
<p> | ||
The <dfn>canMakeActivePayment</dfn> method is called when the page wants to know if the user has a payment method available to use for payment before calling <a data-lt="PaymentRequest.show">show</a>. The <a>canMakeActivePayment</a> method returns a <a>Promise</a> that will be resolved when the <a>user agent</a> has determined if at least one method is available from <a>supportedMethods</a> data. In order to prevent the page from probing different payment methods supported by user, <a>canMakeActivePayment</a> can only be called once per top-level domain. Multiple calls to <a>canMakeActivePayment</a> will result in cached response from previous call. To reduce privacy risks, user agents MAY limit calls to <a>canMakeActivePayment</a> for a certain time before invalidating the cached response per top-level domain. Developers can call <a>canMakeActivePayment</a> multiple times with same set of <a>supportedMethods</a> per top-level domain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see that user agent MAY limit calls to canMakeActivePayment()
for a period of time. Let's specify that the limitation is accomplished via rejection of the promise with QuotaExceededError
.
I think my concerns above were sufficiently addressed in the call today. Happy to move forward! |
Would be great to land this in spec to unblock shipping it this in Chromium. |
We need to fix the spec everywhere for "in parallel" - so let's stick with the current text and I'll go through after and fix that after this gets merged
… On 1 Dec. 2016, at 11:42 am, Mahesh Kulkarni ***@***.***> wrote:
@maheshkk commented on this pull request.
In index.html:
> + <li>
+ Let <var>request</var> be the <a>PaymentRequest</a> object on which the method is called.
+ </li>
+ <li>If the value of <var>request</var>@[[\state]] is not "created", then
+ <a>throw</a> an <a>InvalidStateError</a>.</li>
+ <li>
+ Set the value of <em>request</em>@[[\state]] to "interactive".
+ </li>
+ <li>
+ Let <var>acceptPromise</var> be a new <a>Promise</a>.
+ </li>
+ <li>
+ Store <var>acceptPromise</var> in <em>request</em>@[[\acceptPromise]].
+ </li>
+ <li>
+ Return <var>acceptPromise</var> and asynchronously perform the remaining steps.
@marcoscaceres any pointers to "in parallel" definition Marcos? I could do another PR to add "in parallel" definition through out the spec instead.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Given @adrianba's comments in #310, I think we should go ahead and rename the method to @maheshkk think you can make these changes? |
@zkoch OK will rename the method to canMakePayment. Updating the patch with remaining comments from @marcoscaceres @rsolomakhin |
@marcoscaceres @rsolomakhin @zkoch PTAL I have removed the DateTime algorithm and instead left it to implementations to choose to implement a timeout, Happy to change to add the "Time" constraint if it must be present in the spec. I have left "request" as is and will can go through entire spec to change it to "this". Also I'm not sure if promise should be store in request[[canMakePaymentPromise]]. Thanks for your comments! |
</li> | ||
<li> | ||
In addition, implementations may choose to implement a timeout to reset | ||
<var>canMakePaymentQuotaReached</var> for the <var>topLevelOrigin</var>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really work without a quota reset at some point. I think what we need to specify here is that implementations {should or must} reset the quota after a timeout, with the timeout selected by the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two points where quota reserts, .
- Quota resets after the session ends (window closed/navigate away from the page)
- Optional : reset after certain timeout.
I believe your concern is to remove the optional part from second option. Say if some implementors choose to set timeout to zero then it has same effect as keeping the check optional at the moment. Then I wonder why enforce the step with "Must" or "should"?
A possible alternative to this that could avoid the privacy concerns (and a new call) would be to allow the merchant to specify that they have a checkout mechanism on their website when they make a payment request. Then, the browser's payment request UI can present a "checkout with This does introduce another step to the user's flow, but it eliminates all of the user probing (and related quota complexities) and puts the decision in their hands with respect to deciding if they want to go through some registration process or whatever. |
With the suggestion I made above, the user still gets to see a consistent "items and total" display -- if they choose "checkout with website" the only thing the site needs to show is the page to enter/select their payment credentials to complete the payment (or, in some cases, it may just auto-complete the payment once clicked). |
I’m one of the architect’s on Worldpay’s Hosted Payment Page proposition, and am looking at the proposal around canMakeActivePayment() to understand how it might work with our platform. I strongly suspect that other Payment Service Providers are going to have similar concerns / issues to us, and so wanted to contribute to this discussion. Essentially our proposition allows Merchants a very quick time to market by providing an out of the box payment carousel and single integration to many different payment methods and providers, including cards, bank transfers, bank mandates, loyalty cards, etc. The payment journey consists of an initial set-up process where the Merchant configures our system with a list of Payment Methods they wish to accept, and then during each checkout they initialize the payment flow and then redirect the consumer (shopper / end-user) onto our platform; we then take them through the payment process with the list of the available payment methods (subject to configuration, amount, currency, etc.) typically being the first thing the consumer sees. In terms of how we extend our proposition to work with WebPayments, we have a couple of options;
Both of these suggestions would seem to fall foul of the rate-limiting though in the scenario where a Consumer buys something from Merchant A, who is integrated via Worldpay, and then goes to buy something else from Merchant B, also integrated via Worldpay, within the 30 minute period. In both cases the top-level domain would be the same (payments.worldpay.com), so we’d fail that check, and given the Merchant we’re operating on behalf would vary between different invocations of the page the set of payment methods passed into PaymentMethodData.supportedMethods is also likely to vary; meaning we would also fail the second rate-limit check. In some cases it may be possible to initiate the PaymentRequest from the Merchant’s site, for example if they choose to embed the proposition within an iframe or where they do an initial set-up call with us prior to redirecting the consumer; however typically Hosted Payment Pages are used by the less technically advanced customers. In this case we sell a very basic integration proposition that is deliberately kept as simple as possible; they tell our site about their inventory (including stock control if desired), we generate a unique link for each item which they then paste into their page, and the payment is initiated via the consumer performing a put or get onto that URL – hence anything to do with the PaymentRequest would have to happen from within our domain. @maheshkk added a review comment 4 days ago that states that the quota resets ‘after the session ends; window closed / navigate away from page’ or ‘after certain timeout’. Depending on what is meant by the ‘session’ in this scenario, and how that may vary across browsers, this could help with the problem; however it would still seem to have an edge-case where the Consumer is buying things in parallel or doesn’t close the window before starting another purchase on another tab. I should also note that I do see another potential option here; I expect many Merchants will request a more seamless integration where the Web Payments API populates our payment carousel with the individual payment methods supported by the browser, but all of the additional payment methods supported by a PSP appear alongside them. This means that if a Consumer clicks on Visa, for example, they trigger the Web Payments workflow within their browser; however if they click on an unsupported payment method they fall back into existing PSP payment flows. From a merchant point of view this has the benefit of still providing a unified checkout process, and minimizing Consumer interactions – both of which should reduce drop-off. Technically this is largely just another integration scenario that would be subject to the same API constraints however. |
Please let us know what percent of time this happens, so we can evaluate the prevalence of your use case. |
Attempts with multiple methods creates a fingerprinting risk even with the rate limiting sadly. And perhaps a quick series of forwards through TLDs can defeat the rate limiting anyways. Just some alternative approaches to
|
@mattdevaney: After chatting with @ianbjacobs, we've come up with a solution that should resolve your issue: pre-query <a class="worldpay-button"
href="https://worldpay.com/checkout?cart=item1,1,USD,5.99">Buy</a> In addition to this link, you should provide a javascript snippet that looks like this: <script src="https://worldpay.com/check-payment-request.js"></script> The contents of the javascript file should check whether PaymentRequest is available and update the if (PaymentRequest in window) {
try {
var pr = new PaymentRequest(supportedMethods, shoppingCartContents);
if (pr.canMakePayment) {
pr.canMakePayment()
.then(function(result) {
if (result) {
var buttons = document.getElementsByClassName("worldpay-button");
var i;
for (i = 0; i < buttons.length; i++) {
buttons[i].href += "&use-payment-request=true";
}
}
})
.catch(function(error) {
logError(error);
});
} catch(error) {
logError(error);
}
}
} There're several ways that you can customize the
It'd be great to hear what you and other hosted solution organizations (Shopify?) think about this solution. |
Just to clarify that @rsolomakhin had this good idea and wrote it all up. Thanks Rouslan! Ian |
It was decided to rename canMakeActivePayment to canMakePayment: w3c/payment-request#316 Note that this feature is still behind a runtime flag, now named CanMakePayment. BUG=670967 Review-Url: https://codereview.chromium.org/2545523004 Cr-Commit-Position: refs/heads/master@{#436078} (cherry picked from commit e3da0af) Review URL: https://codereview.chromium.org/2552693002 . Cr-Commit-Position: refs/branch-heads/2924@{#327} Cr-Branched-From: 3a87aec-refs/heads/master@{#433059}
@rsolomakhin : Thanks for the feedback and suggestion. There are two main problems I see with the approach; one of which is almost certainly bigger than the other.
To answer the earlier comment from @rsolomakhin : I don't have any exact stats on how likely it is to happen, but if you consider how many customers (Merchants) a major PSP has today and combine that with future growth then it's going to happen; even if only as an edge-case. If the goal is to build something that Merchant's want to use and adopt, then I'd suggest that designing in limitations such as we're discussing here can only be a detriment to that. Especially since, as @burdges pointed out, finger printing may still be relatively trivially possible even with rate limiting imposed. |
Do you even need to call the |
The feature that @dlongley has proposed is certainly a valid feature on its own, since it would still be nice for a user to “exit gracefully” from the Payment Request sheet in order to continue through a legacy checkout flow (rather than the current implementation which requires the user to “cancel” out of Payment Request). However, there is still a need for the |
I'd think a "Buy Now" can always appear even without If the merchant does not yet know the information they need, like shipping address, then "Buy Now" should send the customer to a page that asks for it, as the payment API should not return any private information. At that point, the merchant can call As an aside, we should expect privacy-oriented browser venders, like say Brave, Tor, etc., to hard wire |
Just a heads-up that there was some support for your idea on our call today: We decided to decouple your proposal from canMakePayment and pursue your proposal independently. @msporny will be contacting you. Ian |
Apologies for the silence over the last couple of days @dlongley - That might work, at least as an initial implementation, as it provides a way to drop through to a wider payment ecosystem (e.g. psp) if one exists. However, as @mattsaxon mentioned on the call yesterday; it doesn't solve the re-integration problem. Anything that requires Merchants to update their webpages to make use of WebPayments is going to reduce the uptake of the functionality (.. based on my experience in getting them to make relatively minor changes to enhance their existing payment integrations!). Having a mechanism that allows the PSP to act as the integration point offers a frictionless way to bring this to a large audience in a (hopefully) short timeframe. Maybe trying to use canMakePayment() for that is the wrong approach though |
Thanks for the heads up. Manu gave me an update -- I'll be putting together a simple PR for the proposal when I get a chance. |
Just an update for everyone: the WG voted to adopt this PR on the call this week. We still have some editorial changes to make, so I'll work through those with the other editors. The easiest thing might be to merge as-is and then submit another PR fixing. |
SGTM. |
Update: After talking with Mahesh, going to take a stab at a new PR. So let's keep this one alive, but don't merge, and we can close it out when the new PR lands. |
Draft of canMakeActivePayment() API from the proposal here https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md