Adoption and DocumentFragment #16348
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
annevk
added a commit
to whatwg/dom
that referenced
this pull request
Apr 15, 2019
And do not allow non-null host DocumentFragment nodes to be adopted. This means that a DocumentFragment is no longer implicitly adopted and a DocumentFragment with a non-null host cannot be put in a "weird" state. Tests: web-platform-tests/wpt#16348. Fixes #744.
This comment has been minimized.
This comment has been minimized.
wpt-pr-bot
requested review from
domenic,
hayatoito,
kojiishi,
rniwa,
snuggs and
TakayoshiKochi
annevk
commented
Apr 15, 2019
|
|
||
| calls = []; | ||
| doc.documentElement.appendChild(shadowRoot); | ||
| if (documentName === "the document of the template elements") { |
There was a problem hiding this comment.
@rniwa this seems a little ugly btw, but I wasn't sure how to avoid it.
There was a problem hiding this comment.
How about doc === host.content.ownerDocument ?
annevk
added a commit
to whatwg/dom
that referenced
this pull request
Dec 6, 2019
And do not allow non-null host DocumentFragment nodes to be adopted. This means that a DocumentFragment is no longer implicitly adopted and a DocumentFragment with a non-null host cannot be put in a "weird" state. Tests: web-platform-tests/wpt#16348. Fixes #744.
Tests for whatwg/dom#744. The assumption here is an early return for DocumentFragment that has a host by adoptNode() and that adoption generally doesn't affect DocumentFragment otherwise (only its children).
annevk
force-pushed
the
annevk/adopt-df
branch
from
5bf98d8
to
1b02a0b
Compare
|
@foolip care to review this? |
|
@tkent-google could you maybe review this given that you reviewed the corresponding spec PR? |
tkent-google
approved these changes
Dec 10, 2019
| return getDocument().then(function (doc) { | ||
| var instance = document.createElement('my-custom-element'); | ||
| var host = document.createElement('template'); | ||
| var shadowRoot = host.content; |
There was a problem hiding this comment.
The variable name shadowRoot is very confusing. It should be documentFragment or something.
|
|
||
| calls = []; | ||
| doc.documentElement.appendChild(shadowRoot); | ||
| if (documentName === "the document of the template elements") { |
There was a problem hiding this comment.
How about doc === host.content.ownerDocument ?
annevk
added a commit
to whatwg/dom
that referenced
this pull request
Dec 11, 2019
Also do not allow DocumentFragment nodes with host to be adopted. This means that a DocumentFragment is no longer implicitly adopted and a DocumentFragment with a (non-null) host cannot be put in a "weird" state. Tests: web-platform-tests/wpt#16348. Fixes #744.
|
Sorry, still getting used to new email filtering setup. I'll not review given that @tkent-google started doing so. |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tests for whatwg/dom#744.
The assumption here is an early return for DocumentFragment that has a host by adoptNode() and that adoption generally doesn't affect DocumentFragment otherwise (only its children).