Created attachment 287481 [details] [review]
Tests and patch

There is a bug here, but your patch does not completely fix it, and it
introduces another bug.

Assume that we start by parsing this file:

<?xml version="1.0" encoding="UTF-8"?>
<myns:root xmlns:myns="http://default.namespace/root">
  <myns:child/>
</myns:root>

Then execute:
  xmlpp::Element* child = root->add_child("child", "myns");
  child->set_namespace_declaration(nsmap["ns1"], "");

With your patch,
  child->set_namespace("");
would now be executed as part of the call to set_namespace_declaration(),
changing the namespace of child from myns to the default namespace.

Another problem is that in a larger XML document, child might have children.
If the added namespace declaration overrides a namespace declaration in
an ancestor node, some of the child's children should perhaps be affected.

This looks like a libxml2 bug. xmlNewNs() should fix this, don't you think?

You're absolutely right!

In fact I would really prefer API upgrade:

Currently when you create a node you can only provide its name and an already defined namespace prefix.

So if you want to create new node with new namespace you have to:
- create the node,
- set new namespace declaration (prefix + uri) to the new node,
- set appropriate namespace prefix to the node.

I think this is really bad.
Libxml++ should provide a better interface for libxml2.
I suggest to add a method allowing to do the job in one call, for example:

add_child(const Glib::ustring& node_name, const Glib:ustring& namespace_uri, const Glib::ustring& namespace_prefix)

With such method, you can add any new node in any new or existing namespace with any namespace prefix.

Probably, it would also be interesting to add method allowing to create new node using current context (ie same namespace as its parent).

Regarding my patch, I think it would be easy to check whether current namespace prefix is empty or not. If so, child->set_namespace("") could be called safely...

This sounds good for node creation but not for node update. But this is also the case for namespace with prefix declared:

- Let's suppose following XML file is parsed:

<?xml version="1.0" encoding="UTF-8"?>
<ns0:root xmlns:ns0="http://default.namespace/root">
  <ns0:child>
    <ns0:sub_child/>
  </ns0:child>
</ns0:root>

- If, child node is retrieved and its namespace is modified:

child->set_namespace_declaration("http://other.namespace", "ns0")

- Then following XML can be dumped:

<?xml version="1.0" encoding="UTF-8"?>
<ns0:root xmlns:ns0="http://default.namespace/root">
  <ns0:child xmlns:ns0="http://other.namespace">
    <ns0:sub_child/>
  </ns0:child>
</ns0:root>

- So by modifying namespace of the child node, namespace of the sub-child is also modified. Well, in fact you need to dump it and parse it again. In fact, sub_child node namespace is still "http://default.namespace/root".
I suppose this can be considered as a bug in libxml2.

I upload an updated version of my patch with new tests.

This patch doesn't solve problem for sub-nodes which can be impacted by namespace modification on one of its ancestor.

Created attachment 287735 [details] [review]
Second patch + tests

(In reply to comment #3)
> Libxml++ should provide a better interface for libxml2.
> I suggest to add a method allowing to do the job in one call, for example:
>
> add_child(const Glib::ustring& node_name, const Glib:ustring& namespace_uri,
> const Glib::ustring& namespace_prefix)

That's a good idea. I can make a patch. Or would you like to do it?

> Probably, it would also be interesting to add method allowing to create new
> node using current context (ie same namespace as its parent).

That's reasonably easy to do with the existing add_child().

  parent->add_child("child_a", parent->get_namespace_prefix());

Or, if you add many children:

  Glib::ustring parent_ns = parent->get_namespace_prefix();
  parent->add_child("child_a", parent_ns);
  parent->add_child("child_b", parent_ns);
  ...

Is it really worth 3 more methods to get rid of the parent_ns parameter?
(There are two add_child() and one add_child_before().)

(In reply to comment #6)
> Second patch + tests

I'm sure this patch works as intended, but I hesitate to push it.
It looks like a work-around for a bug in xmlNewNs(). I can't find an existing
relevant libxml2 bug. We should file a libxml2 bug, and see if we get a
reaction.

I have filed libxml2 bug 738112.

Created attachment 288115 [details] [review]
patch: Element::set_namespace_declaration(): Update associated node

xmlNodeNs() will probably not be changed. I'll add your patch, but I want to
change it as shown in the attached patch.

Created attachment 288116 [details] [review]
patch: Add dom_update_namespace example

I'd also like to change your test case.
I've removed much of the emphasis on default namespaces. It can be used for
testing other namespaces too, after more add_child() functions have been added.

I have pushed the patches in comments 9 and 10 with minor changes.
I have also pushed a patch that adds the methods

  Element* Node::add_child_with_new_ns(const Glib::ustring& name,
    const Glib::ustring& ns_uri,
    const Glib::ustring& ns_prefix = Glib::ustring());
  Element* Node::add_child_with_new_ns(xmlpp::Node* previous_sibling,
    const Glib::ustring& name,
    const Glib::ustring& ns_uri,
    const Glib::ustring& ns_prefix = Glib::ustring());
  Element* Node::add_child_before_with_new_ns(xmlpp::Node* next_sibling,
    const Glib::ustring& name,
    const Glib::ustring& ns_uri,
    const Glib::ustring& ns_prefix = Glib::ustring());

and a patch where the first one of these is called in the dom_update_namespace
example program.