merge lxml-3.3 branch into master

1 files changed, 24 insertions, 26 deletions

diff --git a/src/lxml/apihelpers.pxi b/src/lxml/apihelpers.pxi
index c2b1c8e4..43076d09 100644
--- a/src/lxml/apihelpers.pxi
+++ b/src/lxml/apihelpers.pxi
@@ -74,6 +74,13 @@ cdef _Element _rootNodeOrRaise(object input):
     _assertValidNode(node)
     return node
 
+cdef bint _isAncestorOrSame(xmlNode* c_ancestor, xmlNode* c_node):
+    while c_node:
+        if c_node is c_ancestor:
+            return True
+        c_node = c_node.parent
+    return False
+
 cdef _Element _makeElement(tag, xmlDoc* c_doc, _Document doc,
                            _BaseParser parser, text, tail, attrib, nsmap,
                            dict extra_attrs):
@@ -1243,11 +1250,8 @@ cdef int _appendChild(_Element parent, _Element child) except -1:
     c_node = child._c_node
     c_source_doc = c_node.doc
     # prevent cycles
-    c_parent = parent._c_node
-    while c_parent:
-        if c_parent is c_node:
-            raise ValueError("cannot append parent to itself")
-        c_parent = c_parent.parent
+    if _isAncestorOrSame(c_node, parent._c_node):
+        raise ValueError("cannot append parent to itself")
     # store possible text node
     c_next = c_node.next
     # move node itself
@@ -1265,11 +1269,8 @@ cdef int _prependChild(_Element parent, _Element child) except -1:
     c_node = child._c_node
     c_source_doc = c_node.doc
     # prevent cycles
-    c_parent = parent._c_node
-    while c_parent:
-        if c_parent is c_node:
-            raise ValueError("cannot append parent to itself")
-        c_parent = c_parent.parent
+    if _isAncestorOrSame(c_node, parent._c_node):
+        raise ValueError("cannot append parent to itself")
     # store possible text node
     c_next = c_node.next
     # move node itself
@@ -1288,31 +1289,28 @@ cdef int _prependChild(_Element parent, _Element child) except -1:
 cdef int _appendSibling(_Element element, _Element sibling) except -1:
     u"""Add a new sibling behind an element.
     """
-    c_node = sibling._c_node
-    if element._c_node is c_node:
-        return 0  # nothing to do
-    c_source_doc = c_node.doc
-    # store possible text node
-    c_next = c_node.next
-    # move node itself
-    tree.xmlAddNextSibling(element._c_node, c_node)
-    _moveTail(c_next, c_node)
-    # uh oh, elements may be pointing to different doc when
-    # parent element has moved; change them too..
-    moveNodeToDocument(element._doc, c_source_doc, c_node)
-    return 0
+    return _addSibling(element, sibling, as_next=True)
 
 cdef int _prependSibling(_Element element, _Element sibling) except -1:
     u"""Add a new sibling before an element.
     """
+    return _addSibling(element, sibling, as_next=False)
+
+cdef int _addSibling(_Element element, _Element sibling, bint as_next) except -1:
     c_node = sibling._c_node
-    if element._c_node is c_node:
-        return 0  # nothing to do
     c_source_doc = c_node.doc
+    # prevent cycles
+    if _isAncestorOrSame(c_node, element._c_node):
+        if element._c_node is c_node:
+            return 0  # nothing to do
+        raise ValueError("cannot add ancestor as sibling, please break cycle first")
     # store possible text node
     c_next = c_node.next
     # move node itself
-    tree.xmlAddPrevSibling(element._c_node, c_node)
+    if as_next:
+        tree.xmlAddNextSibling(element._c_node, c_node)
+    else:
+        tree.xmlAddPrevSibling(element._c_node, c_node)
     _moveTail(c_next, c_node)
     # uh oh, elements may be pointing to different doc when
     # parent element has moved; change them too..