Closed Bug 1441277 Opened 2 years ago Closed 2 years ago

nsINode needs better documentation on GetComposedDoc vs GetUncomposedDoc

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: dholbert, Assigned: smaug)

References

Details

Attachments

(1 file)

I'm trying to review a patch that involves some calls to the nsINode APIs GetUncomposedDoc(), GetComposedDoc(), and OwnerDoc(), and I'm having a hard time finding clear documentation on the subtleties of these methods.

In particular, GetComposedDoc is documented as follows:
>   /**
>    * This method returns the owner doc if the node is in the
>    * composed document (as defined in the Shadow DOM spec), otherwise
>    * it returns null.
>    */
>   nsIDocument* GetComposedDoc() const
>   {
>     return IsInShadowTree() ?
>       GetComposedDocInternal() : GetUncomposedDoc();
>   }
https://searchfox.org/mozilla-central/rev/b469db5c90d618f4b202d5ef280e1a78224eb43b/dom/base/nsINode.h#593-598

Several issues here:
 (A) Nowadays, the "Shadow DOM spec"[1] has a warning saying that it's been replaced/upstreamed to the WHATWG DOM spec[2], per https://github.com/w3c/webcomponents/issues/377 , so it's presumably not authoritative anymore.

 (B) Neither of these specs (W3C Shadow DOM nor WHATWG DOM spec) actually *define* the term "composed document". The authoritative one (the DOM spec) never mentions "composed document" at all, and the older one uses it twice without explanation.

So it's a bit bogus for our documentation to lean on this term being "defined in the Shadow DOM spec".  Can we clarify this documentation to make the meaning clearer & grounded in something more concrete, spec-wise?

[1] https://www.w3.org/TR/shadow-dom/
[2] https://dom.spec.whatwg.org
Flags: needinfo?(bugs)
For reference, the previous revision of GetComposedDoc()'s documentation (before bug 1087460) leans a bit less on spec definitions:
>   * This method gets the current doc of the node hosting this content
>   * or the current doc of this content if it is not being hosted. This
>   * method walks through ShadowRoot boundaries until it reach the host
>   * that is located in the root of the "tree of trees" (see Shadow DOM
>   * spec) and returns the current doc for that host.
>   */
>  nsIDocument* GetComposedDoc() const
https://searchfox.org/mozilla-central/rev/63e3218e4c61f9e44f22fb06583477a8bdaadf02/dom/base/nsINode.h#522-529

(Quoting it here in case it helps inform what the correct modern documentation should look like.)
Summary: nsINode needs better documentation on Composed vs Uncomposed → nsINode needs better documentation on GetComposedDoc vs GetUncomposedDoc
The way this works specification-wise is that only events and the box tree operate on a "final/flattened/composed" tree. Say if A was a shadow host and it had a child B and A's shadow root consisted of C and a slot child S. So you had A -> B, C -> S, and A -- C as relations. Then the "final/flattened/composed" tree would be A -> C -> S -> B. For events we don't really have to talk about this abstraction as we can use operations that produce identical results.

My idea has always been that CSS would define this abstraction as it's rather necessary for the box tree to have this, but it's not really needed elsewhere. I filed https://github.com/w3c/csswg-drafts/issues/2365 on this.
In-composed-doc/composedDoc means that there is possibly shadow DOM boundary crossing path from a node to document.
So either traversing .parentNode or .host one will get to a Document object eventually.
Composed doc as such doesn't have anything to do with flattened tree.
Flags: needinfo?(bugs)
Ah, my bad, that is what we call https://dom.spec.whatwg.org/#connected these days.
Flags: needinfo?(bugs)
The reason I needinfo'd you is since you probably need to make a decision on documentation here. Should this block bug 1205323?
perhaps something like this
Assignee: nobody → bugs
Flags: needinfo?(bugs)
Attachment #8963033 - Flags: review?(annevk)
Comment on attachment 8963033 [details] [diff] [review]
better_doc_doc.diff

Review of attachment 8963033 [details] [diff] [review]:
-----------------------------------------------------------------

I think technically I'm not allowed to do this, but since it's a comment...
Attachment #8963033 - Flags: review?(annevk) → review+
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/82340739f3aa
nsINode needs better documentation on GetComposedDoc vs GetUncomposedDoc, r=annevk
https://hg.mozilla.org/mozilla-central/rev/82340739f3aa
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.