From 179daa20b99db2173607566c250ec511a6bd280a Mon Sep 17 00:00:00 2001 From: Nikolay Sivov Date: Fri, 4 Mar 2011 01:03:15 +0300 Subject: [PATCH] msxml3: Remove child with parent method first before insert with insertBefore(). --- dlls/msxml3/domdoc.c | 5 +--- dlls/msxml3/element.c | 6 +--- dlls/msxml3/node.c | 56 +++++++++++++++++--------------------- dlls/msxml3/nodemap.c | 5 +--- dlls/msxml3/tests/domdoc.c | 53 ++++++++++++++++++++++++++++++++++-- 5 files changed, 78 insertions(+), 47 deletions(-) diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c index c89f55f6e75..a5f9e9e5bfd 100644 --- a/dlls/msxml3/domdoc.c +++ b/dlls/msxml3/domdoc.c @@ -1588,10 +1588,7 @@ static HRESULT WINAPI domdoc_put_documentElement( return hr; xmlNode = get_node_obj( elementNode ); - if(!xmlNode) { - FIXME("elementNode is not our object\n"); - return E_FAIL; - } + if(!xmlNode) return E_FAIL; if(!xmlNode->node->parent) if(xmldoc_remove_orphan(xmlNode->node->doc, xmlNode->node) != S_OK) diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c index f2c5bbbfdd0..5559fd2726e 100644 --- a/dlls/msxml3/element.c +++ b/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) - { - FIXME("att_node is not our node implementation\n"); - return E_FAIL; - } + if (!attr_node) return E_FAIL; if (attr_node->parent) { diff --git a/dlls/msxml3/node.c b/dlls/msxml3/node.c index 71b95a80ef6..fc15a5d29f5 100644 --- a/dlls/msxml3/node.c +++ b/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) { - FIXME("newChild is not our node implementation\n"); - return E_FAIL; - } + if(!node_obj) 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) { - FIXME("before node is not our node implementation\n"); - return E_FAIL; - } + if(!before_node_obj) return E_FAIL; - before_node = node_obj->node; - xmlAddPrevSibling(before_node, new_child_node); + /* unlink from current parent first */ + 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) { - FIXME("oldChild is not our node implementation\n"); - return E_FAIL; - } + if(!old_child) 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) { - FIXME("newChild is not our node implementation\n"); - return E_FAIL; - } + if(!new_child) 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) { - FIXME("childNode is not our node implementation\n"); - return E_FAIL; - } + if(!child_node) 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) { - FIXME("styleSheet is not our xmlnode implementation\n"); - return E_FAIL; - } + if(!sheet) return E_FAIL; xsltSS = pxsltParseStylesheetDoc(sheet->node->doc); if(xsltSS) diff --git a/dlls/msxml3/nodemap.c b/dlls/msxml3/nodemap.c index 6df10283c80..7055da5128d 100644 --- a/dlls/msxml3/nodemap.c +++ b/dlls/msxml3/nodemap.c @@ -231,10 +231,7 @@ static HRESULT WINAPI xmlnodemap_setNamedItem( /* Must be an Attribute */ ThisNew = get_node_obj( newItem ); - if(!ThisNew) { - FIXME("ThisNew is not our node implementation\n"); - return E_FAIL; - } + if(!ThisNew) return E_FAIL; if(ThisNew->node->type != XML_ATTRIBUTE_NODE) return E_FAIL; diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index ba35765bf2b..f84947cbba7 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/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(); }