KEMBAR78
Remove entry settings object usage in the WebSocket constructor by domenic · Pull Request #6655 · whatwg/html · GitHub
Skip to content

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.

source Outdated

<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