From 64f099b60d8138614d09f03f55a968f5c904a0bf Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 28 Aug 2021 15:15:07 -0400 Subject: [PATCH] feat/fix: introduce Document#namespace_inheritance attr When true, reparented elements without a namespace will inherit their new parent's namespace (if one exists). Defaults to +false+. This is part of addressing the behavior change introduced in v1.12 by 1f483f0, allowing us to switch it on or off per-document. See #2317. --- ext/java/nokogiri/XmlDocumentFragment.java | 12 --------- ext/java/nokogiri/XmlNode.java | 23 +++++++--------- ext/nokogiri/xml_node.c | 7 +++++ lib/nokogiri/xml/document.rb | 9 +++++++ test/xml/test_document.rb | 7 +++++ test/xml/test_node_reparenting.rb | 31 ++++++++++++++++++++++ 6 files changed, 63 insertions(+), 26 deletions(-) diff --git a/ext/java/nokogiri/XmlDocumentFragment.java b/ext/java/nokogiri/XmlDocumentFragment.java index 17c1be9081..7d8572605f 100644 --- a/ext/java/nokogiri/XmlDocumentFragment.java +++ b/ext/java/nokogiri/XmlDocumentFragment.java @@ -34,8 +34,6 @@ public class XmlDocumentFragment extends XmlNode { - private XmlElement fragmentContext; - public XmlDocumentFragment(Ruby ruby) { @@ -75,10 +73,6 @@ public class XmlDocumentFragment extends XmlNode fragment.setDocument(context, doc); fragment.setNode(context.runtime, doc.getDocument().createDocumentFragment()); - //TODO: Get namespace definitions from doc. - if (args.length == 3 && args[2] != null && args[2] instanceof XmlElement) { - fragment.fragmentContext = (XmlElement)args[2]; - } Helpers.invoke(context, fragment, "initialize", args); return fragment; } @@ -158,12 +152,6 @@ public class XmlDocumentFragment extends XmlNode return null; } - public XmlElement - getFragmentContext() - { - return fragmentContext; - } - @Override public void relink_namespace(ThreadContext context) diff --git a/ext/java/nokogiri/XmlNode.java b/ext/java/nokogiri/XmlNode.java index d1ced80f74..9475e4c561 100644 --- a/ext/java/nokogiri/XmlNode.java +++ b/ext/java/nokogiri/XmlNode.java @@ -12,6 +12,7 @@ import org.apache.xerces.dom.CoreDocumentImpl; import org.jruby.Ruby; import org.jruby.RubyArray; +import org.jruby.RubyBoolean; import org.jruby.RubyClass; import org.jruby.RubyFixnum; import org.jruby.RubyInteger; @@ -421,7 +422,14 @@ public class XmlNode extends RubyObject String nsURI = e.lookupNamespaceURI(prefix); this.node = NokogiriHelpers.renameNode(e, nsURI, e.getNodeName()); - if (nsURI == null || nsURI.isEmpty()) { return; } + if (nsURI == null || nsURI.isEmpty()) { + RubyBoolean ns_inherit = + (RubyBoolean)document(context.runtime).getInstanceVariable("@namespace_inheritance"); + if (ns_inherit.isTrue()) { + set_namespace(context, ((XmlNode)parent(context)).namespace(context)); + } + return; + } String currentPrefix = e.getParentNode().lookupPrefix(nsURI); String currentURI = e.getParentNode().lookupNamespaceURI(prefix); @@ -1743,24 +1751,11 @@ protected enum AdoptScheme { e.appendChild(otherNode); otherNode = e; } else { - addNamespaceURIIfNeeded(otherNode); parent.appendChild(otherNode); } return new Node[] { otherNode }; } - private void - addNamespaceURIIfNeeded(Node child) - { - if (this instanceof XmlDocumentFragment && ((XmlDocumentFragment) this).getFragmentContext() != null) { - XmlElement fragmentContext = ((XmlDocumentFragment) this).getFragmentContext(); - String namespace_uri = fragmentContext.node.getNamespaceURI(); - if (namespace_uri != null && namespace_uri.length() > 0) { - NokogiriHelpers.renameNode(child, namespace_uri, child.getNodeName()); - } - } - } - protected void adoptAsPrevSibling(ThreadContext context, Node parent, diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c index c28fb751a9..4e8533ed9a 100644 --- a/ext/nokogiri/xml_node.c +++ b/ext/nokogiri/xml_node.c @@ -69,6 +69,13 @@ relink_namespace(xmlNodePtr reparented) /* Avoid segv when relinking against unlinked nodes. */ if (reparented->type != XML_ELEMENT_NODE || !reparented->parent) { return; } + /* Make sure that our reparented node has the correct namespaces */ + if (!reparented->ns && + (reparented->doc != (xmlDocPtr)reparented->parent) && + (rb_iv_get(DOC_RUBY_OBJECT(reparented->doc), "@namespace_inheritance") == Qtrue)) { + xmlSetNs(reparented, reparented->parent->ns); + } + /* Search our parents for an existing definition */ if (reparented->nsDef) { xmlNsPtr curr = reparented->nsDef; diff --git a/lib/nokogiri/xml/document.rb b/lib/nokogiri/xml/document.rb index d09c0e41ad..d3410cf1b7 100644 --- a/lib/nokogiri/xml/document.rb +++ b/lib/nokogiri/xml/document.rb @@ -113,9 +113,18 @@ def self.parse string_or_io, url = nil, encoding = nil, options = ParseOptions:: # A list of Nokogiri::XML::SyntaxError found when parsing a document attr_accessor :errors + # @!attribute rw namespace_inheritance + # + # When true, reparented elements without a namespace will inherit their new parent's + # namespace (if one exists). Defaults to +false+. + # + # @return [Boolean] Whether + attr_accessor :namespace_inheritance + def initialize *args # :nodoc: @errors = [] @decorators = nil + @namespace_inheritance = false end ## diff --git a/test/xml/test_document.rb b/test/xml/test_document.rb index e56ffba913..48eed1d10e 100644 --- a/test/xml/test_document.rb +++ b/test/xml/test_document.rb @@ -15,6 +15,13 @@ class TestDocument < Nokogiri::TestCase let(:xml) { Nokogiri::XML.parse(File.read(XML_FILE), XML_FILE) } + def test_default_namespace_inheritance + doc = Nokogiri::XML::Document.new + refute(doc.namespace_inheritance) + doc.namespace_inheritance = true + assert(doc.namespace_inheritance) + end + def test_dtd_with_empty_internal_subset doc = Nokogiri::XML(<<~eoxml) diff --git a/test/xml/test_node_reparenting.rb b/test/xml/test_node_reparenting.rb index 77fc1ebe42..efcd661cd3 100644 --- a/test/xml/test_node_reparenting.rb +++ b/test/xml/test_node_reparenting.rb @@ -557,6 +557,37 @@ def coerce(data) end end end + + describe "given a parent node with a non-default namespace" do + let(:doc) do + Nokogiri::XML(<<~EOF) + + + + + EOF + end + let(:parent) { doc.at_xpath("//foo:parent", "foo" => "http://nokogiri.org/default_ns/test/foo") } + + describe "and namespace_inheritance is off" do + it "inserts a child node that does not inherit the parent's namespace" do + refute(doc.namespace_inheritance) + child = parent.add_child("").first + puts doc.to_xml + puts child.namespace.inspect + assert_nil(child.namespace) + end + end + + describe "and namespace_inheritance is on" do + it "inserts a child node that inherits the parent's namespace" do + doc.namespace_inheritance = true + child = parent.add_child("").first + assert_not_nil(child.namespace) + assert_equal("http://nokogiri.org/default_ns/test/foo", child.namespace.href) + end + end + end end describe "#add_previous_sibling" do