From 3b1bc8eea076c44455802b2cb11d19e0958d9004 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 3 Jul 2023 15:14:44 -0400 Subject: [PATCH] fix: do not dup text siblings in reparent_node_with(xmlAddChild) Related to #283 and #595 Closes #2916 --- CHANGELOG.md | 1 + ext/nokogiri/xml_node.c | 2 +- test/xml/test_node_reparenting.rb | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b63c5d1acf1..8694378b342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA ### Fixed +* [CRuby] Replacing a node's children via methods like `Node#inner_html=`, `#children=`, and `#replace` no longer defensively dups the node's next sibling if it is a Text node. This behavior was originally adopted to work around libxml2's memory management (see [#283](https://github.com/sparklemotion/nokogiri/issues/283) and [#595](https://github.com/sparklemotion/nokogiri/issues/595)) but should not have included operations involving `xmlAddChild()`. [[#2916](https://github.com/sparklemotion/nokogiri/issues/2916)] * [JRuby] Fixed NPE when serializing an unparented HTML node. [[#2559](https://github.com/sparklemotion/nokogiri/issues/2559), [#2895](https://github.com/sparklemotion/nokogiri/issues/2895)] (Thanks, [@cbasguti](https://github.com/cbasguti)!) diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c index 964964814e4..47ca711fccc 100644 --- a/ext/nokogiri/xml_node.c +++ b/ext/nokogiri/xml_node.c @@ -350,7 +350,7 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func xmlUnlinkNode(original_reparentee); - if (prf != xmlAddPrevSibling && prf != xmlAddNextSibling + if (prf != xmlAddPrevSibling && prf != xmlAddNextSibling && prf != xmlAddChild && reparentee->type == XML_TEXT_NODE && pivot->next && pivot->next->type == XML_TEXT_NODE) { /* * libxml merges text nodes in a right-to-left fashion, meaning that if diff --git a/test/xml/test_node_reparenting.rb b/test/xml/test_node_reparenting.rb index 50df891af45..a3828e5186f 100644 --- a/test/xml/test_node_reparenting.rb +++ b/test/xml/test_node_reparenting.rb @@ -673,7 +673,9 @@ def coerce(data) it "should not merge text nodes during the operation" do xml = Nokogiri::XML(%(text node)) replacee = xml.root.children.first + replacee.replace("new text node") + assert_equal "new text node", xml.root.children.first.content end end @@ -682,7 +684,9 @@ def coerce(data) doc = Nokogiri::XML(%{text}) replacee = doc.at_css("child") replacer = doc.create_comment("text") + replacee.replace(replacer) + assert_equal 1, doc.root.children.length assert_equal replacer, doc.root.children.first end @@ -691,11 +695,26 @@ def coerce(data) doc = Nokogiri::XML(%{text}) replacee = doc.at_css("child") replacer = doc.create_cdata("text") + replacee.replace(replacer) + assert_equal 1, doc.root.children.length assert_equal replacer, doc.root.children.first end + it "replacing a child should not dup sibling text nodes" do + # https://github.com/sparklemotion/nokogiri/issues/2916 + xml = "asdfqwer" + doc = Nokogiri::XML.parse(xml) + nodes = doc.root.children + parent = nodes.first + sibling = parent.next + + parent.inner_html = "foo" + + assert_same(sibling, parent.next) + end + describe "when a document has a default namespace" do before do @fruits = Nokogiri::XML(<<~eoxml)