Merge pull request #84 from fsw/replacewith-bugfix

fix replaceWith bugfix for node argument that is already in document tree

Author: terrier989

lib/src/html/dom/node.dart

@@ -393,83 +393,60 @@ abstract class Node extends EventTarget {
     return sb.toString();
   }
 
-  void remove() {
+  void _removeFromTree(Node? replaceWith) {
     final parent = _parent;
     if (parent == null) {
       assert(_previousNode == null);
       assert(_nextNode == null);
       return;
     }
 
-    // Mark node as dirty
-    _markDirty();
-
     // Get previous and next
     final previous = _previousNode;
     final next = _nextNode;
 
     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) {
+  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 replaceWith(Node node) {
     // 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();
   }

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)
@@ -46,11 +50,9 @@ void _testNode() {
 
       // Initial tree
       expect(
-        root.outerHtml,
-        equals(
-          '<sometag k="v"><e0>e0-text</e0><e1>e1-text</e1><e2>e2-text</e2></sometag>',
-        ),
-      );
+          root.outerHtml,
+          equals(
+              '<sometag k="v"><e0>e0-text</e0><e1>e1-text</e1><e2><e2c1></e2c1><e2c2></e2c2></e2></sometag>'));
 
       // Replace child #1 of 'e1'
       {
@@ -64,11 +66,9 @@ void _testNode() {
         expect(e1.firstChild, same(replacement));
         expect(e1.firstChild!.parent, same(e1));
         expect(
-          root.outerHtml,
-          equals(
-            '<sometag k="v"><e0>e0-text</e0><e1>e1-text-replaced</e1><e2>e2-text</e2></sometag>',
-          ),
-        );
+            root.outerHtml,
+            equals(
+                '<sometag k="v"><e0>e0-text</e0><e1>e1-text-replaced</e1><e2><e2c1></e2c1><e2c2></e2c2></e2></sometag>'));
       }
 
       // Replace child #2 of root ('e1')
@@ -84,12 +84,26 @@ void _testNode() {
         expect(e1.nextNode, isNull);
         expect(replacement.parent, same(root));
         expect(replacement.nextNode, same(e2));
+        expect(
+            root.outerHtml,
+            equals(
+                '<sometag k="v"><e0>e0-text</e0>e1-replaced<e2><e2c1></e2c1><e2c2></e2c2></e2></sometag>'));
+      }
+      // 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(
-            '<sometag k="v"><e0>e0-text</e0>e1-replaced<e2>e2-text</e2></sometag>',
-          ),
-        );
+            '<sometag k="v"><e2c1></e2c1>e1-replaced<e2><e2c2></e2c2></e2></sometag>'));
       }
     });
     test('replaceWith when the node has no parent', () {