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

cross-origin-opener-policy/resource-popup.https.html makes incorrect … #29769

Merged
merged 1 commit into from
Jul 26, 2021

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Jul 23, 2021

…assumption about win.location.href

The test expects that win.location.href starts returning something else than "about:blank" after closing.
While this is true in Firefox and Blink, none of the browser engines actually agree on this (Firefox
returns the empty string, Chrome returns undefined and Safari returns "about:blank").

Per the specification, it seems returning "about:blank" after a window is closed is the expected behavior,
see whatwg/html#6899. I am fixing thus fixing the test so that it can run in
all browsers by checking if "the window is closed" OR "href is no longer about:blank because it was
navigated".

…assumption about win.location.href

The test expects that win.location.href starts returning something else than "about:blank" after closing.
While this is true in Firefox and Blink, none of the browser engines actually agree on this (Firefox
returns the empty string, Chrome returns undefined and Safari returns "about:blank").

Per the specification, it seems returning "about:blank" after a window is closed is the expected behavior,
see whatwg/html#6899. I am fixing thus fixing the test so that it can run in
all browsers by checking if "the window is closed" OR "href is no longer about:blank because it was
navigated".
@foolip
Copy link
Member

foolip commented Jul 25, 2021

@cdumez with this change, will there be any tests failing in Chrome and Firefox due to their bug(s)? If not, could the test be copied and minified to just test that, which would be a testing passing only in Safari?

@mikewest
Copy link
Member

@ParisMeuleman is OOO for a while, perhaps @ArthurSonzogni or @antosart can take a look at this in the meantime?

Copy link
Member

@mikewest mikewest left a comment

Choose a reason for hiding this comment

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

Actually, the test change seems clearly reasonable. The underlying behavior change is what folks ought to take a look at. I'll approve this, and CC folks on the HTML issue.

@annevk
Copy link
Member

annevk commented Jul 26, 2021

Interesting, if true I guess we should wait for .closed and then assert the about:blankness.

@cdumez
Copy link
Contributor Author

cdumez commented Jul 26, 2021

@cdumez with this change, will there be any tests failing in Chrome and Firefox due to their bug(s)? If not, could the test be copied and minified to just test that, which would be a testing passing only in Safari?

My changes to this test should not make the test fail in Firefox on Chrome. It merely makes it runnable in Safari.

However, the fact that Firefox and Chrome do not match the spec and location.href does not return 'about:blank' after the browsing context is gone is causing failures on the following existing tests:

@cdumez cdumez merged commit 1389ca3 into web-platform-tests:master Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants