Regression in JRuby Nokogiri add_previous_sibling (1.5.0 -> 1.5.1)

[aselder]

Given the following script

require 'nokogiri'
puts Nokogiri::VERSION
fragment = Nokogiri::HTML::DocumentFragment.parse("a<a> b </a>")
node = fragment.children[1]
puts node.inspect
node.children.each {|n| node.add_previous_sibling(n) }
puts node.inspect

In JRuby Nokogiri 1.5.0 you get the following output:

1.5.0
#<Nokogiri::XML::Element:0x7fe name="a" children=[#<Nokogiri::XML::Text:0x7fc " b ">]>
#<Nokogiri::XML::Element:0x7fe name="a"> 

In Jruby Nokogiri 1.5.1 (and higher, including 1.5.3), you get the following output:

1.5.1
#<Nokogiri::XML::Element:0x7fe name="a" children=[#<Nokogiri::XML::Text:0x7fc " b ">]>
#<Nokogiri::XML::Element:0x7fe name="a" children=[#<Nokogiri::XML::Text:0x7fc " b ">]>

In C Nokogiri 1.5.3, you get the following output:

1.5.3
#<Nokogiri::XML::Element:0x7fe name="a" children=[#<Nokogiri::XML::Text:0x7fc " b ">]>
#<Nokogiri::XML::Element:0x7fe name="a"> 

[aselder]

I discovered this regression while running into issues with the Sanitize gem: rgrove/sanitize#54

[aselder]

Looking at the commit log for XmlNode.java, this commit stands out at referencing the functionality and matching the timeframe: https://github.com/tenderlove/nokogiri/commit/47db28485948ec64f65e0ad05d17935587704250#ext/java/nokogiri/XmlNode.java

[yokolet]

@aselder thank you for finding the suspicious commit. I'll soon work on this.

[jvshahid]

@yokolet I believe I fixed the issue, but I'm seeing two test failing randomly (they fail very frequently) in TestPushPasser, I don't think they're related, but I'm not sure.

1) Error:
test_start_element_ns(Nokogiri::XML::SAX::TestPushParser):
NoMethodError: undefined method `length' for nil:NilClass
    ./test/xml/sax/test_push_parser.rb:75:in `test_start_element_ns'
    org/jruby/RubyKernel.java:2076:in `send'

and

 1) Failure:
test_start_element_with_namespaces(Nokogiri::XML::SAX::TestPushParser) [./test/xml/sax/test_push_parser.rb:58]:
--- expected
+++ actual
@@ -1 +1 @@
-[["p", [["xmlns:foo", "http://foo.example.com/"]]]]
+nil

[jvshahid]

I realized that the final version was very close to what was on master with minor changes, so I reverted my changes and started from scratch, I think the new commit has a much cleaner diff that the previous one.

[yokolet]

@jvshahid your fix looks ok. Would you send me pull request? I can merge your change, but pull request is cleaner to merge your change and your contribution explicitly.

[jvshahid]

I saw you merged the pull request. thanks.

[yokolet]

Thanks!

[flavorjones]

in 1.5.5.rc1

[yokolet]

Thanks!