Skip to content

Commit

Permalink
add test coverage for unlinking orphan nodes
Browse files Browse the repository at this point in the history
fixes #1112
closes #1152
  • Loading branch information
flavorjones committed Feb 17, 2016
1 parent 0aeae1b commit 5317a9b
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rdoc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Several changes were made to improve performance:
* [JRuby] fix NPE when inspecting nodes returned by NodeSet#drop (#1042) (Thanks, @mkristian!)
* [JRuby] fix nil attriubte node's namespace in reader (#1327) (Thanks, @codekitchen!)
* [JRuby] fix Nokogiri munging unicode characters that require more than 2 bytes (#1113) (Thanks, @mkristian!)
* [JRuby] allow unlinking an unparented node (#1112, #1152) (Thanks, @esse!)
* [MRI] fix assertion failure while accessing attribute node's namespace in reader (#843) (Thanks, @2potatocakes!)
* [MRI] fix issue with GCing namespace nodes returned in an xpath query. (#1155)
* [MRI] Ensure C strings are null-terminated. (#1381)
Expand Down
8 changes: 1 addition & 7 deletions ext/java/nokogiri/XmlNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -1413,16 +1413,10 @@ public IRubyObject set_namespace(ThreadContext context, IRubyObject namespace) {

@JRubyMethod(name = {"unlink", "remove"})
public IRubyObject unlink(ThreadContext context) {
if(node.getParentNode() == null) {
//throw context.getRuntime().newRuntimeError("TYPE: " + node.getNodeType()+ " PARENT NULL");
return this;
// if node doesn't have parent node - simply return self, as would Nokogiri for C would do
// because that means that node is already detached
} else {
if (node.getParentNode() != null) {
clearXpathContext(node.getParentNode());
node.getParentNode().removeChild(node);
}

return this;
}

Expand Down
3 changes: 2 additions & 1 deletion test/html/test_document_fragment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ def test_html_parse_encoding
end

def test_unlink_empty_document
Nokogiri::HTML::DocumentFragment.parse('').unlink
frag = Nokogiri::HTML::DocumentFragment.parse('').unlink # must_not_raise
assert_nil frag.parent
end

def test_colons_are_not_removed
Expand Down
13 changes: 13 additions & 0 deletions test/xml/test_unparented_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,19 @@ def test_illegal_replace_of_node_with_doc
old_node = @node.at('.//employee')
assert_raises(ArgumentError){ old_node.replace new_node }
end

def test_unlink_on_unlinked_node_1
node = Nokogiri::XML::Node.new 'div', Nokogiri::XML::Document.new
node.unlink # must_not_raise
assert_nil node.parent
end

def test_unlink_on_unlinked_node_2
node = Nokogiri::XML('<div>foo</div>').at_css("div")
node.unlink
node.unlink # must_not_raise
assert_nil node.parent
end
end
end
end

0 comments on commit 5317a9b

Please sign in to comment.