fix: simplify Builder to do late binding on namespaces

so that nodes can be built that rely on namespaces they define.

Fixes #3458

Author: flavorjones

lib/nokogiri/xml/builder.rb

@@ -314,7 +314,7 @@ def initialize(options = {}, root = nil, &block)
 
         @context = nil
         @arity = nil
-        @ns = nil
+        @ns_prefix = nil
 
         options = DEFAULT_DOCUMENT_OPTIONS.merge(options)
         options.each do |k, v|
@@ -355,20 +355,8 @@ def comment(string)
       ###
       # Build a tag that is associated with namespace +ns+.  Raises an
       # ArgumentError if +ns+ has not been defined higher in the tree.
-      def [](ns)
-        if @parent != @doc
-          @ns = @parent.namespace_definitions.find { |x| x.prefix == ns.to_s }
-        end
-        return self if @ns
-
-        @parent.ancestors.each do |a|
-          next if a == doc
-
-          @ns = a.namespace_definitions.find { |x| x.prefix == ns.to_s }
-          return self if @ns
-        end
-
-        @ns = { pending: ns.to_s }
+      def [](ns_prefix)
+        @ns_prefix = ns_prefix.to_s
         self
       end
 
@@ -395,29 +383,33 @@ def method_missing(method, *args, &block) # :nodoc:
         if @context&.respond_to?(method)
           @context.send(method, *args, &block)
         else
-          node = @doc.create_element(method.to_s.sub(/[_!]$/, ""), *args) do |n|
-            # Set up the namespace
-            if @ns.is_a?(Nokogiri::XML::Namespace)
-              n.namespace = @ns
-              @ns = nil
-            end
-          end
-
-          if @ns.is_a?(Hash)
-            node.namespace = node.namespace_definitions.find { |x| x.prefix == @ns[:pending] }
-            if node.namespace.nil?
-              raise ArgumentError, "Namespace #{@ns[:pending]} has not been defined"
-            end
-
-            @ns = nil
-          end
-
+          node = @doc.create_element(method.to_s.sub(/[_!]$/, ""), *args)
+          bind_ns(node)
           insert(node, &block)
         end
       end
 
       private
 
+      def bind_ns(node)
+        return if @ns_prefix.nil?
+
+        ancestors = [node, parent, parent.ancestors].flatten
+        ancestors.each do |ancestor|
+          break if ancestor.nil? || ancestor == @doc
+
+          if (ns = ancestor.namespace_definitions.find { |x| x.prefix == @ns_prefix })
+            @ns_prefix = nil
+            node.namespace = ns
+            break
+          end
+        end
+
+        return if @ns_prefix.nil?
+
+        raise ArgumentError, "Namespace prefix #{@ns_prefix.inspect} has not been defined"
+      end
+
       ###
       # Insert +node+ as a child of the current Node
       def insert(node, &block)

test/namespaces/test_serializing_namespaces.rb

@@ -47,11 +47,7 @@
     assert_includes(doc, '<dnd:adventure xmlns:dnd="http://www.w3.org/dungeons#">')
     assert_includes(doc, '<dnd:party xmlns:dnd="http://www.w3.org/dragons#">')
     assert_includes(doc, "<dnd:members>")
-    pending_if("https://github.com/sparklemotion/nokogiri/issues/3458", !Nokogiri.jruby?) do
-      # Here MRI Ruby is incorrectly omitting the xmlns namespace declaration, i.e.:
-      # '<dnd:character>'
-      assert_includes(doc, '<dnd:character xmlns:dnd="http://www.w3.org/dungeons#">')
-    end
+    assert_includes(doc, '<dnd:character xmlns:dnd="http://www.w3.org/dungeons#">')
     assert_includes(doc, "<dnd:name>Nigel</dnd:name>")
   end
 

test/xml/test_builder.rb

@@ -78,6 +78,30 @@ def test_builder_namespace_part_deux
         assert_equal({ "xmlns:a" => "x", "xmlns:c" => "z" }, namespaces_defined_on(b))
       end
 
+      def test_builder_namespaces_part_three
+        doc = Nokogiri::XML::Builder.new do |xml|
+          xml["dnd"].adventure("xmlns:dnd" => "http://www.w3.org/dungeons#") do
+            xml["dnd"].party("xmlns:dnd" => "http://www.w3.org/dragons#") do
+              xml["dnd"].character("xmlns:dnd" => "http://www.w3.org/dungeons#") do
+                xml["dnd"].name("Nigel", "xmlns:dnd" => "http://www.w3.org/dungeons#")
+              end
+            end
+          end
+        end.doc
+
+        adventure_node = doc.at_xpath("//*[local-name()='adventure']")
+        assert_equal("http://www.w3.org/dungeons#", adventure_node.namespace.href)
+
+        party_node = doc.at_xpath("//*[local-name()='party']")
+        assert_equal("http://www.w3.org/dragons#", party_node.namespace.href)
+
+        character_node = doc.at_xpath("//*[local-name()='character']")
+        assert_equal("http://www.w3.org/dungeons#", character_node.namespace.href)
+
+        name_node = doc.at_xpath("//*[local-name()='name']")
+        assert_equal("http://www.w3.org/dungeons#", name_node.namespace.href)
+      end
+
       def test_builder_with_unlink
         b = Nokogiri::XML::Builder.new do |xml|
           xml.foo do