Skip to content

Commit

Permalink
fix(jruby): XML::DocumentFragment.dup to another document
Browse files Browse the repository at this point in the history
Back in b92660e (#1834 fixing #1063) I omitted support in JRuby for
the "new_parent_document" argument to `Node#dup` because there was no
performance reason to implement it. So the test was skipped.

However, in 1e7d38a and other commits in #3117 (fixing #316), I
introduced a call to `initialize_copy_with_args` that passes the new
parent document as an argument on both CRuby and JRuby
implementations. Because the test was skipped, I didn't catch that
this broke on JRuby.

In particular this was a problem for Loofah which relies on
decorators, and even more particularly this broke the
`Loofah::TextBehavior` formatting concern for
`Loofah::*::DocumentFragment` objects.

Maybe we should be running downstream tests with JRuby, too? But that
feels like a big investment right now so I'll avoid scarring on the
first cut, and wait to see if it happens again.

(cherry picked from commit dda0be2)
  • Loading branch information
flavorjones committed Dec 12, 2024
1 parent e4bae8a commit 8bd6c6d
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 3 deletions.
3 changes: 2 additions & 1 deletion ext/java/nokogiri/XmlNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -978,10 +978,11 @@ public class XmlNode extends RubyObject

@JRubyMethod(visibility = Visibility.PROTECTED)
public IRubyObject
initialize_copy_with_args(ThreadContext context, IRubyObject other, IRubyObject level, IRubyObject _ignored)
initialize_copy_with_args(ThreadContext context, IRubyObject other, IRubyObject level, IRubyObject document)
{
boolean deep = level instanceof RubyInteger && RubyFixnum.fix2int(level) != 0;
this.node = asXmlNode(context, other).node.cloneNode(deep);
setDocument(context, (XmlDocument)document);
return this;
}

Expand Down
2 changes: 0 additions & 2 deletions test/xml/test_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ def test_dup_same_parent_document_is_default
end

def test_dup_different_parent_document
skip_unless_libxml2("Node.dup with new_parent arg is only implemented on CRuby")

doc1 = XML::Document.parse("<root><div><p>hello</p></div></root>")
doc2 = XML::Document.parse("<div></div>")

Expand Down

0 comments on commit 8bd6c6d

Please sign in to comment.