[PATCH 01/14] mshtml: Retry with VT_NULL when removing an attribute.

[Gabriel Ivăncescu]

set_event_handler fails with "attempt to set to VT_EMPTY in quirks mode"
intentionally. But it shouldn't throw when removing the builtin attribute.

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/dispex.c              |  5 ++++
 dlls/mshtml/tests/documentmode.js | 40 +++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 96a776d..bddb0e3 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1466,6 +1466,11 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)

V_VT(&var) = VT_EMPTY;
         hres = builtin_propput(This, func, &dp, NULL);
+        if(hres == E_NOTIMPL) {
+            /* Setting event handlers to VT_EMPTY fails in quirks mode */
+            V_VT(&var) = VT_NULL;
+            hres = builtin_propput(This, func, &dp, NULL);
+        }
         if(FAILED(hres))
             return hres;

diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index d566223..c197f9b 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1087,6 +1087,46 @@ sync_test("elem_attr", function() {
     ok(r === "cls2", "class attr = " + r);
     r = elem.getAttribute("className");
     ok(r === "cls3", "className attr = " + r);
+
+    var func = function() { };
+    elem.onclick = func;
+    ok(elem.onclick === func, "onclick = " + elem.onclick);
+    r = elem.getAttribute("onclick");
+    ok(r === (v < 8 ? func : null), "onclick attr = " + r);
+    r = elem.removeAttribute("onclick");
+    todo_wine_if(v < 9).
+    ok(r === (v < 9 ? false : undefined), "removeAttribute returned " + r);
+    todo_wine_if(v < 9).
+    ok(elem.onclick === (v != 8 ? func : null), "removed onclick = " + elem.onclick);
+
+    elem.onclick_test = func;
+    ok(elem.onclick_test === func, "onclick_test = " + elem.onclick_test);
+    r = elem.getAttribute("onclick_test");
+    todo_wine_if(v === 8).
+    ok(r === (v < 8 ? func : (v < 9 ? func.toString() : null)), "onclick_test attr = " + r);
+
+    elem.setAttribute("onclick", "test");
+    r = elem.getAttribute("onclick");
+    ok(r === "test", "onclick attr after setAttribute = " + r);
+    r = elem.removeAttribute("onclick");
+    ok(r === (v < 9 ? true : undefined), "removeAttribute after setAttribute returned " + r);
+
+    elem.setAttribute("onclick", "string");
+    r = elem.getAttribute("onclick");
+    ok(r === "string", "onclick attr after setAttribute = " + r);
+    elem.onclick = func;
+    ok(elem.onclick === func, "onclick = " + elem.onclick);
+    r = elem.getAttribute("onclick");
+    todo_wine_if(v === 8).
+    ok(r === (v < 8 ? func : (v < 9 ? null : "string")), "onclick attr = " + r);
+    elem.onclick = "test";
+    r = elem.getAttribute("onclick");
+    todo_wine_if(v === 8).
+    ok(r === (v < 9 ? "test" : "string"), "onclick attr = " + r);
+    r = elem.removeAttribute("onclick");
+    ok(r === (v < 9 ? true : undefined), "removeAttribute returned " + r);
+    todo_wine_if(v >= 8).
+    ok(elem.onclick === null, "removed onclick = " + elem.onclick);
 });

