-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Test snapshotting of allowfullscreen attribute #4354
Conversation
Notifying @aliams, @foolip, @jernoble, and @upsuper. (Learn how reviewing works.) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Seems to be testing what it intends to. Can you see who passes and fails?
|
||
let i = 0; | ||
|
||
iframe.src = "http://{{domains[www1]}}:{{ports[http][0]}}/fullscreen/api/echo-fullscreenEnabled.html"; |
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.
I don't recognize this syntax, is this using a wptserve feature, to make it a cross-origin iframe?
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.
Yes.
Gecko and Edge 13 fail because of live behavior. Chromium and WebKit have prefixed names. Testing with |
Should also test the timing of when it's snapshotted; at document creation time vs completed response headers. |
@zcorpan can you rebase to trigger new CI runs? |
c050814
to
55e4b85
Compare
Rebased. |
55e4b85
to
fc6c02d
Compare
I rebased again to rerun CI checks. |
|
02772dc
to
14a654d
Compare
Lint error was fixed, but CI still failed. The logs were no longer available. Rebasing again. whatwg/html#3287 replaced whatwg/html#2187 . @clelland , do these tests match what the spec now requires? |
14a654d
to
347c7a9
Compare
@@ -0,0 +1,30 @@ | |||
<!DOCTYPE html> | |||
<title>Document#fullscreenEnabled setting allowfullscreen after document creation, before response</title> |
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.
Nice and evil test, I like it!
The spec for this ended up as https://html.spec.whatwg.org/multipage/iframe-embed-object.html#allowed-to-use. The warning there summarizes it well:
|
Thanks @foolip! |
Test for whatwg/html#2187
Edit: this ought to follow whatwg/html#3287 but I don't know right now (2020-01-27) whether it does.