-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 base URL with multiple globals for WebSocket #39978
Conversation
And thereby relative URLs. They are instantly normalized to the ws: and wss: schemes. Tests: web-platform-tests/wpt#39955 and web-platform-tests/wpt#39978. Co-authored-by: Gus Caplan <[email protected]> Co-authored-by: Adam Rice <[email protected]>
window.scriptToRun = ` | ||
const result = {}; | ||
try { | ||
const ws = new WebSocket('foo'); |
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.
This ends up creating the WebSocket
object in the incumbent realm I think, which then ends up being its relevant realm.
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.
In general the way of testing these subtleties for constructor APIs like WebSocket or Worker are quite different than how you'd test them for methods like location.assign(). So I think following the pattern from https://github.com/web-platform-tests/wpt/tree/master/workers/multi-globals would work best.
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.
This is not a great location for this file, since html/browsers/browsing-the-web/navigating-across-documents/ is supposed to be about the location.x()
APIs.
Instead, it's probably best to create a new directory under websockets/, similar to how https://github.com/web-platform-tests/wpt/tree/master/workers/multi-globals is under workers/
window.scriptToRun = ` | ||
const result = {}; | ||
try { | ||
const ws = new WebSocket('foo'); |
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.
In general the way of testing these subtleties for constructor APIs like WebSocket or Worker are quite different than how you'd test them for methods like location.assign(). So I think following the pattern from https://github.com/web-platform-tests/wpt/tree/master/workers/multi-globals would work best.
Thanks, I've changed the test to be more like I used |
See whatwg/websockets#45