Node#replace reimplemented using reparent_node_with. Fixes GH #162.

CHANGELOG.rdoc

@@ -12,6 +12,8 @@
   * XML fragments with namespaces do not raise an exception (regression in 1.4.0)
   * Node#matches? works in nodes contained by a DocumentFragment. GH #158
   * Document should not define add_namespace() method. GH #169
+  * XPath queries returning namespace declarations do not segfault.
+  * Node#replace works with nodes from different documents. GH #162
 
 === 1.4.0 / 2009/10/30
 

ext/nokogiri/xml_node.c

@@ -56,6 +56,17 @@ static void relink_namespace(xmlNodePtr reparented)
   }
 }
 
+/* :nodoc: */
+static xmlNodePtr xmlReplaceNodeWrapper(xmlNodePtr old, xmlNodePtr cur)
+{
+  xmlNodePtr retval ;
+  retval = xmlReplaceNode(old, cur) ;
+  if (retval == old) {
+    return cur ; // return semantics for reparent_node_with
+  }
+  return retval ;
+}
+
 /* :nodoc: */
 static VALUE reparent_node_with(VALUE node_obj, VALUE other_obj, node_other_func func)
 {
@@ -393,14 +404,7 @@ static VALUE next_element(VALUE self)
 /* :nodoc: */
 static VALUE replace(VALUE self, VALUE _new_node)
 {
-  xmlNodePtr node, new_node;
-  Data_Get_Struct(self, xmlNode, node);
-  Data_Get_Struct(_new_node, xmlNode, new_node);
-
-  xmlReplaceNode(node, new_node);
-
-  // Appropriately link in namespaces
-  relink_namespace(new_node);
+  reparent_node_with(_new_node, self, xmlReplaceNodeWrapper) ;
   return self ;
 }
 

lib/nokogiri/ffi/xml/node.rb

@@ -90,8 +90,14 @@ def next_element
       end
 
       def replace_with_node(new_node)
-        LibXML.xmlReplaceNode(cstruct, new_node.cstruct)
-        Node.send(:relink_namespace, new_node.cstruct)
+        Node.reparent_node_with(new_node, self) do |new_node_cstruct, self_cstruct|
+          retval = LibXML.xmlReplaceNode(self_cstruct, new_node_cstruct)
+          if retval == self_cstruct.pointer
+            new_node_cstruct # for reparent_node_with semantics
+          else
+            retval
+          end
+        end
         self
       end
 
@@ -390,7 +396,7 @@ def self.reparent_node_with(node, other, &block)
           node.cstruct.keep_reference_from_document!
         end
 
-        reparented_struct = LibXML::XmlNode.new(reparented_struct)
+        reparented_struct = LibXML::XmlNode.new(reparented_struct) if reparented_struct.is_a?(FFI::Pointer)
 
         # the child was a text node that was coalesced. we need to have the object
         # point at SOMETHING, or we'll totally bomb out.

test/xml/test_node.rb

@@ -757,6 +757,17 @@ def test_illegal_replace_of_node_with_doc
         assert_raises(ArgumentError){ old_node.replace new_node }
       end
 
+      def test_replace_with_node_from_different_docs
+        xml1 = "<test> <caption>Original caption</caption> </test>"
+        xml2 = "<test> <caption>Replacement caption</caption> </test>"
+        doc1 = Nokogiri::XML(xml1)
+        doc2 = Nokogiri::XML(xml2)
+        caption1 = doc1.xpath("//caption")[0]
+        caption2 = doc2.xpath("//caption")[0]
+        caption1.replace(caption2) # this segfaulted under 1.4.0 and earlier
+        assert_equal "Replacement caption", doc1.css("caption").inner_text
+      end
+
       def test_node_equality
         doc1 = Nokogiri::XML.parse(File.read(XML_FILE), XML_FILE)
         doc2 = Nokogiri::XML.parse(File.read(XML_FILE), XML_FILE)