Skip to content

Commit

Permalink
Fix cloning of templates with DOM Parts, and improve testing
Browse files Browse the repository at this point in the history 
This CL improves the testing of template cloning with Parts, testing
these four cases:

  1. Main document parsing
  2. Template (content fragment) parsing
  3. Template/fragment cloning
  4. Declarative Shadow DOM parsing and cloning

This CL fixes the behavior for #3 above, but leaves #4 broken. The
following changes in behavior are made:

1. Part::MoveToRoot() can be used to change the root(), including
   to set it to nullptr. This happens when a Node tree is removed
   from the DOM, and it contains Parts that refer to the old root.
2. IsDocumentPartRoot() is now virtual, because during a tree move,
   the root() for a Part can be made nullptr even when it's a
   ChildNodePart.
3. Part::disconnected_ is added to keep track of whether the
   Part has been disconnected, since root() can now be nullptr.
4. (This is a bug fix) When using ChildNodePart::setNextSibling(),
   the new sibling node wasn't having its Part registered with
   NodeRareData, which caused a CHECK failure when trying to
   subsequently clone that Part. This is caught in the new test
   which clones declaratively-built templates containing Parts.

Bug: 1453291
Change-Id: Ic1c1475431cf6bd658f191db78003204412ef78f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4713668
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1175782}
  • Loading branch information
@mfreed7 @chromium-wpt-export-bot
mfreed7 authored and chromium-wpt-export-bot committed Jul 27, 2023
1 parent e432c73 commit e03faa1
Showing 1 changed file with 115 additions and 47 deletions.
162 changes: 115 additions & 47 deletions dom/parts/basic-dom-part-objects.tentative.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
<span id=sub>B-sub2</span>
</span><span id=c>C</span></div>
</div>
<span id=direct_child_1></span>
<span id=direct_child_2></span>
</template>

<script>{
Expand All @@ -40,13 +42,15 @@
}
[false,true].forEach(useTemplate => {
const doc = useTemplate ? template.content : document;
let target,wrapper;
let target,wrapper,directChildren;
if (useTemplate) {
target = doc.querySelector('#target1');
directChildren = [doc.querySelector('#direct_child_1'),doc.querySelector('#direct_child_2')];
} else {
wrapper = document.body.appendChild(document.createElement('div'));
wrapper.appendChild(template.content.cloneNode(true));
target = wrapper.querySelector('#target1');
directChildren = [doc.documentElement,doc.documentElement];
}
const a = target.querySelector('#a');
const b = target.querySelector('#b');
Expand Down Expand Up @@ -251,18 +255,38 @@
assert_array_equals(target.children,[a,b,c]);
}, `ChildNodePart children manipulation (${description})`);

test((t) => {
const root = doc.getPartRoot();
// Make sure no crashes occur for invalid parts.
const cornerCasePartsInvalid = [
addCleanup(t,new ChildNodePart(root,target, target.children[2],{metadata: ['different parents']})),
addCleanup(t,new ChildNodePart(root,target.children[0], target,{metadata: ['different parents']})),
addCleanup(t,new ChildNodePart(root,target, doc,{metadata: ['different parents']})),
];
const cornerCasePartsValid = [];
const maybeValid = addCleanup(t,new ChildNodePart(root,directChildren[0], directChildren[1],{metadata: ['direct parent of the root container']}));
if (directChildren[0] === directChildren[1]) {
cornerCasePartsInvalid.push(maybeValid);
} else {
cornerCasePartsValid.push(maybeValid);
}
assert_array_equals(root.getParts(),cornerCasePartsValid);
assert_equals(root.clone().getParts().length,cornerCasePartsValid.length);
}, `Corner case ChildNodePart construction and cloning (${description})`);

wrapper?.remove(); // Cleanup
});
}</script>

<div>
<!-- Note - the test will remove this chunk of DOM once the test completes -->
<div id=target2>
Declarative syntax - The template below (with id=declarative) should have
IDENTICAL STRUCTURE. There are three cases to test:
Declarative syntax - The *two* templates below should have IDENTICAL STRUCTURE
to this one. There are four cases to test:
1. Main document parsing (this chunk)
2. Template parsing (the template below)
3. Template/fragment cloning (the clone of the template below)
2. Template parsing (the template below with id=declarative)
3. Template/fragment cloning (a clone of the template with id=declarative)
4. Declarative Shadow DOM parsing (template with id=declarative_shadow_dom and shadowrootmode attribute)
<h1 id="name">
<?child-node-part fullname?>
First
Expand All @@ -277,7 +301,7 @@ <h1 id="name">
<child-node-part test PI without leading ?>
<!--?child-node-partfoobar?-->
<?child-node-partfoobar?>
</div>
</div>
</div>
<template id=declarative>
<div>
Expand All @@ -296,61 +320,105 @@ <h1 id="name">
<child-node-part test PI without leading ?>
<!--?child-node-partfoobar?-->
<?child-node-partfoobar?>
</div>
</div>
</div>
</template>
<div id=declarative_shadow_dom>
<template shadowrootmode="open">
<div>
<div id=target3>Declarative syntax
<h1 id="name">
<?child-node-part fullname?>
First
<!--?child-node-part middle?--> <?node-part middle-node?>Middle <?/child-node-part middle?>
Last
<!-- ?/child-node-part foobar? -->
</h1>
Email: <?node-part email-link?><a id="link"></a>

