Skip to content

Commit

Permalink
Merge branch 'over-eager-namespace-pruning-when-reparenting'
Browse files Browse the repository at this point in the history
  • Loading branch information
flavorjones committed May 30, 2016
2 parents 334aec6 + 26c5eb0 commit 8e305e4
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 18 deletions.
19 changes: 10 additions & 9 deletions ext/nokogiri/xml_node.c
Expand Up @@ -35,13 +35,13 @@ static void relink_namespace(xmlNodePtr reparented)
xmlNsPtr ns;

if (reparented->type != XML_ATTRIBUTE_NODE &&
reparented->type != XML_ELEMENT_NODE) return;
reparented->type != XML_ELEMENT_NODE) { return; }

if (reparented->ns == NULL || reparented->ns->prefix == NULL) {
name = xmlSplitQName2(reparented->name, &prefix);

if(reparented->type == XML_ATTRIBUTE_NODE) {
if (prefix == NULL || strcmp((char*)prefix, XMLNS_PREFIX) == 0) return;
if (prefix == NULL || strcmp((char*)prefix, XMLNS_PREFIX) == 0) { return; }
}

ns = xmlSearchNs(reparented->doc, reparented, prefix);
Expand All @@ -57,26 +57,27 @@ static void relink_namespace(xmlNodePtr reparented)
}

/* Avoid segv when relinking against unlinked nodes. */
if (reparented->type != XML_ELEMENT_NODE || !reparented->parent) return;
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)
if (!reparented->ns && reparented->doc != (xmlDocPtr)reparented->parent) {
xmlSetNs(reparented, reparented->parent->ns);
}

/* Search our parents for an existing definition */
if(reparented->nsDef) {
if (reparented->nsDef) {
xmlNsPtr curr = reparented->nsDef;
xmlNsPtr prev = NULL;

while(curr) {
while (curr) {
xmlNsPtr ns = xmlSearchNsByHref(
reparented->doc,
reparented->parent,
curr->href
);
/* If we find the namespace is already declared, remove it from this
* definition list. */
if(ns && ns != curr) {
if (ns && ns != curr && xmlStrEqual(ns->prefix, curr->prefix)) {
if (prev) {
prev->next = curr->next;
} else {
Expand All @@ -92,12 +93,12 @@ static void relink_namespace(xmlNodePtr reparented)

/* Only walk all children if there actually is a namespace we need to */
/* reparent. */
if(NULL == reparented->ns) return;
if (NULL == reparented->ns) { return; }

/* When a node gets reparented, walk it's children to make sure that */
/* their namespaces are reparented as well. */
child = reparented->children;
while(NULL != child) {
while (NULL != child) {
relink_namespace(child);
child = child->next;
}
Expand Down
156 changes: 147 additions & 9 deletions test/xml/test_node_reparenting.rb
Expand Up @@ -215,26 +215,164 @@ class TestNodeReparenting < Nokogiri::TestCase
end
end

describe "given a parent node with a non-default namespace" do
describe "given a parent node with a default and non-default namespace" do
before do
@doc = Nokogiri::XML(<<-eoxml)
<root xmlns="http://tenderlovemaking.com/" xmlns:foo="http://flavorjon.es/">
<first>
</first>
</root>
eoxml
assert @node = @doc.at('//xmlns:first')
@child = Nokogiri::XML::Node.new('second', @doc)
end

describe "and a child node with a namespace matching the parent's non-default namespace" do
it "inserts a node that inherits the matching parent namespace" do
assert node = @doc.at('//xmlns:first')
child = Nokogiri::XML::Node.new('second', @doc)
describe "and a child with a namespace matching the parent's default namespace" do
describe "and as the default prefix" do
before do
@ns = @child.add_namespace(nil, 'http://tenderlovemaking.com/')
@child.namespace = @ns
end

it "inserts a node that inherits the parent's default namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://tenderlovemaking.com/")
assert reparented.namespace_definitions.empty?
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" => "http://tenderlovemaking.com/",
"xmlns:foo" => "http://flavorjon.es/",
},
reparented.namespaces)
end
end

describe "but with a different prefix" do
before do
@ns = @child.add_namespace("baz", 'http://tenderlovemaking.com/')
@child.namespace = @ns
end

it "inserts a node that uses its own namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://tenderlovemaking.com/")
assert reparented.namespace_definitions.include?(@ns)
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" => "http://tenderlovemaking.com/",
"xmlns:foo" => "http://flavorjon.es/",
"xmlns:baz" => "http://tenderlovemaking.com/",
},
reparented.namespaces)
end
end
end

describe "and a child with a namespace matching the parent's non-default namespace" do
describe "set by #namespace=" do
before do
@ns = @doc.root.namespace_definitions.detect { |x| x.prefix == "foo" }
@child.namespace = @ns
end

it "inserts a node that inherits the matching parent namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://flavorjon.es/")
assert reparented.namespace_definitions.empty?
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" => "http://tenderlovemaking.com/",
"xmlns:foo" => "http://flavorjon.es/",
},
reparented.namespaces)
end
end

describe "with the same prefix" do
before do
@ns = @child.add_namespace("foo", 'http://flavorjon.es/')
@child.namespace = @ns
end

it "inserts a node that uses the parent's namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://flavorjon.es/")
assert reparented.namespace_definitions.empty?
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" => "http://tenderlovemaking.com/",
"xmlns:foo" => "http://flavorjon.es/",
},
reparented.namespaces)
end
end

describe "as the default prefix" do
before do
@ns = @child.add_namespace(nil, 'http://flavorjon.es/')
@child.namespace = @ns
end

it "inserts a node that keeps its namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://flavorjon.es/")
assert reparented.namespace_definitions.include?(@ns)
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" => "http://flavorjon.es/",
"xmlns:foo" => "http://flavorjon.es/",
},
reparented.namespaces)
end
end

describe "but with a different prefix" do
before do
@ns = @child.add_namespace('baz', 'http://flavorjon.es/')
@child.namespace = @ns
end

it "inserts a node that keeps its namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://flavorjon.es/")
assert reparented.namespace_definitions.include?(@ns)
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" =>"http://tenderlovemaking.com/",
"xmlns:foo" =>"http://flavorjon.es/",
"xmlns:baz" =>"http://flavorjon.es/",
},
reparented.namespaces)
end
end
end

ns = @doc.root.namespace_definitions.detect { |x| x.prefix == "foo" }
child.namespace = ns
describe "and a child node with a default namespace not matching the parent's default namespace and a namespace matching a parent namespace but with a different prefix" do
before do
@ns = @child.add_namespace(nil, 'http://example.org/')
@child.namespace = @ns
@ns2 = @child.add_namespace('baz', 'http://tenderlovemaking.com/')
end

node.add_child(child)
assert @doc.at('//foo:second', "foo" => "http://flavorjon.es/")
it "inserts a node that keeps its namespace" do
@node.add_child(@child)
assert reparented = @doc.at('//bar:second', "bar" => "http://example.org/")
assert reparented.namespace_definitions.include?(@ns)
assert reparented.namespace_definitions.include?(@ns2)
assert_equal @ns, reparented.namespace
assert_equal(
{
"xmlns" => "http://example.org/",
"xmlns:foo" => "http://flavorjon.es/",
"xmlns:baz" => "http://tenderlovemaking.com/",
},
reparented.namespaces)
end
end
end
Expand Down

0 comments on commit 8e305e4

Please sign in to comment.