Browse Source

Improvements to new XML class' DOM manipulation functions.

Summary of changes:

- When doing insertChild() remove from old location/parent
- When doing insertChild() update .parent
- When doing removeChild() set child.parent to null
- More precise documentation of behaviour in the comments
- Unit tests to verify this behaviour

Fixes #4094
Jason O'Neil 10 years ago
parent
commit
30f1d1ee7f
2 changed files with 58 additions and 3 deletions
  1. 16 3
      std/Xml.hx
  2. 42 0
      tests/unit/src/unit/issues/Issue4094.hx

+ 16 - 3
std/Xml.hx

@@ -265,7 +265,9 @@ 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, there will be no effect.
+		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();
@@ -284,15 +286,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 removed and added at the new position.
+		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 +326,4 @@ class Xml {
 			throw 'Bad node type, expected Element or Document but found $nodeType';
 		}
 	}
-}
+}

+ 42 - 0
tests/unit/src/unit/issues/Issue4094.hx

@@ -0,0 +1,42 @@
+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 );
+    }
+}