Here are some invalid parts that should not get parsed:
<!--child-node-part test comment without leading ?-->
<child-node-part test PI without leading ?>
<!--?child-node-partfoobar?-->
<?child-node-partfoobar?>
</div>
</div>
</template>
</div>

<script> {
const template = document.getElementById('declarative');
['Main Document','Template','Clone'].forEach(testCase => {
let doc,target,cleanup;
switch (testCase) {
case 'Main Document':
doc = document;
target = doc.querySelector('#target2');
cleanup = [target.parentElement];
break;
case 'Template':
doc = template.content;
target = doc.querySelector('#target3');
cleanup = [];
break;
case 'Clone':
doc = document;
wrapper = document.body.appendChild(document.createElement('div'));
wrapper.appendChild(template.content.cloneNode(true));
target = wrapper.querySelector('#target3');
cleanup = [wrapper];
break;
}
assert_true(!!(doc && target && target.parentElement));

['Main Document','Template','Clone','PartClone','DeclarativeShadowDOM'].forEach(testCase => {
function assertIsComment(node,commentText) {
assert_true(node instanceof Comment);
assert_equals(node.textContent,commentText);
}

test((t) => {
let doc,target,wrapper,cleanup;
let expectDOMParts = true;
switch (testCase) {
case 'Main Document':
doc = document;
target = doc.querySelector('#target2');
cleanup = [target.parentElement];
break;
case 'Template':
doc = template.content;
target = doc.querySelector('#target3');
cleanup = [];
break;
case 'Clone':
doc = document;
wrapper = document.body.appendChild(document.createElement('div'));
wrapper.appendChild(template.content.cloneNode(true));
target = wrapper.querySelector('#target3');
// A "normal" tree clone should not keep DOM Parts:
expectDOMParts = false;
cleanup = [wrapper];
break;
case 'PartClone':
doc = document;
wrapper = document.body.appendChild(document.createElement('div'));
wrapper.appendChild(template.content.getPartRoot().clone().rootContainer);
target = wrapper.querySelector('#target3');
cleanup = [wrapper];
break;
case 'DeclarativeShadowDOM': {
const host = document.getElementById('declarative_shadow_dom');
doc = host.shadowRoot;
target = doc.querySelector('#target3'); // Shadow isolated
cleanup = [host];
break;
}
default:
assert_unreached('Invalid test case');
}
assert_true(!!(doc && target && target.parentElement));
t.add_cleanup(() => cleanup.forEach(el => el.remove())); // Cleanup

const root = doc.getPartRoot();
assert_true(root instanceof DocumentPartRoot);
const expectedRootParts = [{type:'ChildNodePart',metadata:['fullname']},{type:'NodePart',metadata:['email-link']}];
assertEqualParts(root.getParts(),expectedRootParts,0,'declarative root should have two parts');
assert_equals(root.getParts()[1].node,target.querySelector('#link'));
const childPart1 = root.getParts()[0];
assertIsComment(childPart1.previousSibling,'?child-node-part fullname?');
assertIsComment(childPart1.nextSibling,' ?/child-node-part foobar? ');
const expectedChild1Parts = [{type:'ChildNodePart',metadata:['middle']}];
assertEqualParts(childPart1.getParts(),expectedChild1Parts,0,'First level childpart should just have one child part');
const childPart2 = childPart1.getParts()[0];
assertIsComment(childPart2.previousSibling,'?child-node-part middle?');
assertIsComment(childPart2.nextSibling,'?/child-node-part middle?');
const expectedChild2Parts = [{type:'NodePart',metadata:['middle-node']}];
assertEqualParts(childPart2.getParts(),expectedChild2Parts,0,'Second level childpart should have just the node part');
assert_true(childPart2.getParts()[0].node instanceof Text);
assert_equals(childPart2.getParts()[0].node.textContent,'Middle ');
if (expectDOMParts) {
const expectedRootParts = [{type:'ChildNodePart',metadata:['fullname']},{type:'NodePart',metadata:['email-link']}];
assertEqualParts(root.getParts(),expectedRootParts,0,'declarative root should have two parts');
assert_equals(root.getParts()[1].node,target.querySelector('#link'));
const childPart1 = root.getParts()[0];
assertIsComment(childPart1.previousSibling,'?child-node-part fullname?');
assertIsComment(childPart1.nextSibling,' ?/child-node-part foobar? ');
const expectedChild1Parts = [{type:'ChildNodePart',metadata:['middle']}];
assertEqualParts(childPart1.getParts(),expectedChild1Parts,0,'First level childpart should just have one child part');
const childPart2 = childPart1.getParts()[0];
assertIsComment(childPart2.previousSibling,'?child-node-part middle?');
assertIsComment(childPart2.nextSibling,'?/child-node-part middle?');
const expectedChild2Parts = [{type:'NodePart',metadata:['middle-node']}];
assertEqualParts(childPart2.getParts(),expectedChild2Parts,0,'Second level childpart should have just the node part');
assert_true(childPart2.getParts()[0].node instanceof Text);
assert_equals(childPart2.getParts()[0].node.textContent,'Middle ');
} else {
assertEqualParts(root.getParts(),[],[]);
}
}, `Basic declarative DOM Parts (${testCase})`);

cleanup.forEach(el => el.remove()); // Cleanup
});

}</script>

0 comments on commit e03faa1

Please sign in to comment.