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

Revamp the way DOM talks about nodes #1004

Merged
merged 6 commits into from
Aug 31, 2021
Merged

Revamp the way DOM talks about nodes #1004

merged 6 commits into from
Aug 31, 2021

Conversation

annevk
Copy link
Member

@annevk annevk commented Aug 3, 2021

  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.

Fixes whatwg/webidl#659, #597, #636, #719, and #770.

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


Preview | Diff

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.
@annevk annevk requested a review from domenic August 3, 2021 13:20
Copy link
Contributor

@sideshowbarker sideshowbarker left a 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

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Looks good!

dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
@annevk annevk marked this pull request as ready for review August 4, 2021 12:32
dom.bs Outdated Show resolved Hide resolved
<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
Copy link
Member

Choose a reason for hiding this comment

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

"does not implement"?

@annevk annevk requested a review from domenic August 5, 2021 12:35
Copy link
Member

@domenic domenic left a 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
Copy link
Member

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.
Copy link
Member

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
Copy link
Member

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.

@annevk annevk merged commit 0715c2f into main Aug 31, 2021
@annevk annevk deleted the annevk/nodes branch August 31, 2021 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Referencing "implements"
3 participants