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

Add test for Joint session state manipulation #16381

Merged
merged 3 commits into from Jun 4, 2019

Conversation

jutaz
Copy link
Contributor

@jutaz jutaz commented Apr 17, 2019

Some browsers don't seem to agree on how top-level container should handle the navigation of the child, particularly around if the state should be overwritten by the child or not. This is particularly prominent in WebKit based browsers, such as Safari.

As far as I've tested this, it appears that these browsers pass the test:

  • Chrome 73.0.3683.103
  • Chrome Canary 75.0.3767.0
  • Firefox ESR 60

These browsers don't pass the test:

  • Safari 12.1 (14607.1.40.1.4)
  • Safari TP (Safari 12.2, WebKit 14608.1.14)

This leads me to believe that Chrome & Firefox's implementation is correct, thus this test is geared to pass in these browsers. Ideally we'd follow the history spec, but as far as I'm aware, it does not specify how browsers should work in this particular scenario.

Looking at this pragmatically as well, it appears that keeping the window.history.state in the navigation makes sense, as the top window did not actually change it's state, just the child iframe, even if navigation occurred and going back in history means that child iframe will navigate back. That, however, does not alter the state of the parent window, which is why it appears that implementation in Chrome/Firefox seems correct.

References

Some browers don't seem to agree on how top-level container should handle the navigation of the child, particularly around if the state should be overwritten by the child or not.
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

If push state history is indeed per global this looks correct to me. It would be nice to assert the child's state before you navigate it as well.

@cdumez you probably wanna look at this and comment regarding WebKit's perspective.

@annevk
Copy link
Member

annevk commented Apr 30, 2019

(Forgot to say I really appreciate your work on this @jutaz!)

@jutaz
Copy link
Contributor Author

jutaz commented Apr 30, 2019

It would be nice to assert the child's state before you navigate it as well.

Can do 👍

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I'll leave this open for a bit to let @cdumez comment, but do ping again if you feel like you've been waiting too long and I'll get it merged. Thanks again!

@jutaz
Copy link
Contributor Author

jutaz commented Apr 30, 2019

Thanks @annevk! I'd love to hear from Webkit's team, so all good for now 👍

@jutaz
Copy link
Contributor Author

jutaz commented Jun 4, 2019

Bumping this - would love to get this merged 🙏

@annevk annevk merged commit 95fd9ed into web-platform-tests:master Jun 4, 2019
@jutaz
Copy link
Contributor Author

jutaz commented Jun 4, 2019

Thanks @annevk!

@jutaz jutaz deleted the feature/iframe-state-test branch June 4, 2019 09:51
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

4 participants