Merge pull request #4095 from jasononeil/Xml_insertChild_fix

Improvements to new XML class' DOM manipulation functions.

std/Xml.hx

@@ -265,13 +265,13 @@ class Xml {
 
 	/**
 		Adds a child node to the Document or Element.
-		One node can only be inside one given node which is indicated by the [parent] property.
+		A child node can only be inside one given parent node, which is indicated by the [parent] property.
+		If the child is already inside this Document or Element, it will be moved to the last position among the Document or Element's children.
+		If the child node was previously inside a different node, it will be moved to this Document or Element.
 	**/
 	public function addChild( x : Xml ) : Void {
 		ensureElementType();
-		if (x.parent == this) {
-			return;
-		} else if (x.parent != null) {
+		if (x.parent != null) {
 			x.parent.removeChild(x);
 		}
 		children.push(x);
@@ -284,15 +284,26 @@ class Xml {
 	**/
 	public function removeChild( x : Xml ) : Bool {
 		ensureElementType();
-		return children.remove(x);
+		if (children.remove(x)) {
+			x.parent = null;
+			return true;
+		}
+		return false;
 	}
 
 	/**
 		Inserts a child at the given position among the other childs.
+		A child node can only be inside one given parent node, which is indicated by the [parent] property.
+		If the child is already inside this Document or Element, it will be moved to the new position among the Document or Element's children.
+		If the child node was previously inside a different node, it will be moved to this Document or Element.
 	**/
 	public function insertChild( x : Xml, pos : Int ) : Void {
 		ensureElementType();
+		if (x.parent != null) {
+			x.parent.children.remove(x);
+		}
 		children.insert(pos, x);
+		x.parent = this;
 	}
 
 	/**
@@ -313,4 +324,4 @@ class Xml {
 			throw 'Bad node type, expected Element or Document but found $nodeType';
 		}
 	}
-}
+}

tests/unit/src/unit/issues/Issue4094.hx

@@ -0,0 +1,47 @@
+package unit.issues;
+import unit.Test;
+
+class Issue4094 extends Test {
+    function test() {
+        var div = Xml.parse("<div></div>").firstElement();
+        var nodes = Xml.parse("Text1<!--Comment--><span>Span</span><div>Div</div>Text2");
+
+        // Test insertChild() moves the nodes correctly.
+        var text1 = nodes.firstChild();
+        var span = nodes.firstElement();
+
+        div.insertChild( text1, 0 );
+        div.insertChild( span, 1 );
+
+        eq( div.toString(), '<div>Text1<span>Span</span></div>' );
+        eq( nodes.toString(), '<!--Comment--><div>Div</div>Text2' );
+        eq( text1.parent, div );
+        eq( span.parent, div );
+
+        // Test removeChild() removes the nodes correctly
+        var divText = nodes.firstElement().firstChild();
+
+        nodes.firstElement().removeChild( divText );
+        eq( divText.parent, null );
+        eq( nodes.toString(), '<!--Comment--><div/>Text2' );
+
+        // Test addChild() moves the nodes correctly.
+        var comment = nodes.firstChild();
+        var innerDiv = nodes.firstElement();
+
+        div.addChild( comment );
+        div.addChild( innerDiv );
+        div.addChild( divText );
+
+        eq( div.toString(), '<div>Text1<span>Span</span><!--Comment--><div/>Div</div>' );
+        eq( nodes.toString(), 'Text2' );
+        eq( comment.parent, div );
+        eq( innerDiv.parent, div );
+        eq( divText.parent, div );
+
+        // Test addChild() moves a current child to the end.
+
+        div.addChild( span );
+        eq( div.toString(), '<div>Text1<!--Comment--><div/>Div<span>Span</span></div>' );
+    }
+}