[PATCH v2 0/2] 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

--
  v2: msxml3/element: do attribute removal ourselfs

https://gitlab.winehq.org/wine/wine/-/merge_requests/9056

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

[Reinhold Gschweicher]

From: Reinhold Gschweicher

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.
---
 dlls/msxml3/element.c      | 31 +++++++++++++++++++++++++++++--
 dlls/msxml3/tests/domdoc.c |  4 +---
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c
index 3e3c2d06169..41ba2493709 100644
--- a/dlls/msxml3/element.c
+++ b/dlls/msxml3/element.c
@@ -1491,8 +1491,35 @@ static HRESULT WINAPI domelem_removeAttributeNode(
     IXMLDOMAttribute** attributeNode)
 {
     domelem *This = impl_from_IXMLDOMElement( iface );
-    FIXME("(%p)->(%p %p)\n", This, domAttribute, attributeNode);
-    return E_NOTIMPL;
+    DOMNodeType type;
+    HRESULT hr;
+    BSTR attr_name;
+    IXMLDOMNamedNodeMap* attr_map;
+
+    TRACE("(%p)->(%p %p)\n", This, domAttribute, attributeNode);
+
+    if (!domAttribute) {
+        return E_INVALIDARG;
+    }
+
+    /* check given node type to be an attribute */
+    hr = IXMLDOMNode_get_nodeType((IXMLDOMNode *)domAttribute, &type);
+    if (hr != S_OK) return hr;
+    if (type != NODE_ATTRIBUTE) {
+        return E_INVALIDARG;
+    }
+
+    /* get attribute node name and reuse removeNamedItem function */
+    hr = IXMLDOMAttribute_get_nodeName(domAttribute, &attr_name);
+    if (hr != S_OK) return hr;
+    hr = domelem_get_attributes(iface, &attr_map);
+    if (hr != S_OK) return hr;
+    hr = IXMLDOMNamedNodeMap_removeNamedItem(attr_map, attr_name, (IXMLDOMNode **)attributeNode);
+    IXMLDOMNamedNodeMap_Release(attr_map);
+    /* removeNameItem returns S_FALSE if not found,
+     * removeAttributeNode is expected to return E_INVALIDARG */
+    if (hr == S_FALSE) return E_INVALIDARG;
+    return hr;
 }

static HRESULT WINAPI domelem_getElementsByTagName(
diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
index dcda4821125..4e795422bf2 100644
--- a/dlls/msxml3/tests/domdoc.c
+++ b/dlls/msxml3/tests/domdoc.c
@@ -2331,10 +2331,8 @@ static void test_domnode( void )
         ok(attr != NULL, "getAttributeNode returned NULL\n");
         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 */
@@ -2353,7 +2351,7 @@ todo_wine {

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

[PATCH v2 2/2] msxml3/element: do attribute removal ourselfs

[Reinhold Gschweicher]

From: Reinhold Gschweicher

Check if the given attribute has the element as parent. If so remove it
just like `domelem_remove_qualified_item` does.
---
 dlls/msxml3/element.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/dlls/msxml3/element.c b/dlls/msxml3/element.c
index 41ba2493709..c311e327212 100644
--- a/dlls/msxml3/element.c
+++ b/dlls/msxml3/element.c
@@ -1491,35 +1491,30 @@ static HRESULT WINAPI domelem_removeAttributeNode(
     IXMLDOMAttribute** attributeNode)
 {
     domelem *This = impl_from_IXMLDOMElement( iface );
-    DOMNodeType type;
-    HRESULT hr;
-    BSTR attr_name;
-    IXMLDOMNamedNodeMap* attr_map;
+    xmlnode *attr_node;

TRACE("(%p)->(%p %p)\n", This, domAttribute, attributeNode);

if (!domAttribute) {
         return E_INVALIDARG;
     }
-
-    /* check given node type to be an attribute */
-    hr = IXMLDOMNode_get_nodeType((IXMLDOMNode *)domAttribute, &type);
-    if (hr != S_OK) return hr;
-    if (type != NODE_ATTRIBUTE) {
+    attr_node = get_node_obj((IXMLDOMNode*)domAttribute);
+    if (This->node.node != attr_node->node->parent) {
         return E_INVALIDARG;
     }

-    /* get attribute node name and reuse removeNamedItem function */
-    hr = IXMLDOMAttribute_get_nodeName(domAttribute, &attr_name);
-    if (hr != S_OK) return hr;
-    hr = domelem_get_attributes(iface, &attr_map);
-    if (hr != S_OK) return hr;
-    hr = IXMLDOMNamedNodeMap_removeNamedItem(attr_map, attr_name, (IXMLDOMNode **)attributeNode);
-    IXMLDOMNamedNodeMap_Release(attr_map);
-    /* removeNameItem returns S_FALSE if not found,
-     * removeAttributeNode is expected to return E_INVALIDARG */
-    if (hr == S_FALSE) return E_INVALIDARG;
-    return hr;
+    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(

[Reinhold Gschweicher (＠neroburner)]

On Mon Sep 29 06:28:29 2025 +0000, Nikolay Sivov wrote:
> What I mean is that removing a node likely implies that node is actually
> is linked already to this element, as an attribute. Every msxml3 node
> will have a corresponding libxml2 node linked to it. To access that you
> can use get_node_obj(msxmlnode)->node.node or something similar. My
> understanding is that such check will help with extra conditions that
> you have now. And yes, it's not the most straightforward thing to read,
> which mostly comes from an attempt to reconcile libxml2 and msxml differences.

thanks for the hint, now checking against `get_node_obj(msxmlnode)->node.parent`

[Reinhold Gschweicher (＠neroburner)]

Now checking if the given attribute has the element as parent. If so remove it just like the function `domelem_remove_qualified_item` does.

I hope this code duplication is acceptable

[PATCH v2 0/3] MR9056: msxml3/element: Implement removeAttributeNode function.

[Reinhold Gschweicher (＠neroburner)]

wrong formatting slip in
```suggestion:-1+0
    if (attributeNode) {
```