#146: PaymentRequest.canMakeActivePayment()
Discussions
Comment by @sideshowbarker Nov 12, 2016 (See Github)
Relevant time constraints or deadlines: Would like to ship patch in Chrome M56.
Chrome 56 is targeted for stable non‐beta release around mid-January, right?
If so, by what date at the latest would you expect to have received review by the TAG and resolved any issues with the API that TAG might bring up during the review?
Comment by @rsolomakhin Nov 12, 2016 (See Github)
Before December would be ideal.
Comment by @dbaron Nov 18, 2016 (See Github)
@rsolomakhin Do you have a link to the specification as well?
Also, I'm a little curious about the naming of <code>canMakeActivePayment</code>. What does the word “active” mean in this context? (It makes more sense to me in the context of the <code>canMakePaymentsWithActiveCard</code> method that you refer to in the explainer.)
Comment by @dbaron Nov 18, 2016 (See Github)
Ah, following the link through the blink-dev thread, it looks like the spec text is under development at https://github.com/w3c/browser-payment-api/pull/316 and may land in the Payment Request API specification soon.
Comment by @rsolomakhin Nov 18, 2016 (See Github)
The word "active" has also been discussed in web payments WG github. Were you able to find it?
Comment by @dbaron Nov 23, 2016 (See Github)
Ah, I found the discussion of "active" in https://github.com/w3c/browser-payment-api/issues/310 . (Though I see substantial concern about the name there as well.)
I'm also curious how well this aligns with naming of other examples in the Web platform of functions returning promises that resolve to a boolean result. The ones I found (from searching Gecko's WebIDL) are <code>has</code> and <code>delete</code> on <code>CacheStorage</code>, <code>remove</code> and <code>removeDeep</code> on <code>Directory</code>, <code>load</code> on <code>MediaKeySession</code>, <code>unsubscribe</code> on <code>PushSubscription</code>, <code>unregister</code> on <code>ServiceWorkerRegistration</code>, and <code>persisted</code> and <code>persist</code> on <code>StorageManager</code>. Of those, I think <code>has</code> is the only one that sounds like it returns a boolean. (I guess the naming convention for functions returning promises should match the naming convention of synchronous functions, but it just feels weird in this case. Maybe I'm just not used to it yet.)
Comment by @dbaron Dec 1, 2016 (See Github)
OK, so a few somewhat blunt comments here:
First, it's a bit unusual to ask for TAG review of something that doesn't have a specification. That said, it's not something I would want to discourage, since a group might have a particular architectural issue they want feedback on that seems likely to be important or controversial. However, although I'm an outsider to the Blink project, it does seem (to my understanding of it) somewhat contrary to the intent of Blink's launch process's rationale for requiring asking for TAG review; it says, for example:
In practice, we strive to ensure that the features we ship by default have open standards.
and it feels to me that part of the purpose of asking for TAG review is not only to request review that the API fits with the way Web APIs are designed, but also that it fits with the way that they're specified, so that they have a good path towards being implemented interoperably by multiple implementations.
Second, it feels in other ways that you're trying to cram this through without listening to feedback. The first and second comments by others in https://github.com/w3c/browser-payment-api/issues/310 and my initial reaction here were all saying that the name doesn't make sense, and you don't seem to have responded to that concern other than to explain why other proposals are also misleading. However, it does look like the method has been renamed in response to a different concern, although the explainer hasn't been updated, and, to be honest, it's not clear how much of the explainer even still applies following that change. (That is, does the API still meet the rationale for adding it, after that change?)
Further comments, looking at the still unmerged pull request (although I don't know if it's possible to link to the current state of that pull request, or if the current state will even be preserved in the future to make these comments make sense):
In order to prevent the page from probing different payment methods supported by user, <a>canMakePayment</a> can only be called once per top-level domain. Multiple calls to <a>canMakePayment</a> will result in rejection of the promise with QuotaExceededError. To reduce privacy risks, implementations MAY limit calls to <a>canMakePayment</a> for a certain period of time before allowing top-level origin to call <a>canMakePayment</a> again. However <a>canMakePayment</a> can be called multiple times with same set of <a data-lt="PaymentMethodData.supportedMethods">supportedMethods</a> per top-level origin.
This text is quite unclear; it doesn't clearly explain how the passing of time and calls from different TLDs or origins interact to lead to QuotaExceededError. It seems like it should be a clearer English summary of what the algorithm below it leads to.
- Store <var>canMakePaymentPromise</var> in <em>request</em>@[[\canMakePaymentPromise]].
It's not clear what purpose this serves, since this property doesn't appear to be read from, and since multiple calls to canMakePayment() will overwrite this value.
(within point 7) Let <var>cachedSupportedMethods</var> be <a>supportedMethods</a> sequences from each <a>PaymentMethodData</a> from previous call to <a>PaymentRequest</a>. Let <var>cachedResponse</var> be response from previous call to <a>canMakePayment</a>. Let <var>canMakePaymentQuotaReached</var> be boolean value of previous call to <a>canMakePayment</a> for the <a>topLevelOrigin</a>.
This seems oddly less precise than the rest of the algorithm, which does things by storing variables in places. Is it the previous call made by the same origin? Which step in this algorithm does that call need to have reached for it to count? Is previous defined by the order of call or the order of promise resolution?
- If cachedSupportedMethods is preset and matches supportMethods, then resolve canMakePaymentPromise with cachedResponse. Else, set cachedSupportedMethods to supportedMethods.
It seems like the "Else" part of this should come after step 10. Otherwise if you have three calls in succession, with the first having one set of methods, and the second and third having another, then the first call will return a real result, the second call will be rejected with QuotaExceededError, and the third call will return the cached response from the first call (which has different methods, but which were incorrectly stored in the second call).
(This is one of the disadvantages of writing specs so algorithmically: it's harder to detect bugs in the specifications.)
Comment by @triblondon Feb 9, 2017 (See Github)
Picked up at Boston F2F.
Seems that the current thinking on this is that the method should compute the intersection of the payment methods accepted by the website and the methods offered by the user, and return true if the result is non-empty set. However, to defend against fishing expeditions, there needs to be some constraint on how many times this method can be called with different params. @slightlyoff determined that in Chrome, the result is cached for 30 mins for an origin, so if you make a second invocation with different params you will get the previously cached result (which may be - somewhat intentionally - wrong for the new invocation)
Resolved that @dbaron would review the spec and we'll follow up on next week's call.
Comment by @dbaron Apr 27, 2017 (See Github)
Current spec is https://w3c.github.io/browser-payment-api/#canmakepayment-method
Comment by @triblondon Apr 27, 2017 (See Github)
At Tokyo F2F we agreed that we're happy with this as it stands.
OpenedNov 11, 2016
Hello TAG!
I'm requesting a TAG review of:
Further details:
You should also know that...
[please tell us anything you think is relevant to this review]
We'd prefer the TAG provide feedback as (please select one):