fix: reconciliation of ns on unparented nodes

Closes #3459

Author: flavorjones

ext/java/nokogiri/XmlNode.java

@@ -421,6 +421,7 @@ public class XmlNode extends RubyObject
     }
 
     Element e = (Element) node;
+    Node parent = e.getParentNode();
 
     // disable error checking to prevent lines like the following
     // from throwing a `NAMESPACE_ERR' exception:
@@ -439,22 +440,27 @@ public class XmlNode extends RubyObject
         set_namespace(context, ((XmlNode)parent(context)).namespace(context));
       }
     } else {
-      String currentPrefix = e.getParentNode().lookupPrefix(nsURI);
-      String currentURI = e.getParentNode().lookupNamespaceURI(prefix);
-      boolean isDefault = e.getParentNode().isDefaultNamespace(nsURI);
-
       // add xmlns attribute if this is a new root node or if the node's
       // namespace isn't a default namespace in the new document
-      if (e.getParentNode().getNodeType() == Node.DOCUMENT_NODE) {
+      if (parent == null || parent.getNodeType() == Node.DOCUMENT_NODE) {
         // this is the root node, so we must set the namespaces attributes anyway
         e.setAttribute(prefix == null ? "xmlns" : "xmlns:" + prefix, nsURI);
       } else if (prefix == null) {
         // this is a default namespace but isn't the default where this node is being added
-        if (!isDefault) { e.setAttribute("xmlns", nsURI); }
-      } else if (!prefix.equals(currentPrefix) || !nsURI.equals(currentURI)) {
-        // this is a prefixed namespace
-        // but doesn't have the same prefix or the prefix is set to a different URI
-        e.setAttribute("xmlns:" + prefix, nsURI);
+        boolean isDefault = parent.isDefaultNamespace(nsURI);
+
+        if (!isDefault) {
+          e.setAttribute("xmlns", nsURI);
+        }
+      } else if (parent.getNodeType() == Node.ELEMENT_NODE) {
+        String currentPrefix = parent.lookupPrefix(nsURI);
+        String currentURI = parent.lookupNamespaceURI(prefix);
+
+        if (!prefix.equals(currentPrefix) || !nsURI.equals(currentURI)) {
+          // this is a prefixed namespace
+          // but doesn't have the same prefix or the prefix is set to a different URI
+          e.setAttribute("xmlns:" + prefix, nsURI);
+        }
       }
     }
 
@@ -492,7 +498,7 @@ public class XmlNode extends RubyObject
 
     // if this namespace is a duplicate of what's already in the document, remove it.
     // a "duplicate" here is if the prefix and the URI both match what resolves in the parent.
-    if (e.getParentNode().getNodeType() == Node.ELEMENT_NODE) {
+    if (parent != null && parent.getNodeType() == Node.ELEMENT_NODE) {
       RubyArray<?> nsdefs = this.namespace_definitions(context);
       for (int j = 0 ; j < nsdefs.getLength() ; j++) {
         XmlNamespace ns = (XmlNamespace)nsdefs.get(j);
@@ -1524,6 +1530,8 @@ public class XmlNode extends RubyObject
       // and keep the return value. -mbklein
       String new_name = NokogiriHelpers.newQName(ns.getPrefix(), node);
       this.node = NokogiriHelpers.renameNode(node, ns.getHref(), new_name);
+
+      this.relink_namespace(context);
     }
 
     clearXpathContext(getNode());

ext/nokogiri/xml_node.c

@@ -1334,6 +1334,10 @@ set_namespace(VALUE self, VALUE namespace)
 
   xmlSetNs(node, ns);
 
+  if (ns && ns->prefix)  {
+    xmlReconciliateNs(node->doc, node);
+  }
+
   return self;
 }
 

test/xml/test_node.rb

@@ -1336,6 +1336,19 @@ def test_namespace_in_rendered_xml
           assert_match(/xmlns="bar"/, subject.to_xml)
         end
 
+        # https://github.com/sparklemotion/nokogiri/issues/3459
+        def test_namespace_set_on_unparented_node_also_sets_a_definition
+          doc = Nokogiri::XML(%{<root xmlns:foo="http://example.com/foo"></root>})
+          ns = doc.root.namespace_definitions.first
+
+          node = doc.create_element("other")
+          node.namespace = ns
+
+          assert_pattern do
+            node.namespace_definitions => [ { prefix: "foo", href: "http://example.com/foo" } ]
+          end
+        end
+
         # issue 771
         def test_format_noblank
           content = <<~XML

test/xml/test_node_reparenting.rb

@@ -845,6 +845,29 @@ def coerce(data)
             end
           end
         end
+
+        # https://github.com/sparklemotion/nokogiri/issues/3459
+        describe "reparenting with duplicate namespace prefixes" do
+          it "stitches together ok" do
+            doc = Nokogiri::XML(<<~XML)
+              <dnd:adventure xmlns:dnd="http://www.w3.org/dungeons#">
+                <dnd:party xmlns:dnd="http://www.w3.org/dragons#">
+                  <dnd:members>
+                  </dnd:members>
+                </dnd:party>
+              </dnd:adventure>
+            XML
+
+            dungeons_ns = doc.root.namespace_definitions.find { |ns| ns.prefix == "dnd" }
+            parent = doc.xpath("//ns:members", ns: "http://www.w3.org/dragons#").first
+
+            node = doc.create_element("character")
+            node.namespace = dungeons_ns
+            parent.add_child(node)
+
+            assert_includes(doc.to_xml, %{<dnd:character xmlns:dnd="http://www.w3.org/dungeons#"/>})
+          end
+        end
       end
     end
   end