[PATCH v4 0/1] MR9056: msxml3/element: Implement removeAttributeNode function.
Implement the function `IXMLDOMElement_removeAttributeNode`. Essentially pass the hard work to `IXMLDOMNamedNodeMap_removeNamedItem`. Use the fact, that attribute names are unique in elements. One case that isn't checked is wheter or not the value of the provided attribute match with the one stored in the dom-element. This MR implements according to the tests introduced in https://gitlab.winehq.org/wine/wine/-/merge_requests/8928 -- v4: msxml3/element: Implement removeAttributeNode function. https://gitlab.winehq.org/wine/wine/-/merge_requests/9056
From: Reinhold Gschweicher <pyro4hell+winehq(a)gmail.com> Implement the function `IXMLDOMElement_removeAttributeNode`. Check if the given attribute has the element as parent. If so remove it just like `domelem_remove_qualified_item` does. Use the fact, that attribute names are unique in elements. Add unittest to check if `removeAttributeNode` with output variable set to `NULL` still removes the attribute from the element. --- dlls/msxml3/element.c | 24 ++++++++++++++++++++++-- dlls/msxml3/tests/domdoc.c | 26 ++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c index 3e3c2d06169..fd9a2973cd8 100644 --- a/dlls/msxml3/element.c +++ b/dlls/msxml3/element.c @@ -1491,8 +1491,28 @@ static HRESULT WINAPI domelem_removeAttributeNode( IXMLDOMAttribute** attributeNode) { domelem *This = impl_from_IXMLDOMElement( iface ); - FIXME("(%p)->(%p %p)\n", This, domAttribute, attributeNode); - return E_NOTIMPL; + xmlnode *attr_node; + + TRACE("(%p)->(%p %p)\n", This, domAttribute, attributeNode); + + if (!domAttribute) + return E_INVALIDARG; + attr_node = get_node_obj((IXMLDOMNode*)domAttribute); + if (This->node.node != attr_node->node->parent) + return E_INVALIDARG; + + if (attributeNode) + { + xmlUnlinkNode(attr_node->node ); + xmldoc_add_orphan(attr_node->node->doc, attr_node->node); + *attributeNode = (IXMLDOMAttribute*)create_node(attr_node->node); + } + else + { + if (xmlRemoveProp((xmlAttrPtr)attr_node->node) == -1) + return E_INVALIDARG; + } + return S_OK; } static HRESULT WINAPI domelem_getElementsByTagName( diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index dcda4821125..b012a9d0c49 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -2329,12 +2329,13 @@ static void test_domnode( void ) hr = IXMLDOMElement_getAttributeNode( element, str, &attr ); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(attr != NULL, "getAttributeNode returned NULL\n"); + /* store attribute value to restore attribute after removal */ + hr = IXMLDOMElement_getAttribute(element, str, &var); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); attr_out = NULL; hr = IXMLDOMElement_removeAttributeNode(element, attr, &attr_out ); -todo_wine { ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(attr_out != NULL, "removeAttributeNode expected to set attr_out, but got NULL pointer\n"); -} if (attr_out) { /* remove the same attribute again returns invalid arg */ @@ -2342,18 +2343,35 @@ todo_wine { ok(hr == E_INVALIDARG, "removeAttributeNode removed an already removed node, unexpected hr %#lx.\n", hr); /* readd removed attribute to recover previous state */ - hr = IXMLDOMElement_setAttributeNode(element, attr_out, NULL); + hr = IXMLDOMElement_setAttribute(element, str, var); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IXMLDOMAttribute_Release(attr_out); } IXMLDOMAttribute_Release(attr); + /* remove attribute with output set to NULL and check if properly removed */ + attr = NULL; + hr = IXMLDOMElement_getAttributeNode( element, str, &attr ); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(attr != NULL, "getAttributeNode returned NULL\n"); + hr = IXMLDOMElement_removeAttributeNode( element, attr, NULL ); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + attr_out = NULL; + hr = IXMLDOMElement_getAttributeNode( element, str, &attr_out ); + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); + ok(attr_out == NULL, "getAttributeNode found attribute that should be removed\n"); + /* readd removed attribute to recover previous state */ + hr = IXMLDOMElement_setAttribute(element, str, var); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IXMLDOMAttribute_Release(attr); + SysFreeString( str ); + VariantClear( &var ); attr_out = (IXMLDOMAttribute*)0xdeadbeef; hr = IXMLDOMElement_removeAttributeNode( element, NULL, &attr_out ); - todo_wine ok(hr == E_INVALIDARG, "removeAttributeNode removed a NULL pointer hr: %#lx.\n", hr); + ok(hr == E_INVALIDARG, "removeAttributeNode removed a NULL pointer hr: %#lx.\n", hr); ok(attr_out == (IXMLDOMAttribute*)0xdeadbeef, "removeAttributeNode expected to not touch attr_out in error case, got (%p)\n", attr_out); hr = IXMLDOMElement_get_attributes( element, &map ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9056
On Thu Oct 2 12:54:30 2025 +0000, Reinhold Gschweicher wrote:
I don't think we have a test for null out argument, for when attribute still exists. should I add a new test here or in a separate MR? added test with `removeAttributeNode` with output `NULL`, applied formatting, squashed
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9056#note_117425
failing linux32 test unrelated to this MR: https://gitlab.winehq.org/neroburner/wine/-/blob/4d4c5c6c63c7a59239c23c30006... ``` user32:input input.c:4320 Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 018B00DE, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9056#note_117454
participants (2)
-
Reinhold Gschweicher -
Reinhold Gschweicher (＠neroburner)