Skip to content
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

"Exposing available payment methods" is confusing given the existence of canMakePayment() #629

Closed
domenic opened this issue Sep 21, 2017 · 7 comments

Comments

@domenic
Copy link
Collaborator

domenic commented Sep 21, 2017

A Chrome developer ended up confused by this section because it says

The fact that a successful match to a payment method causes a user interface to be displayed mitigates the disclosure risk.

However this is not true for "the payment request API" in general, only for paymentRequest.show(). In particular canMakePayment() can be called without UI.

This section should be rewritten to be specific what methods it's talking about, and talk about canMakePayment()'s step 3 mitigations additionally.


As a separate problem, the "may" requirements in this section are very bad, and should be moved to the show() method.


I can try to work on this "soon", but it's hard to guarantee availability for this week or next, so since it seems things are heading toward some sort of spec freeze, maybe someone else can help out here.

@marcoscaceres
Copy link
Member

I can have a go at fixing it.

@lknik
Copy link

lknik commented Sep 21, 2017

I support this request :)

@marcoscaceres
Copy link
Member

@lknik, be careful, or we will rope you in to help and review stuff.

@ianbjacobs
Copy link
Collaborator

From a republication of CR perspective,this sounds editorial to me.

@marcoscaceres
Copy link
Member

As a separate problem, the "may" requirements in this section are very bad, and should be moved to the show() method.

This will be a MUST #655

Sending a PR that removes this section entirely, because it's bad (also as per #475).

@marcoscaceres
Copy link
Member

Sent PR for this #673

marcoscaceres added a commit that referenced this issue Jan 24, 2018
marcoscaceres added a commit that referenced this issue Jan 24, 2018
@domenic
Copy link
Collaborator Author

domenic commented Jan 24, 2018

Fixed by #673

@domenic domenic closed this as completed Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants