Re: [w3c/payment-request] Add hasEnrolledInstrument() (#833)

Hi @marcoscaceres, thanks for the feedback!

> However, I'd like to do this in two parts because there is some duplication in the two algorithms that I'd like to avoid (mostly an editorial/maintenance issue).

Consolidating redundant algorithm makes sense. I went the other way initially because I felt it may be confusing to embed an "if" and the rate-limit note inside the loop. I gave it another try. PTAL.

> 
 Also note that the `hasEnrolledInstrument()` doesn't seem to integrate with the state machine like the other methods.
> 
> For example, right now:
> 
> ```
> pr = new PaymentRequest("")
> await pr.abort();
> await pr.canMakePayment(); //throws
> await pr.hasEnrolledInstrument(); // doesn't throw.
> ```

This would be a bug. How did you test it? I added a manual WPT test case for this scenario and it was passing on my debug build, but I could have overlooked something: https://github.com/web-platform-tests/wpt/blob/master/payment-request/payment-request-hasenrolledinstrument-method-manual.https.html#L81

> (Note the hasEnrolledInstruments() would introduce another fingerprinting vector - so we might want to document that in the Privacy and Security section, and we should make it ok for UAs to lie for privacy reasons).

Updated the Privacy Considerations section. PTAL.

Do you think it's important to keep the abuse protection for `canMakePayment()`? I feel it's no longer necessary because with the introduction of JIT and removal of handler-specific can make payment check, the new `canMakePayment()` is mostly an UA version detector. `hasEnrolledInstrument()` on the other hand still exposes user information, so should be kept under abuse protection.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/payment-request/pull/833#issuecomment-462407001