Closed
Bug 1441277
Opened 6 years ago
Closed 6 years ago
nsINode needs better documentation on GetComposedDoc vs GetUncomposedDoc
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: dholbert, Assigned: smaug)
References
Details
Attachments
(1 file)
888 bytes,
patch
|
annevk
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(bugs)
Reporter | ||
Comment 1•6 years ago
|
||
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.)
Reporter | ||
Updated•6 years ago
|
Summary: nsINode needs better documentation on Composed vs Uncomposed → nsINode needs better documentation on GetComposedDoc vs GetUncomposedDoc
Comment 2•6 years ago
|
||
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.
Assignee | ||
Comment 3•6 years ago
|
||
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)
Comment 4•6 years ago
|
||
Ah, my bad, that is what we call https://dom.spec.whatwg.org/#connected these days.
Updated•6 years ago
|
Flags: needinfo?(bugs)
Comment 5•6 years ago
|
||
The reason I needinfo'd you is since you probably need to make a decision on documentation here. Should this block bug 1205323?
Assignee | ||
Comment 6•6 years ago
|
||
perhaps something like this
Comment 7•6 years ago
|
||
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
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/82340739f3aa
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
status-firefox60:
affected → ---
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•