xmlfix3: unoxml: eradicate the bizarre concept of "carrier nodes":

add namespace data member to CAttr.
CAttr overrides getPrefix(), setPrefix(), getNamespaceURI().
CDocument::createAttributeNS() uses this new CAttr member instead of creating
a dummy carrier element.
CElement::setAttributeNode_Impl_Lock() and CNode::appendChild() do not free
the no longer existing dummy carrier element.
CNode::insertBefore() calls appendChild() for attributes
instead of ignoring namespace.
CNode::appendChild() does not invalidate attributes, because they are copied.

5 files changed, 136 insertions, 110 deletions

diff --git a/unoxml/source/dom/attr.cxx b/unoxml/source/dom/attr.cxx
index d11b65094ef1..bb73e20c7402 100644
--- a/unoxml/source/dom/attr.cxx
+++ b/unoxml/source/dom/attr.cxx
@@ -47,6 +47,29 @@ namespace DOM
     {
     }
 
+    xmlNsPtr CAttr::GetNamespace(xmlNodePtr const pNode)
+    {
+        if (!m_pNamespace.get()) {
+            return 0;
+        }
+        xmlChar const*const pUri(reinterpret_cast<xmlChar const*>(
+                m_pNamespace->first.getStr()));
+        xmlChar const*const pPrefix(reinterpret_cast<xmlChar const*>(
+                m_pNamespace->second.getStr()));
+        xmlNsPtr pNs = xmlSearchNs(pNode->doc, pNode, pPrefix);
+        if (pNs && (0 != xmlStrcmp(pNs->href, pUri))) {
+            return pNs;
+        }
+        pNs = xmlNewNs(pNode, pUri, pPrefix);
+        if (pNs) {
+            return pNs;
+        }
+        pNs = xmlSearchNsByHref(pNode->doc, pNode, pUri);
+        // if (!pNs) hmm... now what? throw?
+        if (!pNs) { OSL_TRACE("CAtttr: cannot create namespace"); }
+        return pNs;
+    }
+
     OUString SAL_CALL CAttr::getNodeName()
         throw (RuntimeException)
     {
@@ -183,4 +206,53 @@ namespace DOM
         dispatchSubtreeModified();
     }
 
+    void SAL_CALL CAttr::setPrefix(const OUString& prefix)
+        throw (RuntimeException, DOMException)
+    {
+        ::osl::MutexGuard const g(m_rMutex);
+
+        if (!m_aNodePtr) { return; }
+
+        if (m_pNamespace.get()) {
+            OSL_ASSERT(!m_aNodePtr->parent);
+            m_pNamespace->second =
+                OUStringToOString(prefix, RTL_TEXTENCODING_UTF8);
+        } else {
+            CNode::setPrefix(prefix);
+        }
+    }
+
+    OUString SAL_CALL CAttr::getPrefix()
+        throw (RuntimeException)
+    {
+        ::osl::MutexGuard const g(m_rMutex);
+
+        if (!m_aNodePtr) { return ::rtl::OUString(); }
+
+        if (m_pNamespace.get()) {
+            OSL_ASSERT(!m_aNodePtr->parent);
+            OUString const ret(::rtl::OStringToOUString(
+                        m_pNamespace->second, RTL_TEXTENCODING_UTF8));
+            return ret;
+        } else {
+            return CNode::getPrefix();
+        }
+    }
+
+    OUString SAL_CALL CAttr::getNamespaceURI()
+        throw (RuntimeException)
+    {
+        ::osl::MutexGuard const g(m_rMutex);
+
+        if (!m_aNodePtr) { return ::rtl::OUString(); }
+
+        if (m_pNamespace.get()) {
+            OSL_ASSERT(!m_aNodePtr->parent);
+            OUString const ret(::rtl::OStringToOUString(
+                        m_pNamespace->first, RTL_TEXTENCODING_UTF8));
+            return ret;
+        } else {
+            return CNode::getNamespaceURI();
+        }
+    }
 }
diff --git a/unoxml/source/dom/attr.hxx b/unoxml/source/dom/attr.hxx
index 609a97a35c49..6d9f6758c2c1 100644
--- a/unoxml/source/dom/attr.hxx
+++ b/unoxml/source/dom/attr.hxx
@@ -28,6 +28,8 @@
 #ifndef DOM_ATTR_HXX
 #define DOM_ATTR_HXX
 
+#include <memory>
+
 #include <libxml/tree.h>
 
 #include <cppuhelper/implbase1.hxx>
@@ -44,6 +46,8 @@ using namespace com::sun::star::xml::dom;
 
 namespace DOM
 {
+    typedef ::std::pair< ::rtl::OString, ::rtl::OString > stringpair_t;
+
     typedef ::cppu::ImplInheritanceHelper1< CNode, XAttr > CAttr_Base;
 
     class CAttr
@@ -54,12 +58,16 @@ namespace DOM
 
     private:
         xmlAttrPtr m_aAttrPtr;
+        ::std::auto_ptr< stringpair_t > m_pNamespace;
 
     protected:
         CAttr(CDocument const& rDocument, ::osl::Mutex const& rMutex,
                 xmlAttrPtr const pAttr);
 
     public:
+        /// return the libxml namespace corresponding to m_pNamespace on pNode
+        xmlNsPtr GetNamespace(xmlNodePtr const pNode);
+
         /**
         Returns the name of this attribute.
         */
@@ -129,10 +137,7 @@ namespace DOM
         return CNode::getLastChild();
     }
     virtual OUString SAL_CALL getNamespaceURI()
-        throw (RuntimeException)
-    {
-        return CNode::getNamespaceURI();
-    }
+        throw (RuntimeException);
     virtual Reference< XNode > SAL_CALL getNextSibling()
         throw (RuntimeException)
     {
@@ -154,10 +159,7 @@ namespace DOM
         return CNode::getParentNode();
     }
     virtual OUString SAL_CALL getPrefix()
-        throw (RuntimeException)
-    {
-        return CNode::getPrefix();
-    }
+        throw (RuntimeException);
     virtual Reference< XNode > SAL_CALL getPreviousSibling()
         throw (RuntimeException)
     {
@@ -206,10 +208,7 @@ namespace DOM
         return setValue(nodeValue);
     }
     virtual void SAL_CALL setPrefix(const OUString& prefix)
-        throw (RuntimeException, DOMException)
-    {
-        return CNode::setPrefix(prefix);
-    }
+        throw (RuntimeException, DOMException);
 
     };
 }
diff --git a/unoxml/source/dom/document.cxx b/unoxml/source/dom/document.cxx
index aae543116d5c..44f2c41309b2 100644
--- a/unoxml/source/dom/document.cxx
+++ b/unoxml/source/dom/document.cxx
@@ -425,36 +425,29 @@ namespace DOM
         // libxml does not allow a NS definition to be attached to an
         // attribute node - which is a good thing, since namespaces are
         // only defined as parts of element nodes
-        // thus, we create a temporary element node which carries the ns definition
-        // and is removed/merged as soon as the attribute gets append to it's
-        // actual parent
+        // thus the namespace data is stored in CAttr::m_pNamespace
         sal_Int32 i = qname.indexOf(':');
         OString oPrefix, oName, oUri;
-        xmlChar *xPrefix, *xName, *xUri;
         if (i != -1)
         {
             oPrefix = OUStringToOString(qname.copy(0, i), RTL_TEXTENCODING_UTF8);
-            xPrefix = (xmlChar*)oPrefix.getStr();
             oName = OUStringToOString(qname.copy(i+1, qname.getLength()-i-1), RTL_TEXTENCODING_UTF8);
         }
         else
         {
-            xPrefix = (xmlChar*)"";
             oName = OUStringToOString(qname, RTL_TEXTENCODING_UTF8);
         }
-        xName = (xmlChar*)oName.getStr();
         oUri = OUStringToOString(ns, RTL_TEXTENCODING_UTF8);
-        xUri = (xmlChar*)oUri.getStr();
-
-        // create the carrier node
-        xmlNodePtr pNode = xmlNewDocNode(m_aDocPtr, NULL, (xmlChar*)"__private", NULL);
-        xmlNsPtr pNs = xmlNewNs(pNode, xUri, xPrefix);
-        xmlAttrPtr pAttr = xmlNewNsProp(pNode, pNs, xName, NULL);
-        Reference< XAttr > const xRet(
-            static_cast< XNode* >(GetCNode(
-                    reinterpret_cast<xmlNodePtr>(pAttr)).get()),
-            UNO_QUERY_THROW);
-        return xRet;
+        xmlAttrPtr const pAttr = xmlNewDocProp(m_aDocPtr,
+                reinterpret_cast<xmlChar const*>(oName.getStr()), 0);
+        ::rtl::Reference< CAttr > const pCAttr(
+            dynamic_cast< CAttr* >(GetCNode(
+                    reinterpret_cast<xmlNodePtr>(pAttr)).get()));
+        if (!pCAttr.is()) { throw RuntimeException(); }
+        // store the namespace data!
+        pCAttr->m_pNamespace.reset( new stringpair_t(oUri, oPrefix) );
+
+        return pCAttr.get();
     };
 
     // Creates a CDATASection node whose value is the specified string.