sync_test("__proto__", function() {

[PATCH 02/14] mshtml: Actually set the return value when removing an attribute.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/htmlelem.c            | 8 ++++++--
 dlls/mshtml/tests/documentmode.js | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 91ccfb9..4324387 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1202,8 +1202,12 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA

TRACE("(%p)->(%s %x %p)\n", This, debugstr_w(strAttributeName), lFlags, pfSuccess);

-    if(dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8)
-        return element_remove_attribute(This, strAttributeName);
+    if(dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8) {
+        *pfSuccess = element_has_attribute(This, strAttributeName);
+        if(*pfSuccess == VARIANT_TRUE)
+            return element_remove_attribute(This, strAttributeName);
+        return S_OK;
+    }

hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
             lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &id);
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index c197f9b..a2c3714 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1094,7 +1094,7 @@ sync_test("elem_attr", function() {
     r = elem.getAttribute("onclick");
     ok(r === (v < 8 ? func : null), "onclick attr = " + r);
     r = elem.removeAttribute("onclick");
-    todo_wine_if(v < 9).
+    todo_wine_if(v < 8).
     ok(r === (v < 9 ? false : undefined), "removeAttribute returned " + r);
     todo_wine_if(v < 9).
     ok(elem.onclick === (v != 8 ? func : null), "removed onclick = " + elem.onclick);

[PATCH 02/14] mshtml: Actually set the return value when removing an attribute.

[Jacek Caban]

On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> - if(dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8)
> -
>    return element_remove_attribute(This, strAttributeName);
> - if(dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8) {
> -
>    *pfSuccess = element_has_attribute(This, strAttributeName);
> -
>    if(*pfSuccess == VARIANT_TRUE)

Please don't compare bools to a value, this could be simply:

if(*pfSuccess)

Thanks,

Jacek

[PATCH 02/14] mshtml: Actually set the return value when removing an attribute.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=101996

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
events.c:1089: Test failed: unexpected call img_onerror
events: Timeout

=== w1064_tsign (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_ja (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w7u_adm (32 bit report) ===

mshtml:
script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1637079683334 expected 1637079683397"

[PATCH 03/14] mshtml: Return false when removing a read-only builtin attribute rather than throwing.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/dispex.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index bddb0e3..d10064e 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1464,6 +1464,11 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
             return S_OK;
         }

+        if(!func->put_vtbl_off) {
+            *success = VARIANT_FALSE;
+            return S_OK;
+        }
+
         V_VT(&var) = VT_EMPTY;
         hres = builtin_propput(This, func, &dp, NULL);
         if(hres == E_NOTIMPL) {

[PATCH 04/14] mshtml: Handle VT_DISPATCH in variant_to_nsstr.

[Gabriel Ivăncescu]

Some javascript libraries such as prototype.js use this (by setting to an
array) to examine setAttribute quirks.

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/nsembed.c             | 49 +++++++++++++++++++++--
 dlls/mshtml/tests/documentmode.js | 64 +++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c
index 4224d6a..d8210f7 100644
--- a/dlls/mshtml/nsembed.c
+++ b/dlls/mshtml/nsembed.c
@@ -994,6 +994,8 @@ HRESULT return_nsstr_variant(nsresult nsres, nsAString *nsstr, unsigned flags, V
 HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
 {
     WCHAR buf[32];
+    VARIANT strv;
+    HRESULT hres;

switch(V_VT(v)) {
     case VT_NULL:
@@ -1013,10 +1015,7 @@ HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
         nsAString_Init(nsstr, buf);
         break;

-    case VT_R8: {
-        VARIANT strv;
-        HRESULT hres;
-
+    case VT_R8:
         V_VT(&strv) = VT_EMPTY;
         hres = VariantChangeTypeEx(&strv, v, MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT),
                                    0, VT_BSTR);
@@ -1026,6 +1025,48 @@ HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
         nsAString_Init(nsstr, V_BSTR(&strv));
         SysFreeString(V_BSTR(&strv));
         break;
+
+    case VT_DISPATCH: {
+        LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT);
+        IDispatch *disp = V_DISPATCH(v);
+        DISPID dispid;
+        WCHAR *name;
+
+        if(!disp) {
+            nsAString_InitDepend(nsstr, NULL);
+            return S_OK;
+        }
+
+        /* try toString() first */
+        memcpy(buf, L"toString", sizeof(L"toString"));
+        name = buf;
+        hres = IDispatch_GetIDsOfNames(disp, &IID_NULL, &name, 1, 0, &dispid);
+        if(SUCCEEDED(hres) && dispid != DISPID_UNKNOWN) {
+            DISPPARAMS params = { &strv, NULL, 0, 0 };
+            hres = IDispatch_Invoke(disp, dispid, &IID_NULL, lcid, DISPATCH_METHOD, &params, &strv, NULL, NULL);
+            if(SUCCEEDED(hres)) {
+                if(V_VT(&strv) == VT_BSTR) {
+                    nsAString_Init(nsstr, V_BSTR(&strv));
+                    SysFreeString(V_BSTR(&strv));
+                    break;
+                }
+                VariantClear(&strv);
+            }
+        }
+
+        /* try value */
+        hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, lcid, DISPATCH_PROPERTYGET, NULL, &strv, NULL, NULL);
+        if(SUCCEEDED(hres)) {
+            if(V_VT(&strv) == VT_BSTR) {
+                nsAString_Init(nsstr, V_BSTR(&strv));
+                SysFreeString(V_BSTR(&strv));
+                break;
+            }
+            VariantClear(&strv);
+        }
+
+        FIXME("don't know how to handle dispatch %s\n", debugstr_variant(v));
+        return E_NOTIMPL;
     }

default:
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index a2c3714..ce29f23 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1088,6 +1088,41 @@ sync_test("elem_attr", function() {
     r = elem.getAttribute("className");
     ok(r === "cls3", "className attr = " + r);

+    var arr = [3];
+    elem.setAttribute("testattr", arr);
+    r = elem.getAttribute("testattr");
+    ok(r === (v < 8 ? arr : "3"), "testattr = " + r);
+    todo_wine_if(v === 8).
+    ok(elem.testattr === (v < 9 ? arr : undefined), "elem.testattr = " + elem.testattr);
+    r = elem.removeAttribute("testattr");
+    ok(r === (v < 9 ? true : undefined), "testattr removeAttribute returned " + r);
+    ok(elem.testattr === undefined, "removed testattr = " + elem.testattr);
+
+    arr[0] = 9;
+    elem.setAttribute("testattr", "string");
+    elem.testattr = arr;
+    r = elem.getAttribute("testattr");
+    todo_wine_if(v === 8).
+    ok(r === (v < 8 ? arr : (v < 9 ? "9" : "string")), "testattr = " + r);
+    ok(elem.testattr === arr, "elem.testattr = " + elem.testattr);
+    arr[0] = 3;
+    r = elem.getAttribute("testattr");
+    todo_wine_if(v === 8).
+    ok(r === (v < 8 ? arr : (v < 9 ? "3" : "string")), "testattr = " + r);
+    ok(elem.testattr === arr, "elem.testattr = " + elem.testattr);
+    r = elem.removeAttribute("testattr");
+    ok(r === (v < 9 ? true : undefined), "testattr removeAttribute returned " + r);
+    todo_wine_if(v === 8).
+    ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr = " + elem.testattr);
+
+    elem.setAttribute("id", arr);
+    r = elem.getAttribute("id");
+    todo_wine_if(v >= 8 && v < 10).
+    ok(r === (v < 8 || v >= 10 ? "3" : "[object]"), "id = " + r);
+    r = elem.removeAttribute("id");
+    ok(r === (v < 9 ? true : undefined), "id removeAttribute returned " + r);
+    ok(elem.id === "", "removed id = " + elem.id);
+
     var func = function() { };
     elem.onclick = func;
     ok(elem.onclick === func, "onclick = " + elem.onclick);
@@ -1110,6 +1145,13 @@ sync_test("elem_attr", function() {
     ok(r === "test", "onclick attr after setAttribute = " + r);
     r = elem.removeAttribute("onclick");
     ok(r === (v < 9 ? true : undefined), "removeAttribute after setAttribute returned " + r);
+    if(v < 11)  /* IE11 returns an empty function, which we can't check */
+        todo_wine_if(v >= 8).
+        ok(elem.onclick === null, "removed onclick after setAttribute = " + elem.onclick);
+    r = Object.prototype.toString.call(elem.onclick);
+    todo_wine_if(v >= 8 && v < 11).
+    ok(r === (v < 9 ? "[object Object]" : (v < 11 ? "[object Null]" : "[object Function]")),
+        "removed onclick after setAttribute Object.toString returned " + r);

elem.setAttribute("onclick", "string");
     r = elem.getAttribute("onclick");
@@ -1127,6 +1169,28 @@ sync_test("elem_attr", function() {
     ok(r === (v < 9 ? true : undefined), "removeAttribute returned " + r);
     todo_wine_if(v >= 8).
     ok(elem.onclick === null, "removed onclick = " + elem.onclick);
+
+    elem.setAttribute("ondblclick", arr);
+    r = elem.getAttribute("ondblclick");
+    todo_wine_if(v >= 8 && v < 10).
+    ok(r === (v < 8 ? arr : (v < 10 ? "[object]" : "3")), "ondblclick = " + r);
+    r = elem.removeAttribute("ondblclick");
+    todo_wine_if(v < 8).
+    ok(r === (v < 8 ? false : (v < 9 ? true : undefined)), "ondblclick removeAttribute returned " + r);
+    if(v < 11)  /* IE11 returns an empty function, which we can't check */
+        todo_wine_if(v < 8).
+        ok(elem.ondblclick === (v < 8 ? arr : null), "removed ondblclick = " + elem.ondblclick);
+    r = Object.prototype.toString.call(elem.ondblclick);
+    todo_wine_if(v != 8).
+    ok(r === (v < 8 ? "[object Array]" : (v < 9 ? "[object Object]" : (v < 11 ? "[object Null]" : "[object Function]"))),
+        "removed ondblclick Object.toString returned " + r);
+
+    elem.setAttribute("ondblclick", "string");
+    r = elem.getAttribute("ondblclick");
+    ok(r === "string", "ondblclick string = " + r);
+    r = elem.removeAttribute("ondblclick");
+    ok(r === (v < 9 ? true : undefined), "ondblclick string removeAttribute returned " + r);
+    ok(elem.ondblclick === null, "removed ondblclick string = " + elem.ondblclick);
 });

sync_test("__proto__", function() {

[PATCH 04/14] mshtml: Handle VT_DISPATCH in variant_to_nsstr.

[Jacek Caban]

On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> -
>    /* try toString() first */
> -
>    memcpy(buf, L"toString", sizeof(L"toString"));
> -
>    name = buf;
> -
>    hres = IDispatch_GetIDsOfNames(disp, &IID_NULL, &name, 1, 0, &dispid);
> -
>    if(SUCCEEDED(hres) && dispid != DISPID_UNKNOWN) {
> -
>        DISPPARAMS params = { &strv, NULL, 0, 0 };
> -
>        hres = IDispatch_Invoke(disp, dispid, &IID_NULL, lcid, DISPATCH_METHOD, &params, &strv, NULL, NULL);
> -
>        if(SUCCEEDED(hres)) {
> -
>            if(V_VT(&strv) == VT_BSTR) {
> -
>                nsAString_Init(nsstr, V_BSTR(&strv));
> -
>                SysFreeString(V_BSTR(&strv));
> -
>                break;
> -
>            }
> -
>            VariantClear(&strv);
> -
>        }
> -
>    }

Why do we need it in addition to DISPID_VALUE?

Thanks,

Jacek

[PATCH 04/14] mshtml: Handle VT_DISPATCH in variant_to_nsstr.

[Gabriel Ivăncescu]

On 16/11/2021 17:03, Jacek Caban wrote:
> On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> > +        /* try toString() first */
> > +        memcpy(buf, L"toString", sizeof(L"toString"));
> > +        name = buf;
> > +        hres = IDispatch_GetIDsOfNames(disp, &IID_NULL, &name, 1, 0, 
> > &dispid);
> > +        if(SUCCEEDED(hres) && dispid != DISPID_UNKNOWN) {
> > +            DISPPARAMS params = { &strv, NULL, 0, 0 };
> > +            hres = IDispatch_Invoke(disp, dispid, &IID_NULL, lcid, 
> > DISPATCH_METHOD, &params, &strv, NULL, NULL);
> > +            if(SUCCEEDED(hres)) {
> > +                if(V_VT(&strv) == VT_BSTR) {
> > +                    nsAString_Init(nsstr, V_BSTR(&strv));
> > +                    SysFreeString(V_BSTR(&strv));
> > +                    break;
> > +                }
> > +                VariantClear(&strv);
> > +            }
> > +        }
>
> Why do we need it in addition to DISPID_VALUE?
>
> Thanks,
>
> Jacek

Well, for jscript objects, I wasn't sure if they were guaranteed to call 
the toString method on the value. Maybe I should actually test it and 
see what happens on an object with custom toString.

[PATCH 04/14] mshtml: Handle VT_DISPATCH in variant_to_nsstr.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=101998

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
htmldoc.c:3084: Test failed: Incorrect error code: -2146697211
htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/"
htmldoc.c:5861: Test failed: expected OnChanged_1012
htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV
htmldoc.c:5864: Test failed: expected Exec_SETTITLE
htmldoc.c:5905: Test failed: expected FireNavigateComplete2

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w10pro64_he (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_ja (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

[PATCH 05/14] mshtml: Don't remove non-string builtin props when removing an attribute.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/dispex.c              | 18 +++++++++++++++++-
 dlls/mshtml/tests/documentmode.js |  7 ++-----
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index d10064e..79ebc78 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1412,6 +1412,22 @@ static HRESULT invoke_builtin_prop(DispatchEx *This, DISPID id, LCID lcid, WORD
     return hres;
 }

+static BOOL is_string_builtin(DispatchEx *This, func_info_t *func)
+{
+    BOOL ret = FALSE;
+    VARIANT var;
+
+    if(func->prop_vt == VT_BSTR)
+        return TRUE;
+    if(func->prop_vt != VT_VARIANT)
+        return FALSE;
+    if(SUCCEEDED(builtin_propget(This, func, NULL, &var))) {
+        ret = (V_VT(&var) == VT_BSTR);
+        VariantClear(&var);
+    }
+    return ret;
+}
+
 HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
 {
     switch(get_dispid_type(id)) {
@@ -1464,7 +1480,7 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
             return S_OK;
         }

-        if(!func->put_vtbl_off) {
+        if(!func->put_vtbl_off || !is_string_builtin(This, func)) {
             *success = VARIANT_FALSE;
             return S_OK;
         }
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index ce29f23..2bc0cbc 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1129,9 +1129,8 @@ sync_test("elem_attr", function() {
     r = elem.getAttribute("onclick");
     ok(r === (v < 8 ? func : null), "onclick attr = " + r);
     r = elem.removeAttribute("onclick");
-    todo_wine_if(v < 8).
     ok(r === (v < 9 ? false : undefined), "removeAttribute returned " + r);
-    todo_wine_if(v < 9).
+    todo_wine_if(v === 8).
     ok(elem.onclick === (v != 8 ? func : null), "removed onclick = " + elem.onclick);

elem.onclick_test = func;
@@ -1175,13 +1174,11 @@ sync_test("elem_attr", function() {
     todo_wine_if(v >= 8 && v < 10).
     ok(r === (v < 8 ? arr : (v < 10 ? "[object]" : "3")), "ondblclick = " + r);
     r = elem.removeAttribute("ondblclick");
-    todo_wine_if(v < 8).
     ok(r === (v < 8 ? false : (v < 9 ? true : undefined)), "ondblclick removeAttribute returned " + r);
     if(v < 11)  /* IE11 returns an empty function, which we can't check */
-        todo_wine_if(v < 8).
         ok(elem.ondblclick === (v < 8 ? arr : null), "removed ondblclick = " + elem.ondblclick);
     r = Object.prototype.toString.call(elem.ondblclick);
-    todo_wine_if(v != 8).
+    todo_wine_if(v >= 9).
     ok(r === (v < 8 ? "[object Array]" : (v < 9 ? "[object Object]" : (v < 11 ? "[object Null]" : "[object Function]"))),
         "removed ondblclick Object.toString returned " + r);

[PATCH 05/14] mshtml: Don't remove non-string builtin props when removing an attribute.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=101999

Your paranoid android.

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_he (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w7u_adm (32 bit report) ===

mshtml:
script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1637087253936 expected 1637087253999"

[PATCH 06/14] mshtml: Reset contentEditable to the default when removed (empty string).

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/htmlelem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 4324387..b333e07 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -4005,7 +4005,7 @@ static HRESULT WINAPI HTMLElement3_put_contentEditable(IHTMLElement3 *iface, BST
         return E_NOTIMPL;
     }

-    nsAString_InitDepend(&str, v);
+    nsAString_InitDepend(&str, v[0] ? v : L"inherit");
     nsres = nsIDOMHTMLElement_SetContentEditable(This->html_element, &str);
     nsAString_Finish(&str);

[PATCH 07/14] mshtml: Convert the hex_int BOOL to flags in variant_to_nsstr.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/htmlelem.c       | 2 +-
 dlls/mshtml/htmlframe.c      | 4 ++--
 dlls/mshtml/htmlnode.c       | 2 +-
 dlls/mshtml/htmlstyle.c      | 2 +-
 dlls/mshtml/htmltable.c      | 4 ++--
 dlls/mshtml/mshtml_private.h | 4 +++-
 dlls/mshtml/nsembed.c        | 4 ++--
 7 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index b333e07..b7a5d98 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1095,7 +1095,7 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
         nsAString name_str, value_str;
         nsresult nsres;

-        hres = variant_to_nsstr(&AttributeValue, FALSE, &value_str);
+        hres = variant_to_nsstr(&AttributeValue, 0, &value_str);
         if(FAILED(hres))
             return hres;

diff --git a/dlls/mshtml/htmlframe.c b/dlls/mshtml/htmlframe.c
index e794284..07889b0 100644
--- a/dlls/mshtml/htmlframe.c
+++ b/dlls/mshtml/htmlframe.c
@@ -1239,7 +1239,7 @@ static HRESULT WINAPI HTMLIFrameElement2_put_height(IHTMLIFrameElement2 *iface,

TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));

-    hres = variant_to_nsstr(&v, FALSE, &nsstr);
+    hres = variant_to_nsstr(&v, 0, &nsstr);
     if(FAILED(hres))
         return hres;

@@ -1277,7 +1277,7 @@ static HRESULT WINAPI HTMLIFrameElement2_put_width(IHTMLIFrameElement2 *iface, V

TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));

-    hres = variant_to_nsstr(&v, FALSE, &nsstr);
+    hres = variant_to_nsstr(&v, 0, &nsstr);
     if(FAILED(hres))
         return hres;

diff --git a/dlls/mshtml/htmlnode.c b/dlls/mshtml/htmlnode.c
index 996c28c..418c958 100644
--- a/dlls/mshtml/htmlnode.c
+++ b/dlls/mshtml/htmlnode.c
@@ -1245,7 +1245,7 @@ static HRESULT WINAPI HTMLDOMNode3_put_textContent(IHTMLDOMNode3 *iface, VARIANT

TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));

-    variant_to_nsstr(&v, FALSE, &nsstr);
+    variant_to_nsstr(&v, 0, &nsstr);
     nsres = nsIDOMNode_SetTextContent(This->nsnode, &nsstr);
     nsAString_Finish(&nsstr);
     if(NS_FAILED(nsres)) {
diff --git a/dlls/mshtml/htmlstyle.c b/dlls/mshtml/htmlstyle.c
index 2cbd8a5..b00b9f8 100644
--- a/dlls/mshtml/htmlstyle.c
+++ b/dlls/mshtml/htmlstyle.c
@@ -799,7 +799,7 @@ static HRESULT var_to_styleval(CSSStyle *style, VARIANT *v, const style_tbl_entr
     unsigned flags = entry && dispex_compat_mode(&style->dispex) < COMPAT_MODE_IE9
         ? entry->flags : 0;

-    hres = variant_to_nsstr(v, !!(flags & ATTR_HEX_INT), nsstr);
+    hres = variant_to_nsstr(v, (flags & ATTR_HEX_INT) ? VARIANT_TO_NSSTR_HEX_INT : 0, nsstr);
     if(SUCCEEDED(hres) && (flags & ATTR_FIX_PX))
         fix_px_value(nsstr);
     return hres;
diff --git a/dlls/mshtml/htmltable.c b/dlls/mshtml/htmltable.c
index 53c9820..d771d83 100644
--- a/dlls/mshtml/htmltable.c
+++ b/dlls/mshtml/htmltable.c
@@ -337,7 +337,7 @@ static HRESULT WINAPI HTMLTableCell_put_width(IHTMLTableCell *iface, VARIANT v)

TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));

-    hres = variant_to_nsstr(&v, FALSE, &nsstr);
+    hres = variant_to_nsstr(&v, 0, &nsstr);
     if(FAILED(hres))
         return hres;

@@ -368,7 +368,7 @@ static HRESULT WINAPI HTMLTableCell_put_height(IHTMLTableCell *iface, VARIANT v)

TRACE("(%p)->(%s)\n", This, debugstr_variant(&v));

-    hres = variant_to_nsstr(&v, FALSE, &nsstr);
+    hres = variant_to_nsstr(&v, 0, &nsstr);
     if(FAILED(hres))
         return hres;

diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 5cf53bb..66cd10a 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -1012,9 +1012,11 @@ void nsAString_Finish(nsAString*) DECLSPEC_HIDDEN;
 HRESULT map_nsresult(nsresult) DECLSPEC_HIDDEN;
 HRESULT return_nsstr(nsresult,nsAString*,BSTR*) DECLSPEC_HIDDEN;
 HRESULT return_nsstr_variant(nsresult,nsAString*,unsigned,VARIANT*) DECLSPEC_HIDDEN;
-HRESULT variant_to_nsstr(VARIANT*,BOOL,nsAString*) DECLSPEC_HIDDEN;
+HRESULT variant_to_nsstr(VARIANT*,DWORD,nsAString*) DECLSPEC_HIDDEN;
 HRESULT return_nsform(nsresult,nsIDOMHTMLFormElement*,IHTMLFormElement**) DECLSPEC_HIDDEN;

+#define VARIANT_TO_NSSTR_HEX_INT     0x01
+
 nsICommandParams *create_nscommand_params(void) DECLSPEC_HIDDEN;
 HRESULT nsnode_to_nsstring(nsIDOMNode*,nsAString*) DECLSPEC_HIDDEN;
 void setup_editor_controller(GeckoBrowser*) DECLSPEC_HIDDEN;
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c
index d8210f7..6381f7c 100644
--- a/dlls/mshtml/nsembed.c
+++ b/dlls/mshtml/nsembed.c
@@ -991,7 +991,7 @@ HRESULT return_nsstr_variant(nsresult nsres, nsAString *nsstr, unsigned flags, V
  * allocations, the function uses an existing string if available, so caller must
  * ensure that passed VARIANT is unchanged as long as its string representation is used
  */
-HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
+HRESULT variant_to_nsstr(VARIANT *v, DWORD flags, nsAString *nsstr)
 {
     WCHAR buf[32];
     VARIANT strv;
@@ -1011,7 +1011,7 @@ HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
         break;

case VT_I4:
-        wsprintfW(buf, hex_int ? L"#%06x" : L"%d", V_I4(v));
+        wsprintfW(buf, (flags & VARIANT_TO_NSSTR_HEX_INT) ? L"#%06x" : L"%d", V_I4(v));
         nsAString_Init(nsstr, buf);
         break;

[PATCH 08/14] mshtml: Stringify attribute values in IE8+ mode when using setAttribute.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---

Simplifies the next patch.

dlls/mshtml/htmlelem.c       | 40 ++++++++++++++++++++++++++++--------
 dlls/mshtml/mshtml_private.h |  1 +
 dlls/mshtml/nsembed.c        | 19 +++++++++++++++++
 3 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index b7a5d98..2067afd 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1086,34 +1086,56 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
                                                VARIANT AttributeValue, LONG lFlags)
 {
     HTMLElement *This = impl_from_IHTMLElement(iface);
+    compat_mode_t compat_mode = dispex_compat_mode(&This->node.event_target.dispex);
+    BOOL stringify = compat_mode >= COMPAT_MODE_IE8;
+    nsAString name_str, value_str;
+    BOOL needs_free = FALSE;
+    nsresult nsres;
     DISPID dispid;
     HRESULT hres;

TRACE("(%p)->(%s %s %08x)\n", This, debugstr_w(strAttributeName), debugstr_variant(&AttributeValue), lFlags);

-    if(This->dom_element && dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8) {
-        nsAString name_str, value_str;
-        nsresult nsres;
-
-        hres = variant_to_nsstr(&AttributeValue, 0, &value_str);
+    if(stringify) {
+        hres = variant_to_nsstr(&AttributeValue, VARIANT_TO_NSSTR_BSTR_DEPEND, &value_str);
         if(FAILED(hres))
             return hres;

+        if((V_VT(&AttributeValue) & ~VT_BYREF) != VT_BSTR)
+            needs_free = TRUE;
+
+        V_VT(&AttributeValue) = VT_BSTR;
+        nsAString_GetData(&value_str, (const WCHAR**)&V_BSTR(&AttributeValue));
+
+        if(!V_BSTR(&AttributeValue)) {
+            V_VT(&AttributeValue) = VT_NULL;
+            needs_free = FALSE;
+        }
+    }
+
+    if(stringify && This->dom_element) {
         nsAString_InitDepend(&name_str, strAttributeName);
         nsres = nsIDOMElement_SetAttribute(This->dom_element, &name_str, &value_str);
         nsAString_Finish(&name_str);
-        nsAString_Finish(&value_str);
         if(NS_FAILED(nsres))
             WARN("SetAttribute failed: %08x\n", nsres);
-        return map_nsresult(nsres);
+        hres = map_nsresult(nsres);
+        goto done;
     }

hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
             (lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive) | fdexNameEnsure, &dispid);
     if(FAILED(hres))
-        return hres;
+        goto done;
+
+    hres = set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);

-    return set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);
+done:
+    if(stringify)
+        nsAString_Finish(&value_str);
+    if(needs_free)
+        SysFreeString(V_BSTR(&AttributeValue));
+    return hres;
 }

HRESULT get_elem_attr_value_by_dispid(HTMLElement *elem, DISPID dispid, VARIANT *ret)
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 66cd10a..3f04c62 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -1016,6 +1016,7 @@ HRESULT variant_to_nsstr(VARIANT*,DWORD,nsAString*) DECLSPEC_HIDDEN;
 HRESULT return_nsform(nsresult,nsIDOMHTMLFormElement*,IHTMLFormElement**) DECLSPEC_HIDDEN;

#define VARIANT_TO_NSSTR_HEX_INT     0x01
+#define VARIANT_TO_NSSTR_BSTR_DEPEND 0x02

nsICommandParams *create_nscommand_params(void) DECLSPEC_HIDDEN;
 HRESULT nsnode_to_nsstring(nsIDOMNode*,nsAString*) DECLSPEC_HIDDEN;
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c
index 6381f7c..1e8fdbc 100644
--- a/dlls/mshtml/nsembed.c
+++ b/dlls/mshtml/nsembed.c
@@ -1012,6 +1012,13 @@ HRESULT variant_to_nsstr(VARIANT *v, DWORD flags, nsAString *nsstr)

case VT_I4:
         wsprintfW(buf, (flags & VARIANT_TO_NSSTR_HEX_INT) ? L"#%06x" : L"%d", V_I4(v));
+        if(flags & VARIANT_TO_NSSTR_BSTR_DEPEND) {
+            BSTR bstr = SysAllocString(buf);
+            if(!bstr)
+                return E_OUTOFMEMORY;
+            nsAString_InitDepend(nsstr, bstr);
+            break;
+        }
         nsAString_Init(nsstr, buf);
         break;

@@ -1022,6 +1029,10 @@ HRESULT variant_to_nsstr(VARIANT *v, DWORD flags, nsAString *nsstr)
         if(FAILED(hres))
             return hres;

+        if(flags & VARIANT_TO_NSSTR_BSTR_DEPEND) {
+            nsAString_InitDepend(nsstr, V_BSTR(&strv));
+            break;
+        }
         nsAString_Init(nsstr, V_BSTR(&strv));
         SysFreeString(V_BSTR(&strv));
         break;
@@ -1046,6 +1057,10 @@ HRESULT variant_to_nsstr(VARIANT *v, DWORD flags, nsAString *nsstr)
             hres = IDispatch_Invoke(disp, dispid, &IID_NULL, lcid, DISPATCH_METHOD, &params, &strv, NULL, NULL);
             if(SUCCEEDED(hres)) {
                 if(V_VT(&strv) == VT_BSTR) {
+                    if(flags & VARIANT_TO_NSSTR_BSTR_DEPEND) {
+                        nsAString_InitDepend(nsstr, V_BSTR(&strv));
+                        break;
+                    }
                     nsAString_Init(nsstr, V_BSTR(&strv));
                     SysFreeString(V_BSTR(&strv));
                     break;
@@ -1058,6 +1073,10 @@ HRESULT variant_to_nsstr(VARIANT *v, DWORD flags, nsAString *nsstr)
         hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, lcid, DISPATCH_PROPERTYGET, NULL, &strv, NULL, NULL);
         if(SUCCEEDED(hres)) {
             if(V_VT(&strv) == VT_BSTR) {
+                if(flags & VARIANT_TO_NSSTR_BSTR_DEPEND) {
+                    nsAString_InitDepend(nsstr, V_BSTR(&strv));
+                    break;
+                }
                 nsAString_Init(nsstr, V_BSTR(&strv));
                 SysFreeString(V_BSTR(&strv));
                 break;

[PATCH 09/14] mshtml: Fix setting and retrieving attributes in IE8 mode.

[Gabriel Ivăncescu]

For non-builtin props, getAttribute retrieves the stringified value of the
prop. For builtins, however, getAttribute returns null unless they were
set to a string.

Note that setAttribute stringifies the value if it's a builtin, and this
works even for read-only builtins (unlike in previous modes).

Signed-off-by: Gabriel Ivăncescu 
---

I'm testing for/htmlFor since prototype.js also uses it for translations,
but it doesn't seem to be affected here.

Unfortunately this patch can't be split without introducing temporary bad
behavior and failing tests. It does, however, add a lot more tests to it
as well.

dlls/mshtml/dispex.c              |  10 ++
 dlls/mshtml/htmlelem.c            |  93 +++++++++++---
 dlls/mshtml/mshtml_private.h      |   1 +
 dlls/mshtml/tests/documentmode.js | 203 ++++++++++++++++++++++++++++--
 4 files changed, 280 insertions(+), 27 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 79ebc78..6b18d24 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1412,6 +1412,16 @@ static HRESULT invoke_builtin_prop(DispatchEx *This, DISPID id, LCID lcid, WORD
     return hres;
 }

+BOOL is_readonly_builtin(DispatchEx *dispex, DISPID id)
+{
+    func_info_t *func;
+
+    if(FAILED(get_builtin_func(dispex->info, id, &func)))
+        return FALSE;
+
+    return !func->put_vtbl_off;
+}
+
 static BOOL is_string_builtin(DispatchEx *This, func_info_t *func)
 {
     BOOL ret = FALSE;
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 2067afd..77dc5c3 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1096,6 +1096,19 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr

TRACE("(%p)->(%s %s %08x)\n", This, debugstr_w(strAttributeName), debugstr_variant(&AttributeValue), lFlags);

+    /* class and className are special case in IE8, behave like IE9 */
+    if(compat_mode == COMPAT_MODE_IE8 && !wcsnicmp(strAttributeName, L"class", 5) &&
+       (!strAttributeName[5] || !wcsicmp(strAttributeName + 5, L"Name")))
+        compat_mode = COMPAT_MODE_IE9;
+
+    if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) {
+        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+                (lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive) | fdexNameEnsure, &dispid);
+        if(FAILED(hres))
+            return hres;
+        stringify &= (get_dispid_type(dispid) == DISPEXPROP_BUILTIN);
+    }
+
     if(stringify) {
         hres = variant_to_nsstr(&AttributeValue, VARIANT_TO_NSSTR_BSTR_DEPEND, &value_str);
         if(FAILED(hres))
@@ -1113,23 +1126,26 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
         }
     }

-    if(stringify && This->dom_element) {
+    /* style is special case */
+    if(compat_mode == COMPAT_MODE_IE8 && dispid == DISPID_IHTMLELEMENT_STYLE)
+        compat_mode = COMPAT_MODE_IE9;
+
+    if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) {
+        hres = set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);
+        if(FAILED(hres) && compat_mode < COMPAT_MODE_IE8)
+            goto done;
+    }else
+        hres = E_FAIL;
+
+    if(hres == E_FAIL && stringify && This->dom_element) {
         nsAString_InitDepend(&name_str, strAttributeName);
         nsres = nsIDOMElement_SetAttribute(This->dom_element, &name_str, &value_str);
         nsAString_Finish(&name_str);
         if(NS_FAILED(nsres))
             WARN("SetAttribute failed: %08x\n", nsres);
         hres = map_nsresult(nsres);
-        goto done;
     }

-    hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
-            (lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive) | fdexNameEnsure, &dispid);
-    if(FAILED(hres))
-        goto done;
-
-    hres = set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);
-
 done:
     if(stringify)
         nsAString_Finish(&value_str);
@@ -1178,6 +1194,9 @@ static HRESULT WINAPI HTMLElement_getAttribute(IHTMLElement *iface, BSTR strAttr
                                                LONG lFlags, VARIANT *AttributeValue)
 {
     HTMLElement *This = impl_from_IHTMLElement(iface);
+    compat_mode_t compat_mode = dispex_compat_mode(&This->node.event_target.dispex);
+    nsAString name_str, value_str;
+    nsresult nsres;
     DISPID dispid;
     HRESULT hres;

@@ -1186,10 +1205,12 @@ static HRESULT WINAPI HTMLElement_getAttribute(IHTMLElement *iface, BSTR strAttr
     if(lFlags & ~(ATTRFLAG_CASESENSITIVE|ATTRFLAG_ASSTRING))
         FIXME("Unsupported flags %x\n", lFlags);

-    if(This->dom_element && dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8) {
-        nsAString name_str, value_str;
-        nsresult nsres;
+    /* class and className are special case in IE8, behave like IE9 */
+    if(compat_mode == COMPAT_MODE_IE8 && !wcsnicmp(strAttributeName, L"class", 5) &&
+       (!strAttributeName[5] || !wcsicmp(strAttributeName + 5, L"Name")))
+        compat_mode = COMPAT_MODE_IE9;

+    if(This->dom_element && compat_mode >= COMPAT_MODE_IE9) {
         nsAString_InitDepend(&name_str, strAttributeName);
         nsAString_InitDepend(&value_str, NULL);
         nsres = nsIDOMElement_GetAttribute(This->dom_element, &name_str, &value_str);
@@ -1209,8 +1230,40 @@ static HRESULT WINAPI HTMLElement_getAttribute(IHTMLElement *iface, BSTR strAttr
         return hres;
     }

+    if(This->dom_element && compat_mode >= COMPAT_MODE_IE8 && get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
+        if(is_readonly_builtin(&This->node.event_target.dispex, dispid)) {
+            nsAString_InitDepend(&name_str, strAttributeName);
+            nsAString_InitDepend(&value_str, NULL);
+            nsres = nsIDOMElement_GetAttribute(This->dom_element, &name_str, &value_str);
+            nsAString_Finish(&name_str);
+            return return_nsstr_variant(nsres, &value_str, 0, AttributeValue);
+        }
+
+        hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
+        if(SUCCEEDED(hres) && V_VT(AttributeValue) != VT_BSTR) {
+            VariantClear(AttributeValue);
+            V_VT(AttributeValue) = VT_NULL;
+        }
+        return hres;
+    }
+
     hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
-    if(SUCCEEDED(hres) && (lFlags & ATTRFLAG_ASSTRING))
+    if(FAILED(hres))
+        return hres;
+
+    if(compat_mode >= COMPAT_MODE_IE8 && V_VT(AttributeValue) != VT_BSTR) {
+        nsAString value_str;
+
+        hres = variant_to_nsstr(AttributeValue, VARIANT_TO_NSSTR_BSTR_DEPEND, &value_str);
+        VariantClear(AttributeValue);
+        if(FAILED(hres))
+            return hres;
+
+        nsAString_GetData(&value_str, (const WCHAR**)&V_BSTR(AttributeValue));
+        nsAString_Finish(&value_str);
+
+        V_VT(AttributeValue) = V_BSTR(AttributeValue) ? VT_BSTR : VT_NULL;
+    }else if(lFlags & ATTRFLAG_ASSTRING)
         hres = attr_value_to_string(AttributeValue);
     return hres;
 }
@@ -1219,16 +1272,23 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
                                                   LONG lFlags, VARIANT_BOOL *pfSuccess)
 {
     HTMLElement *This = impl_from_IHTMLElement(iface);
+    compat_mode_t compat_mode = dispex_compat_mode(&This->node.event_target.dispex);
     DISPID id;
     HRESULT hres;

TRACE("(%p)->(%s %x %p)\n", This, debugstr_w(strAttributeName), lFlags, pfSuccess);

-    if(dispex_compat_mode(&This->node.event_target.dispex) >= COMPAT_MODE_IE8) {
+    /* class and className are special case in IE8, behave like IE9 */
+    if(compat_mode == COMPAT_MODE_IE8 && !wcsnicmp(strAttributeName, L"class", 5) &&
+       (!strAttributeName[5] || !wcsicmp(strAttributeName + 5, L"Name")))
+        compat_mode = COMPAT_MODE_IE9;
+
+    if(compat_mode >= COMPAT_MODE_IE8) {
         *pfSuccess = element_has_attribute(This, strAttributeName);
         if(*pfSuccess == VARIANT_TRUE)
             return element_remove_attribute(This, strAttributeName);
-        return S_OK;
+        if(compat_mode >= COMPAT_MODE_IE9)
+            return S_OK;
     }

hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
@@ -6485,6 +6545,9 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex)
     nsresult nsres;
     HRESULT hres;

+    if(dispex_compat_mode(dispex) >= COMPAT_MODE_IE9)
+        return S_OK;
+
     if(!This->dom_element)
         return S_FALSE;

diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 3f04c62..9f10d68 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -396,6 +396,7 @@ const void *dispex_get_vtbl(DispatchEx*) DECLSPEC_HIDDEN;
 void dispex_info_add_interface(dispex_data_t*,tid_t,const dispex_hook_t*) DECLSPEC_HIDDEN;
 compat_mode_t dispex_compat_mode(DispatchEx*) DECLSPEC_HIDDEN;
 HRESULT dispex_to_string(DispatchEx*,BSTR*) DECLSPEC_HIDDEN;
+BOOL is_readonly_builtin(DispatchEx*,DISPID) DECLSPEC_HIDDEN;

typedef enum {
     DISPEXPROP_CUSTOM,
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 2bc0cbc..ee834b2 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -518,9 +518,7 @@ sync_test("createElement_inline_attr", function() {
         for(var i = 0; i < tags.length; i++) {
             e = document.createElement("<" + tags[i] + " test='a"' abcd="&quot;b&#34;">");
             ok(e.tagName === tags[i].toUpperCase(), "<" + tags[i] + " test="a" abcd="b">.tagName returned " + e.tagName);
-            todo_wine_if(v == 8).
             ok(e.test === "a"", "<" + tags[i] + " test='a"' abcd="&quot;b&#34;">.test returned " + e.test);
-            todo_wine_if(v == 8).
             ok(e.abcd === ""b"", "<" + tags[i] + " test='a"' abcd="&quot;b&#34;">.abcd returned " + e.abcd);
         }
     }else {
@@ -1063,6 +1061,13 @@ sync_test("elem_attr", function() {
     var v = document.documentMode;
     var elem = document.createElement("div"), r;

+    function test_exposed(prop, expect) {
+        if(expect)
+            ok(prop in elem, prop + " is not exposed from elem");
+        else
+            ok(!(prop in elem), prop + " is exposed from elem");
+    }
+
     r = elem.getAttribute("class");
     ok(r === null, "class attr = " + r);
     r = elem.getAttribute("className");
@@ -1088,11 +1093,37 @@ sync_test("elem_attr", function() {
     r = elem.getAttribute("className");
     ok(r === "cls3", "className attr = " + r);

+    elem.htmlFor = "for";
+    r = elem.getAttribute("for");
+    ok(r === null, "for attr = " + r);
+    r = elem.getAttribute("htmlFor");
+    ok(r === (v < 9 ? "for" : null), "htmlFor attr = " + r);
+
+    elem.setAttribute("for", "for2");
+    ok(elem.htmlFor === "for", "elem.htmlFor = " + elem.htmlFor);
+    r = elem.getAttribute("for");
+    ok(r === "for2", "for attr = " + r);
+    r = elem.getAttribute("htmlFor");
+    ok(r === (v < 9 ? "for" : null), "htmlFor attr = " + r);
+
+    elem.setAttribute("htmlFor", "for3");
+    ok(elem.htmlFor === (v < 9 ? "for3" : "for"), "elem.htmlFor = " + elem.htmlFor);
+    r = elem.getAttribute("for");
+    ok(r === "for2", "for attr = " + r);
+    r = elem.getAttribute("htmlFor");
+    ok(r === "for3", "htmlFor attr = " + r);
+
+    elem.setAttribute("testattr", "test");
+    test_exposed("class", v < 8);
+    test_exposed("className", true);
+    test_exposed("for", v < 9);
+    test_exposed("htmlFor", true);
+    test_exposed("testattr", v < 9);
+
     var arr = [3];
     elem.setAttribute("testattr", arr);
     r = elem.getAttribute("testattr");
     ok(r === (v < 8 ? arr : "3"), "testattr = " + r);
-    todo_wine_if(v === 8).
     ok(elem.testattr === (v < 9 ? arr : undefined), "elem.testattr = " + elem.testattr);
     r = elem.removeAttribute("testattr");
     ok(r === (v < 9 ? true : undefined), "testattr removeAttribute returned " + r);
@@ -1102,17 +1133,14 @@ sync_test("elem_attr", function() {
     elem.setAttribute("testattr", "string");
     elem.testattr = arr;
     r = elem.getAttribute("testattr");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? arr : (v < 9 ? "9" : "string")), "testattr = " + r);
     ok(elem.testattr === arr, "elem.testattr = " + elem.testattr);
     arr[0] = 3;
     r = elem.getAttribute("testattr");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? arr : (v < 9 ? "3" : "string")), "testattr = " + r);
     ok(elem.testattr === arr, "elem.testattr = " + elem.testattr);
     r = elem.removeAttribute("testattr");
     ok(r === (v < 9 ? true : undefined), "testattr removeAttribute returned " + r);
-    todo_wine_if(v === 8).
     ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr = " + elem.testattr);

elem.setAttribute("id", arr);
@@ -1136,7 +1164,6 @@ sync_test("elem_attr", function() {
     elem.onclick_test = func;
     ok(elem.onclick_test === func, "onclick_test = " + elem.onclick_test);
     r = elem.getAttribute("onclick_test");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? func : (v < 9 ? func.toString() : null)), "onclick_test attr = " + r);

elem.setAttribute("onclick", "test");
@@ -1145,10 +1172,10 @@ sync_test("elem_attr", function() {
     r = elem.removeAttribute("onclick");
     ok(r === (v < 9 ? true : undefined), "removeAttribute after setAttribute returned " + r);
     if(v < 11)  /* IE11 returns an empty function, which we can't check */
-        todo_wine_if(v >= 8).
+        todo_wine_if(v >= 9).
         ok(elem.onclick === null, "removed onclick after setAttribute = " + elem.onclick);
     r = Object.prototype.toString.call(elem.onclick);
-    todo_wine_if(v >= 8 && v < 11).
+    todo_wine_if(v >= 9 && v < 11).
     ok(r === (v < 9 ? "[object Object]" : (v < 11 ? "[object Null]" : "[object Function]")),
         "removed onclick after setAttribute Object.toString returned " + r);

@@ -1158,15 +1185,13 @@ sync_test("elem_attr", function() {
     elem.onclick = func;
     ok(elem.onclick === func, "onclick = " + elem.onclick);
     r = elem.getAttribute("onclick");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? func : (v < 9 ? null : "string")), "onclick attr = " + r);
     elem.onclick = "test";
     r = elem.getAttribute("onclick");
-    todo_wine_if(v === 8).
     ok(r === (v < 9 ? "test" : "string"), "onclick attr = " + r);
     r = elem.removeAttribute("onclick");
     ok(r === (v < 9 ? true : undefined), "removeAttribute returned " + r);
-    todo_wine_if(v >= 8).
+    todo_wine_if(v >= 9).
     ok(elem.onclick === null, "removed onclick = " + elem.onclick);

elem.setAttribute("ondblclick", arr);
@@ -1188,6 +1213,160 @@ sync_test("elem_attr", function() {
     r = elem.removeAttribute("ondblclick");
     ok(r === (v < 9 ? true : undefined), "ondblclick string removeAttribute returned " + r);
     ok(elem.ondblclick === null, "removed ondblclick string = " + elem.ondblclick);
+
+    if(v < 9) {
+        var props = [
+            [ "contentEditable", "inherit", "true" ],
+            [ "dir", "", "ltr" ],
+            [ "id", "" ],
+            [ "lang", "" ],
+            [ "language", "" ],
+            [ "onbeforeactivate" ],
+            [ "onblur" ],
+            [ "oncontextmenu" ],
+            [ "ondataavailable" ],
+            [ "ondrag" ],
+            [ "ondragstart" ],
+            [ "onfocus" ],
+            [ "onfocusin" ],
+            [ "onfocusout" ],
+            [ "onhelp" ],
+            [ "onkeydown" ],
+            [ "onkeypress" ],
+            [ "onkeyup" ],
+            [ "onmousedown" ],
+            [ "onmousemove" ],
+            [ "onmouseout" ],
+            [ "onmouseover" ],
+            [ "onmouseup" ],
+            [ "onmousewheel" ],
+            [ "onpaste" ],
+            [ "onreadystatechange" ],
+            [ "onresize" ],
+            [ "onscroll" ],
+            [ "onselectstart" ],
+            [ "title", "" ]
+        ];
+
+        for(var i = 0; i < props.length; i++) {
+            var name = props[i][0];
+            var val = props[i].length > 2 ? props[i][2] : "test";
+
+            r = elem.getAttribute(name);
+            todo_wine_if(v === 8 && props[i][0].substring(0, 2) !== "on").
+            ok(r === (v < 8 && props[i].length > 1 ? props[i][1] : null), name + " attr before set = " + r);
+            eval("elem." + name + " = "" + val + ""; r = elem." + name + ";");
+            ok(r === val, "elem." + name + " = " + r);
+
+            r = elem.getAttribute(name);
+            ok(r === val, name + " attr = " + r);
+            r = elem.removeAttribute(name);
+            ok(r === true, "removeAttribute('" + name + "') returned " + r);
+            eval("r = elem." + name + ";");
+            ok(r === (props[i].length > 1 ? props[i][1] : null), "removed elem." + name + " = " + r);
+
+            elem.setAttribute(name, val);
+            r = elem.getAttribute(name);
+            ok(r === val, name + " attr after setAttribute = " + r);
+            eval("r = elem." + name + ";");
+            ok(r === val, "elem." + name + " after setAttribute = " + r);
+        }
+
+        /* read-only props */
+        props = [
+            "all",
+            "attributes",
+            "childNodes",
+            "children",
+            "clientHeight",
+            "clientLeft",
+            "clientTop",
+            "clientWidth",
+            "currentStyle",
+            "document",
+            "filters",
+            "firstChild",
+            "lastChild",
+            "nextSibling",
+            "nodeName",
+            "nodeType",
+            "offsetHeight",
+            "offsetLeft",
+            "offsetParent",
+            "offsetTop",
+            "offsetWidth",
+            "ownerDocument",
+            "parentElement",
+            "parentNode",
+            "previousSibling",
+            "readyState",
+            "runtimeStyle",
+            "sourceIndex",
+            "tagName",
+            "uniqueID",
+            "uniqueNumber"
+        ];
+
+        for(var i = 0; i < props.length; i++) {
+            var name = props[i], prop;
+
+            try {
+                eval("elem." + name + " = "test";");
+                ok(false, "expected exception setting elem." + name);
+            }catch(ex) { }
+
+            r = elem.getAttribute(name);
+            eval("prop = elem." + name + ";");
+
+            if(v < 8)
+                ok(""+r === ""+prop, name + " attr = " + r);
+            else
+                ok(r === null, name + " attr = " + r);
+            r = elem.removeAttribute(name);
+            ok(r === false, "removeAttribute('" + name + "') returned " + r);
+
+            try {
+                elem.setAttribute(name, "string");
+                ok(v >= 8, "expected exception calling setAttribute('" + name + "')");
+            }catch(ex) {
+                ok(v < 8, "did not expect exception calling setAttribute('" + name + "')");
+            }
+            if(v >= 8) {
+                r = elem.getAttribute(name);
+                ok(r === "string", name + " attr after setAttribute = " + r);
+                eval("r = elem." + name + ";");
+                todo_wine.
+                ok(r === "string", "elem." + name + " after setAttribute = " + r);
+
+                r = elem.removeAttribute(name);
+                ok(r === true, "removeAttribute('" + name + "') after setAttribute returned " + r);
+                eval("r = elem." + name + ";");
+                ok(""+r === ""+prop, name + " attr = " + r);
+            }
+        }
+
+        /* style is special case */
+        try {
+            elem.style = "opacity: 1.0";
+            ok(false, "expected exception setting elem.style");
+        }catch(ex) { }
+
+        var style = elem.style;
+        r = elem.getAttribute("style");
+        ok(r === (v < 8 ? style : null), "style attr = " + r);
+        r = elem.removeAttribute("style");
+        ok(r === true, "removeAttribute('style') returned " + r);
+        r = elem.style;
+        ok(r === style, "removed elem.style = " + r);
+        r = elem.getAttribute("style");
+        todo_wine_if(v === 8).
+        ok(r === (v < 8 ? style : null), "style attr after removal = " + r);
+        elem.setAttribute("style", "opacity: 1.0");
+        r = elem.getAttribute("style");
+        ok(r === (v < 8 ? style : "opacity: 1.0"), "style attr after setAttribute = " + r);
+        r = elem.style;
+        ok(r === style, "elem.style after setAttribute = " + r);
+    }
 });

sync_test("__proto__", function() {

[PATCH 09/14] mshtml: Fix setting and retrieving attributes in IE8 mode.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102003

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
events.c:1089: Test failed: unexpected call img_onerror
events: Timeout

=== w8adm (32 bit report) ===

mshtml:
htmldoc.c:3084: Test failed: Incorrect error code: -2146697211
htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/"
htmldoc.c:5861: Test failed: expected OnChanged_1012
htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV
htmldoc.c:5864: Test failed: expected Exec_SETTITLE
htmldoc.c:5905: Test failed: expected FireNavigateComplete2

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_he (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w10pro64_ja (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_zh_CN (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w7u_el (32 bit report) ===

mshtml:
script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1637090107250 expected 1637090107390"

[PATCH 10/14] mshtml: Set dispatch values to "[object]" for builtin attributes in IE8 and IE9 modes.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/htmlelem.c            | 30 +++++++++++++++++++++++++-----
 dlls/mshtml/tests/documentmode.js |  2 --
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 77dc5c3..8ff4aee 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1110,15 +1110,35 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
     }

if(stringify) {
-        hres = variant_to_nsstr(&AttributeValue, VARIANT_TO_NSSTR_BSTR_DEPEND, &value_str);
-        if(FAILED(hres))
-            return hres;
+        BOOL object_str = FALSE;
+
+        if(V_VT(&AttributeValue) == VT_DISPATCH) {
+            if(compat_mode < COMPAT_MODE_IE9)
+                object_str = TRUE;
+            else if(compat_mode < COMPAT_MODE_IE10) {
+                hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+                        lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &dispid);
+                if(SUCCEEDED(hres) && get_dispid_type(dispid) == DISPEXPROP_BUILTIN)
+                    object_str = TRUE;
+            }
+        }

-        if((V_VT(&AttributeValue) & ~VT_BYREF) != VT_BSTR)
+        if(object_str) {
+            if(!(V_BSTR(&AttributeValue) = SysAllocString(L"[object]")))
+                return E_OUTOFMEMORY;
+            nsAString_InitDepend(&value_str, V_BSTR(&AttributeValue));
             needs_free = TRUE;
+        }else {
+            hres = variant_to_nsstr(&AttributeValue, VARIANT_TO_NSSTR_BSTR_DEPEND, &value_str);
+            if(FAILED(hres))
+                return hres;

+            if((V_VT(&AttributeValue) & ~VT_BYREF) != VT_BSTR)
+                needs_free = TRUE;
+
+            nsAString_GetData(&value_str, (const WCHAR**)&V_BSTR(&AttributeValue));
+        }
         V_VT(&AttributeValue) = VT_BSTR;
-        nsAString_GetData(&value_str, (const WCHAR**)&V_BSTR(&AttributeValue));

if(!V_BSTR(&AttributeValue)) {
             V_VT(&AttributeValue) = VT_NULL;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index ee834b2..2ac8823 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1145,7 +1145,6 @@ sync_test("elem_attr", function() {

elem.setAttribute("id", arr);
     r = elem.getAttribute("id");
-    todo_wine_if(v >= 8 && v < 10).
     ok(r === (v < 8 || v >= 10 ? "3" : "[object]"), "id = " + r);
     r = elem.removeAttribute("id");
     ok(r === (v < 9 ? true : undefined), "id removeAttribute returned " + r);
@@ -1196,7 +1195,6 @@ sync_test("elem_attr", function() {

elem.setAttribute("ondblclick", arr);
     r = elem.getAttribute("ondblclick");
-    todo_wine_if(v >= 8 && v < 10).
     ok(r === (v < 8 ? arr : (v < 10 ? "[object]" : "3")), "ondblclick = " + r);
     r = elem.removeAttribute("ondblclick");
     ok(r === (v < 8 ? false : (v < 9 ? true : undefined)), "ondblclick removeAttribute returned " + r);

[PATCH 10/14] mshtml: Set dispatch values to "[object]" for builtin attributes in IE8 and IE9 modes.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102004

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
htmldoc.c:3084: Test failed: Incorrect error code: -2146697211
htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/"
htmldoc.c:5861: Test failed: expected OnChanged_1012
htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV
htmldoc.c:5864: Test failed: expected Exec_SETTITLE
htmldoc.c:5905: Test failed: expected FireNavigateComplete2

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_zh_CN (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

[PATCH 11/14] mshtml: Search attributes first when retrieving read-only builtin props in IE8 mode.

[Gabriel Ivăncescu]

If an attribute was set over a read-only builtin, reading the prop gives
the attribute's value string, until it is removed, at which point it reverts
back to retrieving the prop's value.

Hooks have to be used, though, because some builtins return non-string values
(such as clientWidth returning integer value), so we need to override them.

Signed-off-by: Gabriel Ivăncescu 
---

This extends the hook callbacks with name and dispid, as well as the init_info
method so that it optionally returns a structure that contains generic hooks
for all read-only builtin props (and in next patch, for read-write builtins).

dlls/mshtml/dispex.c              | 26 +++++++++++++--
 dlls/mshtml/htmldoc.c             |  3 +-
 dlls/mshtml/htmlelem.c            | 54 ++++++++++++++++++++++++++++++-
 dlls/mshtml/htmlevent.c           | 11 ++++---
 dlls/mshtml/htmlnode.c            |  4 +--
 dlls/mshtml/htmlstyle.c           |  3 +-
 dlls/mshtml/htmlstyle.h           |  2 +-
 dlls/mshtml/htmlstyleelem.c       |  6 ++--
 dlls/mshtml/htmlstylesheet.c      |  3 +-
 dlls/mshtml/htmlwindow.c          |  4 +--
 dlls/mshtml/mshtml_private.h      | 20 ++++++++----
 dlls/mshtml/tests/documentmode.js |  2 --
 dlls/mshtml/xmlhttprequest.c      |  9 +++---
 13 files changed, 114 insertions(+), 33 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 6b18d24..b30ca97 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -470,11 +470,12 @@ static int __cdecl func_name_cmp(const void *p1, const void *p2)

static dispex_data_t *preprocess_dispex_data(dispex_static_data_t *desc, compat_mode_t compat_mode)
 {
+    const generic_prop_hooks_t *prop_hooks = NULL;
     const tid_t *tid;
     dispex_data_t *data;
-    DWORD i;
     ITypeInfo *dti;
     HRESULT hres;
+    DWORD i, j;

if(desc->disp_tid) {
         hres = get_typeinfo(desc->disp_tid, &dti);
@@ -503,7 +504,7 @@ static dispex_data_t *preprocess_dispex_data(dispex_static_data_t *desc, compat_
     list_add_tail(&dispex_data_list, &data->entry);

if(desc->init_info)
-        desc->init_info(data, compat_mode);
+        prop_hooks = desc->init_info(data, compat_mode);

for(tid = desc->iface_tids; *tid; tid++) {
         hres = process_interface(data, *tid, dti, NULL);
@@ -519,6 +520,25 @@ static dispex_data_t *preprocess_dispex_data(dispex_static_data_t *desc, compat_
         return data;
     }

+    if(prop_hooks) {
+        for(i = 0; i < data->func_cnt; i++) {
+            func_info_t *func = &data->funcs[i];
+
+            if(func->hook || func->func_disp_idx != -1)
+                continue;
+
+            if(prop_hooks->exclude_list) {
+                for(j = 0; prop_hooks->exclude_list[j] != DISPID_UNKNOWN; j++)
+                    if(prop_hooks->exclude_list[j] == func->id)
+                        break;
+                if(prop_hooks->exclude_list[j] == func->id)
+                    continue;
+            }
+
+            func->hook = func->put_vtbl_off ? NULL : prop_hooks->ro_prop_hook;
+        }
+    }
+

data->funcs = heap_realloc(data->funcs, data->func_cnt * sizeof(func_info_t));
     qsort(data->funcs, data->func_cnt, sizeof(func_info_t), dispid_cmp);
@@ -1365,7 +1385,7 @@ static HRESULT invoke_builtin_prop(DispatchEx *This, DISPID id, LCID lcid, WORD
         return hres;

if(func->hook) {
-        hres = func->hook(This, lcid, flags, dp, res, ei, caller);
+        hres = func->hook(This, id, func->name, lcid, flags, dp, res, ei, caller);
         if(hres != S_FALSE)
             return hres;
     }
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c
index d2caaef..1a02fc7 100644
--- a/dlls/mshtml/htmldoc.c
+++ b/dlls/mshtml/htmldoc.c
@@ -5628,7 +5628,7 @@ static const tid_t HTMLDocumentNode_iface_tids[] = {
     0
 };

-static void HTMLDocumentNode_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
+static const generic_prop_hooks_t *HTMLDocumentNode_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     HTMLDOMNode_init_dispex_info(info, mode);

@@ -5646,6 +5646,7 @@ static void HTMLDocumentNode_init_dispex_info(dispex_data_t *info, compat_mode_t
         dispex_info_add_interface(info, IHTMLDocument6_tid, NULL);
         dispex_info_add_interface(info, IHTMLDocument3_tid, NULL);
     }
+    return NULL;
 }

static dispex_static_data_t HTMLDocumentNode_dispex = {
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 8ff4aee..edc0e8e 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1334,6 +1334,9 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
         if(FAILED(hres))
             return hres;

+        if(compat_mode >= COMPAT_MODE_IE8)
+            element_remove_attribute(This, strAttributeName);
+
         *pfSuccess = VARIANT_TRUE;
         return S_OK;
     }
@@ -6690,13 +6693,60 @@ static IHTMLEventObj *HTMLElement_set_current_event(DispatchEx *dispex, IHTMLEve
     return default_set_current_event(This->node.doc->window, event);
 }

-void HTMLElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
+static HRESULT ie8_ro_prop_hook(DispatchEx *dispex, DISPID id, BSTR name, LCID lcid, WORD flags,
+        DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
+{
+    HTMLElement *This = impl_from_DispatchEx(dispex);
+    nsAString name_str, val_str;
+    const PRUnichar *val;
+    nsresult nsres;
+    cpp_bool b;
+
+    if(!res || !This->dom_element)
+        return S_FALSE;
+
+    nsAString_InitDepend(&name_str, name);
+    nsres = nsIDOMElement_HasAttribute(This->dom_element, &name_str, &b);
+    if(NS_FAILED(nsres) || !b) {
+        nsAString_Finish(&name_str);
+        return S_FALSE;
+    }
+
+    nsAString_Init(&val_str, NULL);
+    nsres = nsIDOMElement_GetAttribute(This->dom_element, &name_str, &val_str);
+    nsAString_Finish(&name_str);
+    if(NS_FAILED(nsres)) {
+        nsAString_Finish(&val_str);
+        return S_FALSE;
+    }
+
+    nsAString_GetData(&val_str, &val);
+    V_BSTR(res) = SysAllocString(val);
+    nsAString_Finish(&val_str);
+
+    if(!V_BSTR(res))
+        return E_OUTOFMEMORY;
+    V_VT(res) = VT_BSTR;
+
+    return S_OK;
+}
+
+const generic_prop_hooks_t *HTMLElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     static const dispex_hook_t elem2_ie11_hooks[] = {
         {DISPID_IHTMLELEMENT2_DOSCROLL, NULL},
         {DISPID_IHTMLELEMENT2_READYSTATE, NULL},
         {DISPID_UNKNOWN}
     };
+    static const DISPID ie8_prop_hooks_exclude_list[] = {
+        DISPID_IHTMLELEMENT_CLASSNAME,
+        DISPID_IHTMLELEMENT_STYLE,
+        DISPID_UNKNOWN
+    };
+    static const generic_prop_hooks_t ie8_prop_hooks = {
+        ie8_ro_prop_hook,
+        ie8_prop_hooks_exclude_list
+    };

HTMLDOMNode_init_dispex_info(info, mode);

@@ -6715,6 +6765,8 @@ void HTMLElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
         dispex_info_add_interface(info, IHTMLElement7_tid, NULL);
         dispex_info_add_interface(info, IWineHTMLElementPrivate_tid, NULL);
     }
+
+    return (mode == COMPAT_MODE_IE8) ? &ie8_prop_hooks : NULL;
 }

static const tid_t HTMLElement_iface_tids[] = {
diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c
index 3f545bb..bab792b 100644
--- a/dlls/mshtml/htmlevent.c
+++ b/dlls/mshtml/htmlevent.c
@@ -3314,8 +3314,8 @@ static HRESULT WINAPI EventTarget_dispatchEvent(IEventTarget *iface, IDOMEvent *
     return dispatch_event_object(This, event, DISPATCH_STANDARD, result);
 }

-static HRESULT IEventTarget_addEventListener_hook(DispatchEx *dispex, LCID lcid, WORD flags,
-        DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
+static HRESULT IEventTarget_addEventListener_hook(DispatchEx *dispex, DISPID id, BSTR name, LCID lcid,
+        WORD flags, DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
 {
     /* If only two arguments were given, implicitly set capture to false */
     if((flags & DISPATCH_METHOD) && dp->cArgs == 2 && !dp->cNamedArgs) {
@@ -3335,8 +3335,8 @@ static HRESULT IEventTarget_addEventListener_hook(DispatchEx *dispex, LCID lcid,
     return S_FALSE; /* fallback to default */
 }

-static HRESULT IEventTarget_removeEventListener_hook(DispatchEx *dispex, LCID lcid, WORD flags,
-        DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
+static HRESULT IEventTarget_removeEventListener_hook(DispatchEx *dispex, DISPID id, BSTR name, LCID lcid,
+        WORD flags, DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
 {
     /* If only two arguments were given, implicitly set capture to false */
     if((flags & DISPATCH_METHOD) && dp->cArgs == 2 && !dp->cNamedArgs) {
@@ -3413,7 +3413,7 @@ HRESULT EventTarget_QI(EventTarget *event_target, REFIID riid, void **ppv)
     return E_NOINTERFACE;
 }

-void EventTarget_init_dispex_info(dispex_data_t *dispex_info, compat_mode_t compat_mode)
+const generic_prop_hooks_t *EventTarget_init_dispex_info(dispex_data_t *dispex_info, compat_mode_t compat_mode)
 {
     static const dispex_hook_t IEventTarget_hooks[] = {
         {DISPID_IEVENTTARGET_ADDEVENTLISTENER, IEventTarget_addEventListener_hook},
@@ -3423,6 +3423,7 @@ void EventTarget_init_dispex_info(dispex_data_t *dispex_info, compat_mode_t comp

if(compat_mode >= COMPAT_MODE_IE9)
         dispex_info_add_interface(dispex_info, IEventTarget_tid, IEventTarget_hooks);
+    return NULL;
 }

static int event_id_cmp(const void *key, const struct wine_rb_entry *entry)
diff --git a/dlls/mshtml/htmlnode.c b/dlls/mshtml/htmlnode.c
index 418c958..3a2e746 100644
--- a/dlls/mshtml/htmlnode.c
+++ b/dlls/mshtml/htmlnode.c
@@ -1432,12 +1432,12 @@ static HRESULT HTMLDOMNode_clone(HTMLDOMNode *This, nsIDOMNode *nsnode, HTMLDOMN
     return create_node(This->doc, nsnode, ret);
 }

-void HTMLDOMNode_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
+const generic_prop_hooks_t *HTMLDOMNode_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     if(mode >= COMPAT_MODE_IE9)
         dispex_info_add_interface(info, IHTMLDOMNode3_tid, NULL);

-    EventTarget_init_dispex_info(info, mode);
+    return EventTarget_init_dispex_info(info, mode);
 }

static const cpc_entry_t HTMLDOMNode_cpc[] = {{NULL}};
diff --git a/dlls/mshtml/htmlstyle.c b/dlls/mshtml/htmlstyle.c
index b00b9f8..7ca7367 100644
--- a/dlls/mshtml/htmlstyle.c
+++ b/dlls/mshtml/htmlstyle.c
@@ -9983,12 +9983,13 @@ static HRESULT CSSStyle_get_dispid(DispatchEx *dispex, BSTR name, DWORD flags, D
     return DISP_E_UNKNOWNNAME;
 }

-void CSSStyle_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
+const generic_prop_hooks_t *CSSStyle_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     if(mode >= COMPAT_MODE_IE9)
         dispex_info_add_interface(info, IHTMLCSSStyleDeclaration_tid, NULL);
     if(mode >= COMPAT_MODE_IE10)
         dispex_info_add_interface(info, IHTMLCSSStyleDeclaration2_tid, NULL);
+    return NULL;
 }

const dispex_static_data_vtbl_t CSSStyle_dispex_vtbl = {
diff --git a/dlls/mshtml/htmlstyle.h b/dlls/mshtml/htmlstyle.h
index 25f87c8..59fc281 100644
--- a/dlls/mshtml/htmlstyle.h
+++ b/dlls/mshtml/htmlstyle.h
@@ -155,7 +155,7 @@ HRESULT create_computed_style(nsIDOMCSSStyleDeclaration*,compat_mode_t,IHTMLCSSS
 void init_css_style(CSSStyle*,nsIDOMCSSStyleDeclaration*,style_qi_t,
                     dispex_static_data_t*,compat_mode_t) DECLSPEC_HIDDEN;

-void CSSStyle_init_dispex_info(dispex_data_t *info, compat_mode_t mode) DECLSPEC_HIDDEN;
+const generic_prop_hooks_t *CSSStyle_init_dispex_info(dispex_data_t *info, compat_mode_t mode) DECLSPEC_HIDDEN;
 extern const dispex_static_data_vtbl_t CSSStyle_dispex_vtbl DECLSPEC_HIDDEN;

HRESULT get_style_property(CSSStyle*,styleid_t,BSTR*) DECLSPEC_HIDDEN;
diff --git a/dlls/mshtml/htmlstyleelem.c b/dlls/mshtml/htmlstyleelem.c
index cf57547..82ef2d7 100644
--- a/dlls/mshtml/htmlstyleelem.c
+++ b/dlls/mshtml/htmlstyleelem.c
@@ -420,21 +420,21 @@ static void HTMLStyleElement_unlink(HTMLDOMNode *iface)
     }
 }

-static void HTMLStyleElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
+static const generic_prop_hooks_t *HTMLStyleElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     static const dispex_hook_t ie11_hooks[] = {
         {DISPID_IHTMLSTYLEELEMENT_READYSTATE, NULL},
         {DISPID_IHTMLSTYLEELEMENT_STYLESHEET, NULL},
         {DISPID_UNKNOWN}
     };
-
-    HTMLElement_init_dispex_info(info, mode);
+    const generic_prop_hooks_t *prop_hooks = HTMLElement_init_dispex_info(info, mode);

dispex_info_add_interface(info, IHTMLStyleElement_tid,
                               mode >= COMPAT_MODE_IE11 ? ie11_hooks : NULL);

if(mode >= COMPAT_MODE_IE9)
         dispex_info_add_interface(info, IHTMLStyleElement2_tid, NULL);
+    return prop_hooks;
 }

static const NodeImplVtbl HTMLStyleElementImplVtbl = {
diff --git a/dlls/mshtml/htmlstylesheet.c b/dlls/mshtml/htmlstylesheet.c
index c31c780..4f38c12 100644
--- a/dlls/mshtml/htmlstylesheet.c
+++ b/dlls/mshtml/htmlstylesheet.c
@@ -1140,10 +1140,11 @@ static const IHTMLStyleSheet4Vtbl HTMLStyleSheet4Vtbl = {
     HTMLStyleSheet4_deleteRule,
 };

-static void HTMLStyleSheet_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
+static const generic_prop_hooks_t *HTMLStyleSheet_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     if(mode >= COMPAT_MODE_IE9)
         dispex_info_add_interface(info, IHTMLStyleSheet4_tid, NULL);
+    return NULL;
 }

static const tid_t HTMLStyleSheet_iface_tids[] = {
diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c
index 635b046..7298b03 100644
--- a/dlls/mshtml/htmlwindow.c
+++ b/dlls/mshtml/htmlwindow.c
@@ -3734,7 +3734,7 @@ static void HTMLWindow_bind_event(DispatchEx *dispex, eventid_t eid)
     ensure_doc_nsevent_handler(This->doc, NULL, eid);
 }

-static void HTMLWindow_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode)
+static const generic_prop_hooks_t *HTMLWindow_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode)
 {
     if(compat_mode >= COMPAT_MODE_IE9)
         dispex_info_add_interface(info, IHTMLWindow7_tid, NULL);
@@ -3744,7 +3744,7 @@ static void HTMLWindow_init_dispex_info(dispex_data_t *info, compat_mode_t compa
         dispex_info_add_interface(info, IWineHTMLWindowPrivate_tid, NULL);

dispex_info_add_interface(info, IHTMLWindow5_tid, NULL);
-    EventTarget_init_dispex_info(info, compat_mode);
+    return EventTarget_init_dispex_info(info, compat_mode);
 }

static IHTMLEventObj *HTMLWindow_set_current_event(DispatchEx *dispex, IHTMLEventObj *event)
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 9f10d68..7683b73 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -319,6 +319,9 @@ typedef struct dispex_dynamic_data_t dispex_dynamic_data_t;

typedef struct DispatchEx DispatchEx;

+typedef HRESULT (*dispex_hook_invoke_t)(DispatchEx*,DISPID,BSTR,LCID,WORD,DISPPARAMS*,
+                                        VARIANT*,EXCEPINFO*,IServiceProvider*);
+
 typedef struct {
     HRESULT (*value)(DispatchEx*,LCID,WORD,DISPPARAMS*,VARIANT*,EXCEPINFO*,IServiceProvider*);
     HRESULT (*get_dispid)(DispatchEx*,BSTR,DWORD,DISPID*);
@@ -327,19 +330,22 @@ typedef struct {
     HRESULT (*populate_props)(DispatchEx*);
 } dispex_static_data_vtbl_t;

+typedef struct {
+    dispex_hook_invoke_t ro_prop_hook;
+
+    const DISPID *exclude_list;
+} generic_prop_hooks_t;
+
 typedef struct {
     const WCHAR *name;
     const dispex_static_data_vtbl_t *vtbl;
     const tid_t disp_tid;
     const tid_t* const iface_tids;
-    void (*init_info)(dispex_data_t*,compat_mode_t);
+    const generic_prop_hooks_t* (*init_info)(dispex_data_t*,compat_mode_t);
     dispex_data_t *info_cache[COMPAT_MODE_CNT];
     dispex_data_t *delayed_init_info;
 } dispex_static_data_t;

-typedef HRESULT (*dispex_hook_invoke_t)(DispatchEx*,LCID,WORD,DISPPARAMS*,VARIANT*,
-                                        EXCEPINFO*,IServiceProvider*);
-
 typedef struct {
     DISPID dispid;
     dispex_hook_invoke_t invoke;
@@ -1134,18 +1140,18 @@ void HTMLElement_Init(HTMLElement*,HTMLDocumentNode*,nsIDOMElement*,dispex_stati

void EventTarget_Init(EventTarget*,IUnknown*,dispex_static_data_t*,compat_mode_t) DECLSPEC_HIDDEN;
 HRESULT EventTarget_QI(EventTarget*,REFIID,void**) DECLSPEC_HIDDEN;
-void EventTarget_init_dispex_info(dispex_data_t*,compat_mode_t) DECLSPEC_HIDDEN;
+const generic_prop_hooks_t *EventTarget_init_dispex_info(dispex_data_t*,compat_mode_t) DECLSPEC_HIDDEN;

HRESULT HTMLDOMNode_QI(HTMLDOMNode*,REFIID,void**) DECLSPEC_HIDDEN;
 void HTMLDOMNode_destructor(HTMLDOMNode*) DECLSPEC_HIDDEN;
-void HTMLDOMNode_init_dispex_info(dispex_data_t*,compat_mode_t) DECLSPEC_HIDDEN;
+const generic_prop_hooks_t *HTMLDOMNode_init_dispex_info(dispex_data_t*,compat_mode_t) DECLSPEC_HIDDEN;

HRESULT HTMLElement_QI(HTMLDOMNode*,REFIID,void**) DECLSPEC_HIDDEN;
 void HTMLElement_destructor(HTMLDOMNode*) DECLSPEC_HIDDEN;
 HRESULT HTMLElement_clone(HTMLDOMNode*,nsIDOMNode*,HTMLDOMNode**) DECLSPEC_HIDDEN;
 HRESULT HTMLElement_get_attr_col(HTMLDOMNode*,HTMLAttributeCollection**) DECLSPEC_HIDDEN;
 HRESULT HTMLElement_handle_event(HTMLDOMNode*,DWORD,nsIDOMEvent*,BOOL*) DECLSPEC_HIDDEN;
-void HTMLElement_init_dispex_info(dispex_data_t*,compat_mode_t) DECLSPEC_HIDDEN;
+const generic_prop_hooks_t *HTMLElement_init_dispex_info(dispex_data_t*,compat_mode_t) DECLSPEC_HIDDEN;

HRESULT get_node(nsIDOMNode*,BOOL,HTMLDOMNode**) DECLSPEC_HIDDEN;
 HRESULT get_element(nsIDOMElement*,HTMLElement**) DECLSPEC_HIDDEN;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 2ac8823..d5fee5f 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1333,7 +1333,6 @@ sync_test("elem_attr", function() {
                 r = elem.getAttribute(name);
                 ok(r === "string", name + " attr after setAttribute = " + r);
                 eval("r = elem." + name + ";");
-                todo_wine.
                 ok(r === "string", "elem." + name + " after setAttribute = " + r);

r = elem.removeAttribute(name);
@@ -1357,7 +1356,6 @@ sync_test("elem_attr", function() {
         r = elem.style;
         ok(r === style, "removed elem.style = " + r);
         r = elem.getAttribute("style");
-        todo_wine_if(v === 8).
         ok(r === (v < 8 ? style : null), "style attr after removal = " + r);
         elem.setAttribute("style", "opacity: 1.0");
         r = elem.getAttribute("style");
diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c
index 56e18d0..ca90542 100644
--- a/dlls/mshtml/xmlhttprequest.c
+++ b/dlls/mshtml/xmlhttprequest.c
@@ -475,8 +475,8 @@ static HRESULT WINAPI HTMLXMLHttpRequest_abort(IHTMLXMLHttpRequest *iface)
     return S_OK;
 }

-static HRESULT HTMLXMLHttpRequest_open_hook(DispatchEx *dispex, LCID lcid, WORD flags,
-        DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
+static HRESULT HTMLXMLHttpRequest_open_hook(DispatchEx *dispex, DISPID id, BSTR name, LCID lcid,
+        WORD flags, DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
 {
     /* If only two arguments were given, implicitly set async to false */
     if((flags & DISPATCH_METHOD) && dp->cArgs == 2 && !dp->cNamedArgs) {
@@ -870,15 +870,16 @@ static void HTMLXMLHttpRequest_bind_event(DispatchEx *dispex, eventid_t eid)
         This->event_listener->load_event = TRUE;
 }

-static void HTMLXMLHttpRequest_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode)
+static const generic_prop_hooks_t *HTMLXMLHttpRequest_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode)
 {
     static const dispex_hook_t xhr_hooks[] = {
         {DISPID_IHTMLXMLHTTPREQUEST_OPEN, HTMLXMLHttpRequest_open_hook},
         {DISPID_UNKNOWN}
     };
+    const generic_prop_hooks_t *prop_hooks = EventTarget_init_dispex_info(info, compat_mode);

-    EventTarget_init_dispex_info(info, compat_mode);
     dispex_info_add_interface(info, IHTMLXMLHttpRequest_tid, compat_mode >= COMPAT_MODE_IE10 ? xhr_hooks : NULL);
+    return prop_hooks;
 }

static event_target_vtbl_t HTMLXMLHttpRequest_event_target_vtbl = {

[PATCH 11/14] mshtml: Search attributes first when retrieving read-only builtin props in IE8 mode.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102005

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
htmldoc.c:3084: Test failed: Incorrect error code: -2146697211
htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/"
htmldoc.c:5861: Test failed: expected OnChanged_1012
htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV
htmldoc.c:5864: Test failed: expected Exec_SETTITLE
htmldoc.c:5905: Test failed: expected FireNavigateComplete2

=== w10pro64_he (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w10pro64_ja (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

[PATCH 12/14] mshtml: Fix getAttribute for props that weren't set yet in IE8 mode.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/dispex.c              |  2 +-
 dlls/mshtml/htmlelem.c            | 57 ++++++++++++++++++++++++++-----
 dlls/mshtml/mshtml_private.h      |  1 +
 dlls/mshtml/tests/documentmode.js |  1 -
 4 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index b30ca97..7824f6f 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -535,7 +535,7 @@ static dispex_data_t *preprocess_dispex_data(dispex_static_data_t *desc, compat_
                     continue;
             }

-            func->hook = func->put_vtbl_off ? NULL : prop_hooks->ro_prop_hook;
+            func->hook = func->put_vtbl_off ? prop_hooks->rw_prop_hook : prop_hooks->ro_prop_hook;
         }
     }

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index edc0e8e..62bf2fa 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1219,6 +1219,7 @@ static HRESULT WINAPI HTMLElement_getAttribute(IHTMLElement *iface, BSTR strAttr
     nsresult nsres;
     DISPID dispid;
     HRESULT hres;
+    cpp_bool b;

TRACE("(%p)->(%s %08x %p)\n", This, debugstr_w(strAttributeName), lFlags, AttributeValue);

@@ -1251,18 +1252,26 @@ static HRESULT WINAPI HTMLElement_getAttribute(IHTMLElement *iface, BSTR strAttr
     }

if(This->dom_element && compat_mode >= COMPAT_MODE_IE8 && get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
+        nsAString_InitDepend(&name_str, strAttributeName);
+
         if(is_readonly_builtin(&This->node.event_target.dispex, dispid)) {
-            nsAString_InitDepend(&name_str, strAttributeName);
             nsAString_InitDepend(&value_str, NULL);
             nsres = nsIDOMElement_GetAttribute(This->dom_element, &name_str, &value_str);
             nsAString_Finish(&name_str);
             return return_nsstr_variant(nsres, &value_str, 0, AttributeValue);
         }

-        hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
-        if(SUCCEEDED(hres) && V_VT(AttributeValue) != VT_BSTR) {
-            VariantClear(AttributeValue);
+        nsres = nsIDOMElement_HasAttribute(This->dom_element, &name_str, &b);
+        nsAString_Finish(&name_str);
+        if(NS_SUCCEEDED(nsres) && b) {
+            hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
+            if(SUCCEEDED(hres) && V_VT(AttributeValue) != VT_BSTR) {
+                VariantClear(AttributeValue);
+                V_VT(AttributeValue) = VT_NULL;
+            }
+        }else {
             V_VT(AttributeValue) = VT_NULL;
+            hres = S_OK;
         }
         return hres;
     }
@@ -1293,6 +1302,7 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
 {
     HTMLElement *This = impl_from_IHTMLElement(iface);
     compat_mode_t compat_mode = dispex_compat_mode(&This->node.event_target.dispex);
+    VARIANT_BOOL tmp;
     DISPID id;
     HRESULT hres;

@@ -1305,10 +1315,12 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA

if(compat_mode >= COMPAT_MODE_IE8) {
         *pfSuccess = element_has_attribute(This, strAttributeName);
-        if(*pfSuccess == VARIANT_TRUE)
-            return element_remove_attribute(This, strAttributeName);
         if(compat_mode >= COMPAT_MODE_IE9)
-            return S_OK;
+            return (*pfSuccess == VARIANT_TRUE) ? element_remove_attribute(This, strAttributeName) : S_OK;
+
+        /* remove the prop without influencing the return value */
+        if(*pfSuccess == VARIANT_TRUE)
+            pfSuccess = &tmp;
     }

hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
@@ -1341,7 +1353,10 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
         return S_OK;
     }

-    return remove_attribute(&This->node.event_target.dispex, id, pfSuccess);
+    hres = remove_attribute(&This->node.event_target.dispex, id, pfSuccess);
+    if(compat_mode >= COMPAT_MODE_IE8)
+        element_remove_attribute(This, strAttributeName);
+    return hres;
 }

static HRESULT WINAPI HTMLElement_put_className(IHTMLElement *iface, BSTR v)
@@ -6731,6 +6746,31 @@ static HRESULT ie8_ro_prop_hook(DispatchEx *dispex, DISPID id, BSTR name, LCID l
     return S_OK;
 }

+static HRESULT ie8_rw_prop_hook(DispatchEx *dispex, DISPID id, BSTR name, LCID lcid, WORD flags,
+        DISPPARAMS *dp, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
+{
+    HTMLElement *This = impl_from_DispatchEx(dispex);
+    nsAString name_str, val_str;
+    nsresult nsres;
+    cpp_bool b;
+
+    if(flags != DISPATCH_PROPERTYPUT || dp->cArgs != 1 || dp->cNamedArgs > 1 || V_VT(dp->rgvarg) != VT_BSTR ||
+       (dp->cNamedArgs == 1 && *dp->rgdispidNamedArgs != DISPID_PROPERTYPUT) || !This->dom_element)
+        return S_FALSE;
+
+    /* make sure attribute is set if it doesn't exist, for getAttribute checks */
+    nsAString_InitDepend(&name_str, name);
+    nsres = nsIDOMElement_HasAttribute(This->dom_element, &name_str, &b);
+    if(NS_SUCCEEDED(nsres) && !b) {
+        nsAString_InitDepend(&val_str, L"");
+        nsIDOMElement_SetAttribute(This->dom_element, &name_str, &val_str);
+        nsAString_Finish(&val_str);
+    }
+    nsAString_Finish(&name_str);
+
+    return S_FALSE;
+}
+
 const generic_prop_hooks_t *HTMLElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
     static const dispex_hook_t elem2_ie11_hooks[] = {
@@ -6745,6 +6785,7 @@ const generic_prop_hooks_t *HTMLElement_init_dispex_info(dispex_data_t *info, co
     };
     static const generic_prop_hooks_t ie8_prop_hooks = {
         ie8_ro_prop_hook,
+        ie8_rw_prop_hook,
         ie8_prop_hooks_exclude_list
     };

diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 7683b73..afe75d6 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -332,6 +332,7 @@ typedef struct {

typedef struct {
     dispex_hook_invoke_t ro_prop_hook;
+    dispex_hook_invoke_t rw_prop_hook;

const DISPID *exclude_list;
 } generic_prop_hooks_t;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index d5fee5f..4f87316 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1251,7 +1251,6 @@ sync_test("elem_attr", function() {
             var val = props[i].length > 2 ? props[i][2] : "test";

r = elem.getAttribute(name);
-            todo_wine_if(v === 8 && props[i][0].substring(0, 2) !== "on").
             ok(r === (v < 8 && props[i].length > 1 ? props[i][1] : null), name + " attr before set = " + r);
             eval("elem." + name + " = "" + val + ""; r = elem." + name + ";");
             ok(r === val, "elem." + name + " = " + r);

[PATCH 12/14] mshtml: Fix getAttribute for props that weren't set yet in IE8 mode.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102006

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
events.c:1089: Test failed: unexpected call img_onerror
events: Timeout

=== w10pro64 (32 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w10pro64_zh_CN (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

[PATCH 13/14] mshtml: Clear the builtin prop even if removeAttribute returns false on IE8 mode.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/dispex.c              | 13 ++++++++++++-
 dlls/mshtml/tests/documentmode.js |  1 -
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 7824f6f..46eb687 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1460,6 +1460,8 @@ static BOOL is_string_builtin(DispatchEx *This, func_info_t *func)

HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
 {
+    VARIANT_BOOL tmp;
+
     switch(get_dispid_type(id)) {
     case DISPEXPROP_CUSTOM:
         FIXME("DISPEXPROP_CUSTOM not supported\n");
@@ -1510,11 +1512,20 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
             return S_OK;
         }

-        if(!func->put_vtbl_off || !is_string_builtin(This, func)) {
+        if(!func->put_vtbl_off) {
             *success = VARIANT_FALSE;
             return S_OK;
         }

+        if(!is_string_builtin(This, func)) {
+            *success = VARIANT_FALSE;
+            if(dispex_compat_mode(This) < COMPAT_MODE_IE8)
+                return S_OK;
+
+            /* remove the prop without influencing the return value */
+            success = &tmp;
+        }
+
         V_VT(&var) = VT_EMPTY;
         hres = builtin_propput(This, func, &dp, NULL);
         if(hres == E_NOTIMPL) {
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 4f87316..5bf5f08 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1157,7 +1157,6 @@ sync_test("elem_attr", function() {
     ok(r === (v < 8 ? func : null), "onclick attr = " + r);
     r = elem.removeAttribute("onclick");
     ok(r === (v < 9 ? false : undefined), "removeAttribute returned " + r);
-    todo_wine_if(v === 8).
     ok(elem.onclick === (v != 8 ? func : null), "removed onclick = " + elem.onclick);

elem.onclick_test = func;

[PATCH 13/14] mshtml: Clear the builtin prop even if removeAttribute returns false on IE8 mode.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102007

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
events.c:1089: Test failed: unexpected call img_onerror
events: Timeout

=== w8adm (32 bit report) ===

mshtml:
htmldoc.c:3084: Test failed: Incorrect error code: -2146697211
htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/"
htmldoc.c:5861: Test failed: expected OnChanged_1012
htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV
htmldoc.c:5864: Test failed: expected Exec_SETTITLE
htmldoc.c:5905: Test failed: expected FireNavigateComplete2

[PATCH 14/14] jscript: Return the correct string for Object.toString(null) in ES5+ mode.

[Gabriel Ivăncescu]

This has to be special cased for Object_toString because null is otherwise
replaced by the global object for any other case.

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/jscript/function.c           | 10 ++++++++++
 dlls/jscript/jscript.h            |  1 +
 dlls/jscript/object.c             |  2 +-
 dlls/mshtml/tests/documentmode.js |  2 +-
 dlls/mshtml/tests/es5.js          |  1 -
 5 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c
index 318d6be..8ea6a3f 100644
--- a/dlls/jscript/function.c
+++ b/dlls/jscript/function.c
@@ -604,6 +604,16 @@ static HRESULT NativeFunction_call(script_ctx_t *ctx, FunctionInstance *func, ID

if(this_disp)
         set_disp(&vthis, this_disp);
+    else if(function->proc == Object_toString && ctx->version >= SCRIPTLANGUAGEVERSION_ES5) {
+        jsstr_t *ret;
+        if(!r)
+            return S_OK;
+        ret = jsstr_alloc(L"[object Null]");
+        if(!ret)
+            return E_OUTOFMEMORY;
+        *r = jsval_string(ret);
+        return S_OK;
+    }
     else
         set_disp(&vthis, lookup_global_host(ctx));

diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h
index 69897cd..e0056f5 100644
--- a/dlls/jscript/jscript.h
+++ b/dlls/jscript/jscript.h
@@ -515,6 +515,7 @@ BOOL bool_obj_value(jsdisp_t*) DECLSPEC_HIDDEN;
 unsigned array_get_length(jsdisp_t*) DECLSPEC_HIDDEN;

HRESULT JSGlobal_eval(script_ctx_t*,vdisp_t*,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN;
+HRESULT Object_toString(script_ctx_t*,vdisp_t*,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN;

static inline BOOL is_class(jsdisp_t *jsdisp, jsclass_t class)
 {
diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c
index 89684f5..1451c98 100644
--- a/dlls/jscript/object.c
+++ b/dlls/jscript/object.c
@@ -24,7 +24,7 @@

WINE_DEFAULT_DEBUG_CHANNEL(jscript);

-static HRESULT Object_toString(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv,
+HRESULT Object_toString(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv,
         jsval_t *r)
 {
     jsdisp_t *jsdisp;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 5bf5f08..205e945 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1200,7 +1200,7 @@ sync_test("elem_attr", function() {
     if(v < 11)  /* IE11 returns an empty function, which we can't check */
         ok(elem.ondblclick === (v < 8 ? arr : null), "removed ondblclick = " + elem.ondblclick);
     r = Object.prototype.toString.call(elem.ondblclick);
-    todo_wine_if(v >= 9).
+    todo_wine_if(v >= 11).
     ok(r === (v < 8 ? "[object Array]" : (v < 9 ? "[object Object]" : (v < 11 ? "[object Null]" : "[object Function]"))),
         "removed ondblclick Object.toString returned " + r);

diff --git a/dlls/mshtml/tests/es5.js b/dlls/mshtml/tests/es5.js
index 81c06a4..20ae374 100644
--- a/dlls/mshtml/tests/es5.js
+++ b/dlls/mshtml/tests/es5.js
@@ -784,7 +784,6 @@ sync_test("toString", function() {
     todo_wine.
     ok(tmp === "[object Window]", "toString.call(null) = " + tmp);
     tmp = Object.prototype.toString.call(null);
-    todo_wine.
     ok(tmp === "[object Null]", "toString.call(null) = " + tmp);
     tmp = Object.prototype.toString.call(undefined);
     todo_wine.

[PATCH 14/14] jscript: Return the correct string for Object.toString(null) in ES5+ mode.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102008

Your paranoid android.

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

[Jacek Caban]

Hi Gabriel,

On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
> index 96a776d..bddb0e3 100644
> --- a/dlls/mshtml/dispex.c
> +++ b/dlls/mshtml/dispex.c
> @@ -1466,6 +1466,11 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
>       V_VT(&var) = VT_EMPTY;
>       hres = builtin_propput(This, func, &dp, NULL);
> -
>    if(hres == E_NOTIMPL) {
> -
>        /* Setting event handlers to VT_EMPTY fails in quirks mode */
> -
>        V_VT(&var) = VT_NULL;
> -
>        hres = builtin_propput(This, func, &dp, NULL);
> -
>    }
>     if(FAILED(hres))
>         return hres;
>
> diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
> index d566223..c197f9b 100644
> --- a/dlls/mshtml/tests/documentmode.js
> +++ b/dlls/mshtml/tests/documentmode.js
> @@ -1087,6 +1087,46 @@ sync_test("elem_attr", function() {
>       ok(r === "cls2", "class attr = " + r);
>       r = elem.getAttribute("className");
>       ok(r === "cls3", "className attr = " + r);
> -
> - var func = function() { };
> - elem.onclick = func;
> - ok(elem.onclick === func, "onclick = " + elem.onclick);
> - r = elem.getAttribute("onclick");
> - ok(r === (v < 8 ? func : null), "onclick attr = " + r);
> - r = elem.removeAttribute("onclick");
> - todo_wine_if(v < 9).
> - ok(r === (v < 9 ? false : undefined), "removeAttribute returned " + r);
> - todo_wine_if(v < 9).
> - ok(elem.onclick === (v != 8 ? func : null), "removed onclick = " + elem.onclick);

This test shows that removeAttribute() should not work in this case 
(although it should not throw), so making it work does not seem right.

Thanks,

Jacek

[Gabriel Ivăncescu]

Hi Jacek,

On 16/11/2021 16:57, Jacek Caban wrote:
> Hi Gabriel,
>
> On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> > diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
> > index 96a776d..bddb0e3 100644
> > --- a/dlls/mshtml/dispex.c
> > +++ b/dlls/mshtml/dispex.c
> > @@ -1466,6 +1466,11 @@ HRESULT remove_attribute(DispatchEx *This, 
> > DISPID id, VARIANT_BOOL *success)
> >           V_VT(&var) = VT_EMPTY;
> >           hres = builtin_propput(This, func, &dp, NULL);
> > +        if(hres == E_NOTIMPL) {
> > +            /* Setting event handlers to VT_EMPTY fails in quirks 
> > mode */
> > +            V_VT(&var) = VT_NULL;
> > +            hres = builtin_propput(This, func, &dp, NULL);
> > +        }
> >           if(FAILED(hres))
> >               return hres;
> > diff --git a/dlls/mshtml/tests/documentmode.js 
> > b/dlls/mshtml/tests/documentmode.js
> > index d566223..c197f9b 100644
> > --- a/dlls/mshtml/tests/documentmode.js
> > +++ b/dlls/mshtml/tests/documentmode.js
> > @@ -1087,6 +1087,46 @@ sync_test("elem_attr", function() {
> >       ok(r === "cls2", "class attr = " + r);
> >       r = elem.getAttribute("className");
> >       ok(r === "cls3", "className attr = " + r);
> > -
> >
> > +    var func = function() { };
> > +    elem.onclick = func;
> > +    ok(elem.onclick === func, "onclick = " + elem.onclick);
> > +    r = elem.getAttribute("onclick");
> > +    ok(r === (v < 8 ? func : null), "onclick attr = " + r);
> > +    r = elem.removeAttribute("onclick");
> > +    todo_wine_if(v < 9).
> > +    ok(r === (v < 9 ? false : undefined), "removeAttribute returned "
> > - r);
> >
> > +    todo_wine_if(v < 9).
> > +    ok(elem.onclick === (v != 8 ? func : null), "removed onclick = "
> > - elem.onclick);
>
> This test shows that removeAttribute() should not work in this case 
> (although it should not throw), so making it work does not seem right.
>
> Thanks,
>
> Jacek

Well, in subsequent tests, it does work. The reason it doesn't work here 
is because the attribute is set to a non-string value (there's tests + 
fixes later in the series for that). Should I move the tests with 
todo_wine then?

[Jacek Caban]

On 16/11/2021 16:48, Gabriel Ivăncescu wrote:
> Hi Jacek,
>
> On 16/11/2021 16:57, Jacek Caban wrote:
> > Hi Gabriel,
> >
> > On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> > > diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
> > > index 96a776d..bddb0e3 100644
> > > --- a/dlls/mshtml/dispex.c
> > > +++ b/dlls/mshtml/dispex.c
> > > @@ -1466,6 +1466,11 @@ HRESULT remove_attribute(DispatchEx *This, 
> > > DISPID id, VARIANT_BOOL *success)
> > >           V_VT(&var) = VT_EMPTY;
> > >           hres = builtin_propput(This, func, &dp, NULL);
> > > +        if(hres == E_NOTIMPL) {
> > > +            /* Setting event handlers to VT_EMPTY fails in quirks 
> > > mode */
> > > +            V_VT(&var) = VT_NULL;
> > > +            hres = builtin_propput(This, func, &dp, NULL);
> > > +        }
> > >           if(FAILED(hres))
> > >               return hres;
> > > diff --git a/dlls/mshtml/tests/documentmode.js 
> > > b/dlls/mshtml/tests/documentmode.js
> > > index d566223..c197f9b 100644
> > > --- a/dlls/mshtml/tests/documentmode.js
> > > +++ b/dlls/mshtml/tests/documentmode.js
> > > @@ -1087,6 +1087,46 @@ sync_test("elem_attr", function() {
> > >       ok(r === "cls2", "class attr = " + r);
> > >       r = elem.getAttribute("className");
> > >       ok(r === "cls3", "className attr = " + r);
> > > -
> > >
> > > +    var func = function() { };
> > > +    elem.onclick = func;
> > > +    ok(elem.onclick === func, "onclick = " + elem.onclick);
> > > +    r = elem.getAttribute("onclick");
> > > +    ok(r === (v < 8 ? func : null), "onclick attr = " + r);
> > > +    r = elem.removeAttribute("onclick");
> > > +    todo_wine_if(v < 9).
> > > +    ok(r === (v < 9 ? false : undefined), "removeAttribute returned 
> > > " + r);
> > > +    todo_wine_if(v < 9).
> > > +    ok(elem.onclick === (v != 8 ? func : null), "removed onclick = 
> > > " + elem.onclick);
> >
> > This test shows that removeAttribute() should not work in this case 
> > (although it should not throw), so making it work does not seem right.
> >
> > Thanks,
> >
> > Jacek
>
> Well, in subsequent tests, it does work. The reason it doesn't work 
> here is because the attribute is set to a non-string value (there's 
> tests + fixes later in the series for that). Should I move the tests 
> with todo_wine then?

I would hope for something cleaner instead. So far this patch and patch 
5 is backed only on event handler tests and event handlers are likely to 
behave differently than other properties. I can't be sure without 
trying, but for those tests it seems to me that you could replace 
throwing by returning false from remove_attribute and try to use 
dispex_get_dprop_ref to clear the value (which would cover string values 
of event handler).

Thanks,

Jacek

[Gabriel Ivăncescu]

On 16/11/2021 18:47, Jacek Caban wrote:
> On 16/11/2021 16:48, Gabriel Ivăncescu wrote:
> > Hi Jacek,
> >
> > On 16/11/2021 16:57, Jacek Caban wrote:
> > > Hi Gabriel,
> > >
> > > On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
> > > > diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
> > > > index 96a776d..bddb0e3 100644
> > > > --- a/dlls/mshtml/dispex.c
> > > > +++ b/dlls/mshtml/dispex.c
> > > > @@ -1466,6 +1466,11 @@ HRESULT remove_attribute(DispatchEx *This, 
> > > > DISPID id, VARIANT_BOOL *success)
> > > >           V_VT(&var) = VT_EMPTY;
> > > >           hres = builtin_propput(This, func, &dp, NULL);
> > > > +        if(hres == E_NOTIMPL) {
> > > > +            /* Setting event handlers to VT_EMPTY fails in quirks 
> > > > mode */
> > > > +            V_VT(&var) = VT_NULL;
> > > > +            hres = builtin_propput(This, func, &dp, NULL);
> > > > +        }
> > > >           if(FAILED(hres))
> > > >               return hres;
> > > > diff --git a/dlls/mshtml/tests/documentmode.js 
> > > > b/dlls/mshtml/tests/documentmode.js
> > > > index d566223..c197f9b 100644
> > > > --- a/dlls/mshtml/tests/documentmode.js
> > > > +++ b/dlls/mshtml/tests/documentmode.js
> > > > @@ -1087,6 +1087,46 @@ sync_test("elem_attr", function() {
> > > >       ok(r === "cls2", "class attr = " + r);
> > > >       r = elem.getAttribute("className");
> > > >       ok(r === "cls3", "className attr = " + r);
> > > > -
> > > >
> > > > +    var func = function() { };
> > > > +    elem.onclick = func;
> > > > +    ok(elem.onclick === func, "onclick = " + elem.onclick);
> > > > +    r = elem.getAttribute("onclick");
> > > > +    ok(r === (v < 8 ? func : null), "onclick attr = " + r);
> > > > +    r = elem.removeAttribute("onclick");
> > > > +    todo_wine_if(v < 9).
> > > > +    ok(r === (v < 9 ? false : undefined), "removeAttribute returned 
> > > > " + r);
> > > > +    todo_wine_if(v < 9).
> > > > +    ok(elem.onclick === (v != 8 ? func : null), "removed onclick = 
> > > > " + elem.onclick);
> > >
> > > This test shows that removeAttribute() should not work in this case 
> > > (although it should not throw), so making it work does not seem right.
> > >
> > > Thanks,
> > >
> > > Jacek
> >
> > Well, in subsequent tests, it does work. The reason it doesn't work 
> > here is because the attribute is set to a non-string value (there's 
> > tests + fixes later in the series for that). Should I move the tests 
> > with todo_wine then?
>
> I would hope for something cleaner instead. So far this patch and patch 
> 5 is backed only on event handler tests and event handlers are likely to 
> behave differently than other properties. I can't be sure without 
> trying, but for those tests it seems to me that you could replace 
> throwing by returning false from remove_attribute and try to use 
> dispex_get_dprop_ref to clear the value (which would cover string values 
> of event handler).
>
> Thanks,
>
> Jacek

Interesting. I considered that at first, but I actually thought that 
using event-handler implementation details would be "less clean". :-)

Your suggestion should be good enough, though. I don't think we'll ever 
know exactly, because the event handlers are the only read-write builtin 
props in quirks modes that use VT_VARIANT instead of something more 
specific, so it doesn't matter in the end.

Also, the toString is (surprisingly) correct, but I'll resend with extra 
tests of course, and move them around earlier where possible.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=101995

Your paranoid android.

=== w8adm (32 bit report) ===

mshtml:
events.c:1089: Test failed: unexpected call img_onerror
events: Timeout

=== w8 (32 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c:350: Test failed: expected Exec_SETTITLE
htmldoc.c:2859: Test failed: unexpected call Exec_SETTITLE

=== w1064_tsign (32 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w1064_tsign (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_ar (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS

=== w10pro64_zh_CN (64 bit report) ===

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS