-
Notifications
You must be signed in to change notification settings - Fork 299
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
Revamp the way DOM talks about nodes #1004
Conversation
1. Address numerous xref issues around the term node and prevent some from occurring for the term children. 2. Embrace Attr as a node even more. 3. Rely on Web IDL's implements and interface primitives to reduce the opportunities for confusion.
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.
Reviewed source and tested/reviewed Bikeshed output
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.
Looks good!
<li><p>If <var>node</var> is not a {{DocumentFragment}}, {{DocumentType}}, {{Element}}, {{Text}}, | ||
{{ProcessingInstruction}}, or {{Comment}} <a for=/>node</a>, then <a>throw</a> a | ||
<li><p>If <var>node</var> is not a {{DocumentFragment}}, {{DocumentType}}, {{Element}}, or | ||
{{CharacterData}} <a for=/>node</a>, then <a>throw</a> a |
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.
"does not implement"?
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.
LGTM with nits
dom.bs
Outdated
|
||
<p>Objects that implement {{DocumentFragment}} sometimes implement {{ShadowRoot}}. | ||
|
||
<p>Objects that <a>implement</a> {{Element}} also typically implement an inherited interface, such |
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.
The inconsistent linking of "implement" here is a bit confusing. I'd suggest either in every sentence in this note, or in just the first instance.
dom.bs
Outdated
<dt>{{ProcessingInstruction}} | ||
<dt>{{Comment}} | ||
<dt>{{CharacterData}} | ||
<dt>{{Attr}} | ||
<dd><p>None. |
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.
"No allowed children"? I had to go back and re-read the intro paragraph to see what you meant.
dom.bs
Outdated
<p>To determine the <dfn export id=concept-node-length for=Node>length</dfn> of a <a>node</a> | ||
<var>node</var>, switch on <var>node</var>: | ||
<p class=note>{{Attr}} <a for=/>nodes</a> <a>participate</a> in a <a>tree</a> for historical | ||
reasons; they never have a <a for=tree>parent</a> or <a for=tree>children</a> and are therefore |
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.
I think "their parent is always null" is a bit more technically accurate. Because https://whatpr.org/dom/1004.html#concept-tree-participate says "An object that participates in a tree has a parent", which seems to contradict this.
I guess technically "never have children" should be "always have an empty set for their children", but that one bothers me much less for some reason.
Fixes whatwg/webidl#659, #597, #636, #719, and #770.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff