msxml3: Remove child with parent method first before insert with insertBefore().

dlls/msxml3/domdoc.c

@@ -1588,10 +1588,7 @@ static HRESULT WINAPI domdoc_put_documentElement(
         return hr;
 
     xmlNode = get_node_obj( elementNode );
-    if(!xmlNode) {
+    if(!xmlNode) return E_FAIL;
-        FIXME("elementNode is not our object\n");
-        return E_FAIL;
-    }
 
     if(!xmlNode->node->parent)
         if(xmldoc_remove_orphan(xmlNode->node->doc, xmlNode->node) != S_OK)

dlls/msxml3/element.c

@@ -1192,11 +1192,7 @@ static HRESULT WINAPI domelem_setAttributeNode(
     if (!attribute) return E_INVALIDARG;
 
     attr_node = get_node_obj((IXMLDOMNode*)attribute);
-    if (!attr_node)
+    if (!attr_node) return E_FAIL;
-    {
-        FIXME("att_node is not our node implementation\n");
-        return E_FAIL;
-    }
 
     if (attr_node->parent)
     {

dlls/msxml3/node.c

@@ -121,10 +121,11 @@ BOOL node_query_interface(xmlnode *This, REFIID riid, void **ppv)
 
 xmlnode *get_node_obj(IXMLDOMNode *node)
 {
-    xmlnode *obj;
+    xmlnode *obj = NULL;
     HRESULT hres;
 
     hres = IXMLDOMNode_QueryInterface(node, &IID_xmlnode, (void**)&obj);
+    if (!obj) WARN("node is not our IXMLDOMNode implementation\n");
     return SUCCEEDED(hres) ? obj : NULL;
 }
 
@@ -291,7 +292,7 @@ HRESULT node_get_next_sibling(xmlnode *This, IXMLDOMNode **ret)
 HRESULT node_insert_before(xmlnode *This, IXMLDOMNode *new_child, const VARIANT *ref_child,
         IXMLDOMNode **ret)
 {
-    xmlNodePtr before_node, new_child_node;
+    xmlNodePtr new_child_node;
     IXMLDOMNode *before = NULL;
     xmlnode *node_obj;
     HRESULT hr;
@@ -300,10 +301,7 @@ HRESULT node_insert_before(xmlnode *This, IXMLDOMNode *new_child, const VARIANT
         return E_INVALIDARG;
 
     node_obj = get_node_obj(new_child);
-    if(!node_obj) {
+    if(!node_obj) return E_FAIL;
-        FIXME("newChild is not our node implementation\n");
-        return E_FAIL;
-    }
 
     switch(V_VT(ref_child))
     {
@@ -313,7 +311,7 @@ HRESULT node_insert_before(xmlnode *This, IXMLDOMNode *new_child, const VARIANT
 
     case VT_UNKNOWN:
     case VT_DISPATCH:
-        hr = IUnknown_QueryInterface(V_UNKNOWN(ref_child), &IID_IXMLDOMNode, (LPVOID)&before);
+        hr = IUnknown_QueryInterface(V_UNKNOWN(ref_child), &IID_IXMLDOMNode, (void**)&before);
         if(FAILED(hr)) return hr;
         break;
 
@@ -331,22 +329,27 @@ HRESULT node_insert_before(xmlnode *This, IXMLDOMNode *new_child, const VARIANT
 
     if(before)
     {
-        node_obj = get_node_obj(before);
+        xmlnode *before_node_obj = get_node_obj(before);
         IXMLDOMNode_Release(before);
-        if(!node_obj) {
+        if(!before_node_obj) return E_FAIL;
-            FIXME("before node is not our node implementation\n");
-            return E_FAIL;
-        }
 
-        before_node = node_obj->node;
+        /* unlink from current parent first */
-        xmlAddPrevSibling(before_node, new_child_node);
+        if(node_obj->parent)
+            IXMLDOMNode_removeChild(node_obj->parent, node_obj->iface, NULL);
+        xmlAddPrevSibling(before_node_obj->node, new_child_node);
+        node_obj->parent = This->parent;
     }
     else
     {
+        /* unlink from current parent first */
+        if(node_obj->parent)
+            IXMLDOMNode_removeChild(node_obj->parent, node_obj->iface, NULL);
         xmlAddChild(This->node, new_child_node);
+        node_obj->parent = This->iface;
     }
 
-    if(ret) {
+    if(ret)
+    {
         IXMLDOMNode_AddRef(new_child);
         *ret = new_child;
     }
@@ -371,10 +374,7 @@ HRESULT node_replace_child(xmlnode *This, IXMLDOMNode *newChild, IXMLDOMNode *ol
         *ret = NULL;
 
     old_child = get_node_obj(oldChild);
-    if(!old_child) {
+    if(!old_child) return E_FAIL;
-        FIXME("oldChild is not our node implementation\n");
-        return E_FAIL;
-    }
 
     if(old_child->node->parent != This->node)
     {
@@ -383,10 +383,7 @@ HRESULT node_replace_child(xmlnode *This, IXMLDOMNode *newChild, IXMLDOMNode *ol
     }
 
     new_child = get_node_obj(newChild);
-    if(!new_child) {
+    if(!new_child) return E_FAIL;
-        FIXME("newChild is not our node implementation\n");
-        return E_FAIL;
-    }
 
     my_ancestor = This->node;
     while(my_ancestor)
@@ -407,6 +404,8 @@ HRESULT node_replace_child(xmlnode *This, IXMLDOMNode *newChild, IXMLDOMNode *ol
     xmldoc_add_ref(old_child->node->doc);
     xmlReplaceNode(old_child->node, new_child->node);
     xmldoc_release(leaving_doc);
+    new_child->parent = old_child->parent;
+    old_child->parent = NULL;
 
     xmldoc_add_orphan(old_child->node->doc, old_child->node);
 
@@ -429,10 +428,7 @@ HRESULT node_remove_child(xmlnode *This, IXMLDOMNode* child, IXMLDOMNode** oldCh
         *oldChild = NULL;
 
     child_node = get_node_obj(child);
-    if(!child_node) {
+    if(!child_node) return E_FAIL;
-        FIXME("childNode is not our node implementation\n");
-        return E_FAIL;
-    }
 
     if(child_node->node->parent != This->node)
     {
@@ -441,6 +437,7 @@ HRESULT node_remove_child(xmlnode *This, IXMLDOMNode* child, IXMLDOMNode** oldCh
     }
 
     xmlUnlinkNode(child_node->node);
+    child_node->parent = NULL;
 
     if(oldChild)
     {
@@ -959,10 +956,7 @@ HRESULT node_transform_node(const xmlnode *This, IXMLDOMNode *stylesheet, BSTR *
     *p = NULL;
 
     sheet = get_node_obj(stylesheet);
-    if(!sheet) {
+    if(!sheet) return E_FAIL;
-        FIXME("styleSheet is not our xmlnode implementation\n");
-        return E_FAIL;
-    }
 
     xsltSS = pxsltParseStylesheetDoc(sheet->node->doc);
     if(xsltSS)

dlls/msxml3/nodemap.c

@@ -231,10 +231,7 @@ static HRESULT WINAPI xmlnodemap_setNamedItem(
 
     /* Must be an Attribute */
     ThisNew = get_node_obj( newItem );
-    if(!ThisNew) {
+    if(!ThisNew) return E_FAIL;
-        FIXME("ThisNew is not our node implementation\n");
-        return E_FAIL;
-    }
 
     if(ThisNew->node->type != XML_ATTRIBUTE_NODE)
         return E_FAIL;

dlls/msxml3/tests/domdoc.c

@@ -2958,13 +2958,17 @@ static void test_removeChild(void)
 
     /* ba_node is a descendant of element, but not a direct child. */
     removed_node = (void*)0xdeadbeef;
+    EXPECT_REF(ba_node, 1);
     r = IXMLDOMElement_removeChild( element, ba_node, &removed_node );
     ok( r == E_INVALIDARG, "ret %08x\n", r );
     ok( removed_node == NULL, "%p\n", removed_node );
+    EXPECT_REF(ba_node, 1);
 
+    EXPECT_REF(ba_node, 1);
     r = IXMLDOMElement_removeChild( element, fo_node, &removed_node );
     ok( r == S_OK, "ret %08x\n", r);
     ok( fo_node == removed_node, "node %p node2 %p\n", fo_node, removed_node );
+    EXPECT_REF(ba_node, 1);
 
     /* try removing already removed child */
     temp_node = (void*)0xdeadbeef;
@@ -7397,6 +7401,8 @@ static void test_createNode(void)
     doc = create_document(&IID_IXMLDOMDocument);
     if (!doc) return;
 
+    EXPECT_REF(doc, 1);
+
     /* reference count tests */
     hr = IXMLDOMDocument_createElement(doc, _bstr_("elem"), &elem);
     ok( hr == S_OK, "got 0x%08x\n", hr);
@@ -7409,6 +7415,14 @@ todo_wine {
     /* it's released already, attempt to release now will crash it */
 }
 
+    hr = IXMLDOMDocument_createElement(doc, _bstr_("elem"), &elem);
+    ok( hr == S_OK, "got 0x%08x\n", hr);
+    todo_wine EXPECT_REF(elem, 2);
+    IXMLDOMDocument_Release(doc);
+    todo_wine EXPECT_REF(elem, 2);
+
+    doc = create_document(&IID_IXMLDOMDocument);
+
     /* NODE_ELEMENT nodes */
     /* 1. specified namespace */
     V_VT(&v) = VT_I4;
@@ -7964,9 +7978,9 @@ todo_wine {
 
 static void test_insertBefore(void)
 {
-    IXMLDOMDocument *doc;
+    IXMLDOMDocument *doc, *doc2;
     IXMLDOMAttribute *attr;
-    IXMLDOMElement *elem1, *elem2, *elem3;
+    IXMLDOMElement *elem1, *elem2, *elem3, *elem4, *elem5;
     IXMLDOMNode *node, *newnode;
     HRESULT hr;
     VARIANT v;
@@ -8109,11 +8123,44 @@ static void test_insertBefore(void)
     ok(node == (void*)elem2, "got %p\n", node);
 
     EXPECT_CHILDREN(elem3);
-    todo_wine EXPECT_NO_CHILDREN(elem1);
+    EXPECT_NO_CHILDREN(elem1);
+
+    /* cross document case - try to add as child to a node created with other doc */
+    doc2 = create_document(&IID_IXMLDOMDocument);
+
+    hr = IXMLDOMDocument_createElement(doc2, _bstr_("elem4"), &elem4);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    todo_wine EXPECT_REF(elem4, 2);
+
+    /* same name, another instance */
+    hr = IXMLDOMDocument_createElement(doc2, _bstr_("elem4"), &elem5);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    todo_wine EXPECT_REF(elem5, 2);
+
+    todo_wine EXPECT_REF(elem3, 2);
+    V_VT(&v) = VT_NULL;
+    node = NULL;
+    hr = IXMLDOMElement_insertBefore(elem3, (IXMLDOMNode*)elem4, v, &node);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    ok(node == (void*)elem4, "got %p\n", node);
+    todo_wine EXPECT_REF(elem4, 3);
+    todo_wine EXPECT_REF(elem3, 2);
+
+    V_VT(&v) = VT_NULL;
+    node = NULL;
+    hr = IXMLDOMElement_insertBefore(elem3, (IXMLDOMNode*)elem5, v, &node);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    ok(node == (void*)elem5, "got %p\n", node);
+    todo_wine EXPECT_REF(elem4, 3);
+    todo_wine EXPECT_REF(elem5, 3);
+
+    IXMLDOMDocument_Release(doc2);
 
     IXMLDOMElement_Release(elem1);
     IXMLDOMElement_Release(elem2);
     IXMLDOMElement_Release(elem3);
+    IXMLDOMElement_Release(elem4);
+    IXMLDOMElement_Release(elem5);
     IXMLDOMDocument_Release(doc);
     free_bstrs();
 }