Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bug] JRuby: reparenting a node does not apply the parent's default namespace #3457

Open
johnnyshields opened this issue Mar 9, 2025 · 1 comment · May be fixed by #3463
Open

[bug] JRuby: reparenting a node does not apply the parent's default namespace #3457

johnnyshields opened this issue Mar 9, 2025 · 1 comment · May be fixed by #3463
Labels
platform/jruby topic/namespaces

Comments

@johnnyshields
Copy link
Contributor

johnnyshields commented Mar 9, 2025 •
edited
Loading

The JRuby parser has a bug where default namespaces on child elements are reverted to the root node's default namesapce, rather than the default namespace of their immediate parent.

Unlike #3455 which was a "syntax preference", this is a bona-fide "bug". The JRuby-parsed and out XML is semantically different than the MRI-parsed one, in a way that can break consumers.

    it 'handles nested default namespaces' do
      doc = Nokogiri::XML::Builder.new do |xml|
        xml.root(xmlns: 'http://outer-namespace.org/') do
          xml.outer('in outer namespace')
          xml.inner(xmlns: 'http://inner-namespace.org/') do
            xml.element('in inner namespace')
          end
          xml.another('back in outer namespace')
        end
      end.doc.to_xml(save_with: Nokogiri::XML::Node::SaveOptions::AS_XML)

      assert_includes(doc, '<root xmlns="http://outer-namespace.org/">')
      assert_includes(doc, '<outer>in outer namespace</outer>')
      assert_includes(doc, '<inner xmlns="http://inner-namespace.org/">')
      # TODO: THE NEXT LINE FAILS ON JRUBY!!
      assert_includes(doc, '<element>in inner namespace</element>')
      # TODO: THE NEXT LINE PASSES ON JRUBY, BUT SHOULD FAIL!!
      # assert_includes(doc, '<element xmlns="http://outer-namespace.org/">in inner namespace</element>')
      assert_includes(doc, '<another>back in outer namespace</another>')
    end

This PR includes test cases which cover this issue: #3456. They are made to pass for now using if Nokogiri.jruby?
@johnnyshields johnnyshields added the state/needs-triage Inbox for non-installation-related bug reports or help requests label Mar 9, 2025
@johnnyshields johnnyshields mentioned this issue Mar 9, 2025
Merged
@flavorjones
Copy link
Member

I alluded to this in my review of #3456 but if we're not testing Builder behavior, I'd prefer to write this test as a simple parse of an input string:

    it 'handles nested default namespaces' do
      doc = Nokogiri::XML::Document.parse(<<~XML).to_xml(save_with: Nokogiri::XML::Node::SaveOptions::AS_XML)
        <root xmlns="http://outer-namespace.org/">
          <outer>in outer namespace</outer>
          <inner xmlns="http://inner-namespace.org/">
            <element>in inner namespace</element>
          </inner>
          <another>back in outer namespace</another>
        </root>
      XML

      assert_includes(doc, '<root xmlns="http://outer-namespace.org/">')
      assert_includes(doc, '<outer>in outer namespace</outer>')
      assert_includes(doc, '<inner xmlns="http://inner-namespace.org/">')
      # TODO: THE NEXT LINE FAILS ON JRUBY!!
      assert_includes(doc, '<element>in inner namespace</element>')
      # TODO: THE NEXT LINE PASSES ON JRUBY, BUT SHOULD FAIL!!
      # assert_includes(doc, '<element xmlns="http://outer-namespace.org/">in inner namespace</element>')
      assert_includes(doc, '<another>back in outer namespace</another>')
    end

but when I do this, the test passes on both CRuby and JRuby. My point here is that writing the test as simply as possible helps understand where the bug is originating.

In this case, it seems to be a bug in node reparenting (which is how Builder works under the hood), and not a parser bug or a serialization bug.

The most descriptive test I could write for this particular reparenting bug is:

    it "foo" do
      doc = Nokogiri::XML(<<~XML)
        <root xmlns='http://outer-namespace.org'>
          <inner xmlns='http://inner-namespace.org'/>
        </root>
      XML
      
      node = doc.create_element("element")
      doc.root.elements.first.add_child(node)

      assert_equal("http://inner-namespace.org", node.namespace.href)
    end

which passes on CRuby and fails on JRuby.

@flavorjones flavorjones changed the title [bug] JRuby default namespace inheritance bug [bug] JRuby: reparenting a node does not apply the parent's default namespace Mar 9, 2025
@flavorjones flavorjones added platform/jruby topic/namespaces and removed state/needs-triage Inbox for non-installation-related bug reports or help requests labels Mar 9, 2025
flavorjones added a commit that referenced this issue Mar 10, 2025
@flavorjones
fix(jruby): create_element should not set the doc's default ns
8a4033e 
The ns should be set explicitly or when the node is parented.

Fixes #3457
@flavorjones flavorjones linked a pull request Mar 10, 2025 that will close this issue
Open
flavorjones added a commit that referenced this issue Mar 10, 2025
@flavorjones
fix(jruby): create_element should not set the doc's default ns
ad5960d 
The ns should be set explicitly or when the node is parented.

Fixes #3457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
platform/jruby topic/namespaces
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(jruby): create_element should not set the doc's default ns sparklemotion/nokogiri
2 participants
@flavorjones @johnnyshields