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

Remove entry settings object usage in the WebSocket constructor #6655

Merged
merged 2 commits into from
May 6, 2021

Conversation

domenic
Copy link
Member

@domenic domenic commented May 5, 2021

This matches implementations, and helps with #1431.

Also, slightly modernize the algorithm.

(See WHATWG Working Mode: Changes for more details.)

/cc @ricea


/web-sockets.html ( diff )

This matches implementations, and helps with #1431.

Also, slightly modernize the algorithm.
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.

@ricea FYI.

@@ -97687,8 +97687,8 @@ interface <dfn>WebSocket</dfn> : <span>EventTarget</span> {

<div w-nodev>

<p>The <dfn constructor for="WebSocket"><code data-x="dom-WebSocket">WebSocket(<var>url</var>,
<var>protocols</var>)</code></dfn> constructor, when invoked, must run these steps:</p>
<p>The <dfn constructor for="WebSocket"><code data-x="dom-WebSocket">new WebSocket(<var>url</var>,
Copy link
Member

Choose a reason for hiding this comment

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

For Bikeshed you'd have to add lt="WebSocket(url, protocols)". (Otherwise it wouldn't be able to find this from the IDL.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Wattsi disallows lt="" on <dfn> right now :(. I think maybe we should have Wattsi or Bikeshed be smarter about this case, e.g. Wattsi could generate lt="" when the existing text starts with new, or Bikeshed could learn about this style.

Copy link
Member

Choose a reason for hiding this comment

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

We used data-lt="" elsewhere, but I also agree with that.

@ricea
Copy link

ricea commented May 6, 2021

lgtm

@domenic domenic merged commit 083c65c into main May 6, 2021
@domenic domenic deleted the no-websocket-entry branch May 6, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants