[PATCH v4 0/1] MR9056: msxml3/element: Implement removeAttributeNode function.

[Reinhold Gschweicher (＠neroburner)]

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

[PATCH v4 1/1] msxml3/element: Implement removeAttributeNode function.

[Reinhold Gschweicher]

From: Reinhold Gschweicher

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 );

[Reinhold Gschweicher (＠neroburner)]

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

[Reinhold Gschweicher (＠neroburner)]

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 
```