-
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
show() must be triggered by user activation #655
Conversation
Note that this has significant impact on the test suite. So, once we have agreement we will need to deal with that before merging. |
143bd40
to
fbdd4ec
Compare
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.
Spec text changes LGTM; unsure of consensus status.
We can implement this only after show promise. (Otherwise some merchants will break.) Landing the spec change is OK. |
@rsolomakhin, ok, good to know... Let's try land them together. |
This is implemented in WebKit already. Added link to Gecko bug. @rsolomakhin, do you have a bug number for Chrome? |
c3c14b8
to
1d93a6e
Compare
Rebased... bit rot. |
1d93a6e
to
207d1ca
Compare
links to payment-request-show-method-manual.https.html
Quick update... I've changed the necessary WPTests to be manual. Just waiting on reviews and we should be good to merge. Marking a blocked on those. |
Ok, tests are updated, so feel ok about merging this. |
@ianbjacobs, could you please send a request to republish the document? |
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.
Looks good!
closes #651
The following tasks have been completed:
Implementation commitment:
Preview | Diff