From 8c1cd4ed86c5a4ce7c7f05c9b3303f41c32f8786 Mon Sep 17 00:00:00 2001 From: Franciszek Szczepan Wawrzak Date: Mon, 11 Sep 2023 14:17:49 +0200 Subject: [PATCH 1/2] fix replaceWith for node argument that is already in document tree --- lib/src/html/dom/node.dart | 62 ++++++++++++++----------------------- test/src/html/dom/node.dart | 28 ++++++++++++++--- 2 files changed, 48 insertions(+), 42 deletions(-) diff --git a/lib/src/html/dom/node.dart b/lib/src/html/dom/node.dart index a57ceee..78dd126 100644 --- a/lib/src/html/dom/node.dart +++ b/lib/src/html/dom/node.dart @@ -389,13 +389,23 @@ abstract class Node extends EventTarget { void remove() { final parent = _parent; if (parent == null) { - assert(_previousNode == null); - assert(_nextNode == null); return; } // Mark node as dirty _markDirty(); + _removeFromTree(null); + parent._mutated(); + _mutated(); + } + + void _removeFromTree(Node? replaceWith) { + final parent = _parent; + if (parent == null) { + assert(_previousNode == null); + assert(_nextNode == null); + return; + } // Get previous and next final previous = _previousNode; @@ -403,66 +413,42 @@ abstract class Node extends EventTarget { if (previous == null) { // This was the first sibling - parent._firstChild = next; + parent._firstChild = replaceWith ?? next; } else { // Mutate the previous sibling - previous._nextNode = next; + previous._nextNode = replaceWith ?? next; } if (next == null) { // This was the last sibling - parent._lastChild = previous; + parent._lastChild = replaceWith ?? previous; } else { // Mutate the next sibling - next._previousNode = previous; + next._previousNode = replaceWith ?? previous; + } + if (replaceWith != null) { + // move replaceWith + replaceWith._parent = parent; + replaceWith._previousNode = previous; + replaceWith._nextNode = next; } - // Set fields of this node _parent = null; _previousNode = null; _nextNode = null; - parent._mutated(); - _mutated(); } void replaceWith(Node node) { final parent = _parent; if (parent == null) { - assert(_previousNode == null); - assert(_nextNode == null); return; } // Mark nodes as dirty _markDirty(); node._markDirty(); - - // Get previous and next - final previous = _previousNode; - final next = _nextNode; - - if (previous == null) { - // This was the first sibling - parent._firstChild = node; - } else { - // Mutate the previous sibling - previous._nextNode = node; - } - - if (next == null) { - // This was the last sibling - parent._lastChild = node; - } else { - // Mutate the next sibling - next._previousNode = node; - } - - node._parent = parent; - node._previousNode = previous; - node._nextNode = next; - _parent = null; - _previousNode = null; - _nextNode = null; + node._removeFromTree(null); + _removeFromTree(node); _mutated(); node._mutated(); } diff --git a/test/src/html/dom/node.dart b/test/src/html/dom/node.dart index 6c0dbc9..ecb578d 100644 --- a/test/src/html/dom/node.dart +++ b/test/src/html/dom/node.dart @@ -37,7 +37,11 @@ void _testNode() { test('replaceWith', () { final e0 = Element.tag('e0')..appendText('e0-text'); final e1 = Element.tag('e1')..appendText('e1-text'); - final e2 = Element.tag('e2')..appendText('e2-text'); + final e2 = Element.tag('e2')..append( + Element.tag('e2c1') + )..append( + Element.tag('e2c2') + ); final root = Element.tag('sometag') ..setAttribute('k', 'v') ..append(e0) @@ -48,7 +52,7 @@ void _testNode() { expect( root.outerHtml, equals( - 'e0-texte1-texte2-text')); + 'e0-texte1-text')); // Replace child #1 of 'e1' { @@ -64,7 +68,7 @@ void _testNode() { expect( root.outerHtml, equals( - 'e0-texte1-text-replacede2-text')); + 'e0-texte1-text-replaced')); } // Replace child #2 of root ('e1') @@ -83,7 +87,23 @@ void _testNode() { expect( root.outerHtml, equals( - 'e0-texte1-replacede2-text')); + 'e0-texte1-replaced')); + } + // Replace with existing node to check if it moved properly + { + final replacement = e2.children.first; + e0.replaceWith(replacement); + + expect(e0.nextNode, isNull); + expect(e0.previousNode, isNull); + expect(e0.parent, isNull); + expect(replacement.parent, same(root)); + expect(replacement.previousNode, isNull); + expect(replacement.nextNode, isNotNull); + expect( + root.outerHtml, + equals( + 'e1-replaced')); } }); test('replaceWith when the node has no parent', () { From 529a0339cb985b8d34d6ba85d8275a4c41b9c9d9 Mon Sep 17 00:00:00 2001 From: Franciszek Szczepan Wawrzak Date: Mon, 11 Sep 2023 14:23:50 +0200 Subject: [PATCH 2/2] fix replaceWith, cleanup --- lib/src/html/dom/node.dart | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/src/html/dom/node.dart b/lib/src/html/dom/node.dart index 78dd126..7523ee0 100644 --- a/lib/src/html/dom/node.dart +++ b/lib/src/html/dom/node.dart @@ -386,19 +386,6 @@ abstract class Node extends EventTarget { return sb.toString(); } - void remove() { - final parent = _parent; - if (parent == null) { - return; - } - - // Mark node as dirty - _markDirty(); - _removeFromTree(null); - parent._mutated(); - _mutated(); - } - void _removeFromTree(Node? replaceWith) { final parent = _parent; if (parent == null) { @@ -438,12 +425,16 @@ abstract class Node extends EventTarget { _nextNode = null; } - void replaceWith(Node node) { + void remove() { final parent = _parent; - if (parent == null) { - return; - } + // Mark node as dirty + _markDirty(); + _removeFromTree(null); + parent?._mutated(); + _mutated(); + } + void replaceWith(Node node) { // Mark nodes as dirty _markDirty(); node._markDirty();