diff --git a/unoxml/source/dom/element.cxx b/unoxml/source/dom/element.cxx
index 4339fb569e7e..1403fc4060a5 100644
--- a/unoxml/source/dom/element.cxx
+++ b/unoxml/source/dom/element.cxx
@@ -503,9 +503,9 @@ namespace DOM
     */
     Reference< XAttr >
     CElement::setAttributeNode_Impl_Lock(
-            Reference< XAttr > const& newAttr, bool const bNS)
+            Reference< XAttr > const& xNewAttr, bool const bNS)
     {
-        if (newAttr->getOwnerDocument() != getOwnerDocument()) {
+        if (xNewAttr->getOwnerDocument() != getOwnerDocument()) {
             DOMException e;
             e.Code = DOMExceptionType_WRONG_DOCUMENT_ERR;
             throw e;
@@ -518,48 +518,30 @@ namespace DOM
         }
 
         // get the implementation
-        CNode *const pCNode = CNode::GetImplementation(newAttr);
-        if (!pCNode) { throw RuntimeException(); }
+        CAttr *const pCAttr = dynamic_cast<CAttr*>(
+                CNode::GetImplementation(xNewAttr));
+        if (!pCAttr) { throw RuntimeException(); }
         xmlAttrPtr const pAttr =
-            reinterpret_cast<xmlAttrPtr>(pCNode->GetNodePtr());
+            reinterpret_cast<xmlAttrPtr>(pCAttr->GetNodePtr());
         if (!pAttr) { throw RuntimeException(); }
 
         // check whether the attribute is not in use by another element
-        xmlNsPtr pNs = NULL;
-        if (pAttr->parent != NULL)
-        {
-            if (strcmp((char*)pAttr->parent->name, "__private") == 0
-                && pNs && pAttr->ns != NULL)
-            {
-                pNs = xmlSearchNs(m_aNodePtr->doc, m_aNodePtr,
-                        pAttr->ns->prefix);
-                if (pNs == NULL ||
-                    strcmp((char*)pNs->href, (char*)pAttr->ns->href) != 0)
-                {
-                    pNs = xmlNewNs(m_aNodePtr, pAttr->ns->href,
-                            pAttr->ns->href);
-                } else {
-                    throw RuntimeException();
-                }
-            }
+        if (pAttr->parent) {
+            DOMException e;
+            e.Code = DOMExceptionType_INUSE_ATTRIBUTE_ERR;
+            throw e;
         }
 
         xmlAttrPtr res = NULL;
 
         if (bNS) {
+            xmlNsPtr const pNs( pCAttr->GetNamespace(m_aNodePtr) );
             res = xmlNewNsProp(m_aNodePtr,
                     pNs, pAttr->name, pAttr->children->content);
         } else {
             res = xmlNewProp(m_aNodePtr, pAttr->name, pAttr->children->content);
         }
 
-        // free carrier node ...
-        if (pAttr->parent != NULL &&
-            strcmp((char*)pAttr->parent->name, "__private") == 0)
-        {
-            xmlFreeNode(pAttr->parent);
-        }
-
         // get the new attr node
         Reference< XAttr > const xAttr(
             static_cast< XNode* >(GetOwnerDocument().GetCNode(
diff --git a/unoxml/source/dom/node.cxx b/unoxml/source/dom/node.cxx
index 54645e35ad78..68ceb9bfa3e4 100644
--- a/unoxml/source/dom/node.cxx
+++ b/unoxml/source/dom/node.cxx
@@ -43,6 +43,7 @@
 #include <com/sun/star/xml/sax/FastToken.hpp>
 
 #include <document.hxx>
+#include <attr.hxx>
 #include <childlist.hxx>
 
 #include "../events/eventdispatcher.hxx"
@@ -317,67 +318,40 @@ namespace DOM
             e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
             throw e;
         }
-        // already has parent and is not attribute
-        if (cur->parent != NULL && cur->type != XML_ATTRIBUTE_NODE) {
+        if (cur->parent != NULL) {
             DOMException e;
             e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
             throw e;
         }
 
-        // check whether this is an attribute node so we remove it's
-        // carrier node if it has one
+        // check whether this is an attribute node; it needs special handling
         xmlNodePtr res = NULL;
         if (cur->type == XML_ATTRIBUTE_NODE)
         {
-            if (cur->parent != NULL) {
-                if (m_aNodePtr->type != XML_ELEMENT_NODE ||
-                    strcmp((char*)cur->parent->name, "__private") != 0)
-                {
-                    DOMException e;
-                    e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
-                    throw e;
-                }
-
-                xmlNsPtr pAttrNs = cur->ns;
-                xmlNsPtr pParentNs =
-                    xmlSearchNs(m_aNodePtr->doc, m_aNodePtr, pAttrNs->prefix);
-                if (pParentNs == NULL ||
-                    strcmp((char*)pParentNs->href, (char*)pAttrNs->href) != 0)
-                {
-                    pParentNs =
-                        xmlNewNs(m_aNodePtr, pAttrNs->href, pAttrNs->prefix);
-                }
-
-                if (cur->children != NULL) {
-                    res = (xmlNodePtr)xmlNewNsProp(m_aNodePtr,
-                            pParentNs, cur->name, cur->children->content);
-                } else {
-                    res = (xmlNodePtr)xmlNewProp(m_aNodePtr,
-                            cur->name, (xmlChar*) "");
-                }
-
-                xmlFreeNode(cur->parent);
-                cur->parent = NULL;
+            xmlChar const*const pChildren((cur->children)
+                    ? cur->children->content
+                    : reinterpret_cast<xmlChar const*>(""));
+            CAttr *const pCAttr(dynamic_cast<CAttr *>(pNewChild));
+            if (!pCAttr) { throw RuntimeException(); }
+            xmlNsPtr const pNs( pCAttr->GetNamespace(m_aNodePtr) );
+            if (pNs) {
+                res = reinterpret_cast<xmlNodePtr>(
+                        xmlNewNsProp(m_aNodePtr, pNs, cur->name, pChildren));
             } else {
-                if (cur->children != NULL) {
-                    res = (xmlNodePtr)xmlNewProp(m_aNodePtr,
-                            cur->name, cur->children->content);
-                } else {
-                    res = (xmlNodePtr)xmlNewProp(m_aNodePtr,
-                            cur->name, (xmlChar*) "");
-                }
+                res = reinterpret_cast<xmlNodePtr>(
+                        xmlNewProp(m_aNodePtr, cur->name, pChildren));
             }
         }
         else
         {
             res = xmlAddChild(m_aNodePtr, cur);
-        }
 
-        // libxml can do optimizations, when appending nodes.
-        // if res != cur, something was optimized and the newchild-wrapper
-        // should be updated
-        if (res && (cur != res)) {
-            pNewChild->invalidate(); // cur has been freed
+            // libxml can do optimization when appending nodes.
+            // if res != cur, something was optimized and the newchild-wrapper
+            // should be updated
+            if (res && (cur != res)) {
+                pNewChild->invalidate(); // cur has been freed
+            }
         }
 
         if (!res) { return 0; }
@@ -696,7 +670,7 @@ namespace DOM
             throw e;
         }
 
-        ::osl::MutexGuard const g(m_rMutex);
+        ::osl::ClearableMutexGuard guard(m_rMutex);
 
         CNode *const pNewNode(CNode::GetImplementation(newChild));
         CNode *const pRefNode(CNode::GetImplementation(refChild));
@@ -710,14 +684,20 @@ namespace DOM
             e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
             throw e;
         }
-        // already has parent and is not attribute
-        if (pNewChild->parent != NULL && pNewChild->type != XML_ATTRIBUTE_NODE)
+        // already has parent
+        if (pNewChild->parent != NULL)
         {
             DOMException e;
             e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
             throw e;
         }
 
+        // attributes are unordered anyway, so just do appendChild
+        if (XML_ATTRIBUTE_NODE == pNewChild->type) {
+            guard.clear();
+            return appendChild(newChild);
+        }
+
         xmlNodePtr cur = m_aNodePtr->children;
 
         //search child before which to insert
@@ -881,8 +861,8 @@ namespace DOM
             e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
             throw e;
         }
-        // already has parent and is not attribute
-        if (pNew->parent != NULL && pNew->type != XML_ATTRIBUTE_NODE) {
+        // already has parent
+        if (pNew->parent != NULL) {
             DOMException e;
             e.Code = DOMExceptionType_HIERARCHY_REQUEST_ERR;
             throw e;