> I don't know enough about XML/XPATH to know if this is a good idea or not, >
Actually currently because of the namespace problem, xpath() returns wrong result (even worse: invalid xml!). So this patch corrects the behavior of xpath() to the correct one. > but I did go read the documentation of xmlCopyNode(), and I notice it says > the second argument is > > extended: if 1 do a recursive copy (properties, namespaces and children > when applicable) if 2 copy properties and namespaces (when applicable) > > Do we really want it to "copy children"? Perhaps the value should be 2? > (I have no idea, I'm just raising the question.) > If we put 1 as the parameter, the resulting Node will link to the existing children. In this case, the namespace problem for the children might be still exists. If any of the children uses different namespace(s) than the parent, the namespace definition will not be copied correctly. Just to be safe, in the patch 1 passed as the second argument. Ideally, we can traverse the Node object and generate XML accordingly, but it is a lot of work, and a lot of duplicating libxml's code. Maybe we should push this upstream to libxml? I also notice that it says > > Returns: a new #xmlNodePtr, or NULL in case of error. > > and the patch is failing to cover the error case. Most likely, that > would represent out-of-memory, so it definitely seems to be something > we should account for. > Ah, overlooked that. Skimming through libxml2's source, it looks like there are some other cases beside out-of-memory. Will update the patch according to these cases. Thanks for the review. -- Ali Akbar
