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

show() must be triggered by user activation #9990

Merged
merged 3 commits into from Mar 14, 2018
Merged

Conversation

marcoscaceres
Copy link
Contributor

@marcoscaceres marcoscaceres commented Mar 13, 2018

}
}, "Must be possible to construct a payment request");

promise_test(async t => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domenic, I'm a bit unsure if using setTimeout to race these two promises is ok? Basically, a conforming implementation would return acceptPromise as rejected with a SecurityError. However, in a non-conforming browser, calling .show() will show the payment sheet, causing the test to get stuck waiting on acceptPromise to resolve.

In order to avoid the test getting stuck, I call .abort() after 100 milliseconds. Is there a better was of doing this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we let tests timeout in non-conforming browsers. That way, their CI infrastructure can control the timeout period, instead of getting it hard-coded to 100 ms as here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I’ll split this into manual and automated test files. Then the automated can time out.

@w3c-bots
Copy link

w3c-bots commented Mar 14, 2018

Build PASSED

Started: 2018-03-14 00:49:56
Finished: 2018-03-14 00:51:27

View more information about this build on:

@marcoscaceres marcoscaceres merged commit 3686c5f into master Mar 14, 2018
@marcoscaceres marcoscaceres deleted the show_manual_3 branch March 21, 2018 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants