[PATCH v4 01/12] jscript: Try to_primitive first when getting the default value.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/dispex.c    |  9 +++++++--
 dlls/jscript/tests/run.c | 41 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
index e64a520..cc46ee0 100644
--- a/dlls/jscript/dispex.c
+++ b/dlls/jscript/dispex.c
@@ -1548,7 +1548,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
         V_VT(pvarRes) = VT_EMPTY;
 
     prop = get_prop(This, id);
-    if(!prop || prop->type == PROP_DELETED) {
+    if((!prop || prop->type == PROP_DELETED) && (id != DISPID_VALUE || wFlags != DISPATCH_PROPERTYGET)) {
         TRACE("invalid id\n");
         return DISP_E_MEMBERNOTFOUND;
     }
@@ -1580,7 +1580,12 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
     case DISPATCH_PROPERTYGET: {
         jsval_t r;
 
-        hres = prop_get(This, prop, &r);
+        hres = E_FAIL;
+        if(id == DISPID_VALUE)
+            hres = to_primitive(This->ctx, jsval_obj(This), &r, NO_HINT);
+        if(FAILED(hres))
+            hres = prop_get(This, prop, &r);
+
         if(SUCCEEDED(hres)) {
             hres = jsval_to_variant(r, pvarRes);
             jsval_release(r);
diff --git a/dlls/jscript/tests/run.c b/dlls/jscript/tests/run.c
index ef0f39f..c71e550 100644
--- a/dlls/jscript/tests/run.c
+++ b/dlls/jscript/tests/run.c
@@ -2789,6 +2789,7 @@ static void test_retval(void)
 
 static void test_default_value(void)
 {
+    IActiveScript *script;
     DISPPARAMS dp = {0};
     IDispatch *disp;
     VARIANT v;
@@ -2806,9 +2807,49 @@ static void test_default_value(void)
     {
         ok(V_VT(&v) == VT_BSTR, "V_VT(v) = %d\n", V_VT(&v));
     }
+    VariantClear(&v);
+    IDispatch_Release(disp);
+
+    hres = parse_script_expr(L"var arr = [5]; arr.toString = function() {return \"foo\";}; arr.valueOf = function() {return 42;}; arr", &v, &script);
+    ok(hres == S_OK, "parse_script_expr failed: %08x\n", hres);
+    ok(V_VT(&v) == VT_DISPATCH, "V_VT(v) = %d\n", V_VT(&v));
+    disp = V_DISPATCH(&v);
 
+    V_VT(&v) = VT_EMPTY;
+    hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, 0, DISPATCH_PROPERTYGET, &dp, &v, NULL, NULL);
+    ok(hres == S_OK, "Invoke failed: %08x\n", hres);
+    ok(V_VT(&v) == VT_I4, "V_VT(v) = %d\n", V_VT(&v));
+    ok(V_I4(&v) == 42, "V_I4(v) = %s\n", wine_dbgstr_w(V_BSTR(&v)));
+    IDispatch_Release(disp);
+    close_script(script);
+
+    hres = parse_script_expr(L"var arr = [5]; arr.toString = function() {return \"foo\";}; arr", &v, &script);
+    ok(hres == S_OK, "parse_script_expr failed: %08x\n", hres);
+    ok(V_VT(&v) == VT_DISPATCH, "V_VT(v) = %d\n", V_VT(&v));
+    disp = V_DISPATCH(&v);
+
+    V_VT(&v) = VT_EMPTY;
+    hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, 0, DISPATCH_PROPERTYGET, &dp, &v, NULL, NULL);
+    ok(hres == S_OK, "Invoke failed: %08x\n", hres);
+    ok(V_VT(&v) == VT_BSTR, "V_VT(v) = %d\n", V_VT(&v));
+    ok(!lstrcmpW(V_BSTR(&v), L"foo"), "V_BSTR(v) = %s\n", wine_dbgstr_w(V_BSTR(&v)));
+    VariantClear(&v);
+    IDispatch_Release(disp);
+    close_script(script);
+
+    hres = parse_script_expr(L"var arr = [5]; arr", &v, &script);
+    ok(hres == S_OK, "parse_script_expr failed: %08x\n", hres);
+    ok(V_VT(&v) == VT_DISPATCH, "V_VT(v) = %d\n", V_VT(&v));
+    disp = V_DISPATCH(&v);
+
+    V_VT(&v) = VT_EMPTY;
+    hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, 0, DISPATCH_PROPERTYGET, &dp, &v, NULL, NULL);
+    ok(hres == S_OK, "Invoke failed: %08x\n", hres);
+    ok(V_VT(&v) == VT_BSTR, "V_VT(v) = %d\n", V_VT(&v));
+    ok(!lstrcmpW(V_BSTR(&v), L"5"), "V_BSTR(v) = %s\n", wine_dbgstr_w(V_BSTR(&v)));
     VariantClear(&v);
     IDispatch_Release(disp);
+    close_script(script);
 }
 
 static void test_script_exprs(void)
-- 
2.31.1

[PATCH v4 02/12] 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 <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/nsembed.c             | 41 ++++++++++++----
 dlls/mshtml/tests/documentmode.js | 77 +++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c
index 4224d6a..40e7b4a 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];
+    HRESULT hres;
+    VARIANT var;
 
     switch(V_VT(v)) {
     case VT_NULL:
@@ -1013,19 +1015,42 @@ HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
         nsAString_Init(nsstr, buf);
         break;
 
-    case VT_R8: {
-        VARIANT strv;
-        HRESULT hres;
-
-        V_VT(&strv) = VT_EMPTY;
-        hres = VariantChangeTypeEx(&strv, v, MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT),
+    case VT_R8:
+        V_VT(&var) = VT_EMPTY;
+        hres = VariantChangeTypeEx(&var, v, MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT),
                                    0, VT_BSTR);
         if(FAILED(hres))
             return hres;
 
-        nsAString_Init(nsstr, V_BSTR(&strv));
-        SysFreeString(V_BSTR(&strv));
+        nsAString_Init(nsstr, V_BSTR(&var));
+        SysFreeString(V_BSTR(&var));
         break;
+
+    case VT_DISPATCH: {
+        LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT);
+        IDispatch *disp = V_DISPATCH(v);
+
+        if(!disp) {
+            nsAString_InitDepend(nsstr, NULL);
+            return S_OK;
+        }
+
+        hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, lcid, DISPATCH_PROPERTYGET, NULL, &var, NULL, NULL);
+        if(FAILED(hres))
+            return hres;
+
+        if(V_VT(&var) == VT_BSTR) {
+            nsAString_Init(nsstr, V_BSTR(&var));
+            SysFreeString(V_BSTR(&var));
+            break;
+        }
+        if(V_VT(&var) == (VT_BSTR | VT_BYREF)) {
+            nsAString_Init(nsstr, *V_BSTRREF(&var));
+            break;
+        }
+        hres = variant_to_nsstr(&var, hex_int, nsstr);
+        VariantClear(&var);
+        return hres;
     }
 
     default:
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index d5f33dd..954dafd 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1088,6 +1088,72 @@ 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);
+
+    arr.toString = function() { return 42; }
+    elem.testattr = arr;
+    r = elem.getAttribute("testattr");
+    todo_wine_if(v === 8).
+    ok(r === (v < 8 ? arr : (v < 9 ? "42" : null)), "testattr with custom toString = " + r);
+    elem.setAttribute("testattr", arr);
+    r = elem.getAttribute("testattr");
+    ok(r === (v < 8 ? arr : "42"), "testattr after setAttribute with custom toString = " + r);
+    ok(elem.testattr === arr, "elem.testattr after setAttribute with custom toString = " + elem.testattr);
+    r = elem.removeAttribute("testattr");
+    ok(r === (v < 9 ? true : undefined), "testattr removeAttribute with custom toString returned " + r);
+    todo_wine_if(v === 8).
+    ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr with custom toString = " + elem.testattr);
+
+    arr.valueOf = function() { return "arrval"; }
+    elem.testattr = arr;
+    r = elem.getAttribute("testattr");
+    todo_wine_if(v === 8).
+    ok(r === (v < 8 ? arr : (v < 9 ? "arrval" : null)), "testattr with custom valueOf = " + r);
+    elem.setAttribute("testattr", arr);
+    r = elem.getAttribute("testattr");
+    todo_wine_if(v >= 10).
+    ok(r === (v < 8 ? arr : (v < 10 ? "arrval" : "42")), "testattr after setAttribute with custom valueOf = " + r);
+    ok(elem.testattr === arr, "elem.testattr after setAttribute with custom valueOf = " + elem.testattr);
+    r = elem.removeAttribute("testattr");
+    ok(r === (v < 9 ? true : undefined), "testattr removeAttribute with custom valueOf returned " + r);
+    todo_wine_if(v === 8).
+    ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr with custom valueOf = " + elem.testattr);
+    delete arr.valueOf;
+    delete arr.toString;
+
+    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);
@@ -1136,6 +1202,17 @@ sync_test("elem_attr", function() {
     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");
+    ok(r === (v < 8 ? false : (v < 9 ? true : undefined)), "ondblclick removeAttribute returned " + r);
+    r = Object.prototype.toString.call(elem.ondblclick);
+    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);
+
     elem.setAttribute("ondblclick", "string");
     r = elem.getAttribute("ondblclick");
     ok(r === "string", "ondblclick string = " + r);
-- 
2.31.1

[PATCH v4 02/12] 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=102247

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

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

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

[PATCH v4 02/12] mshtml: Handle VT_DISPATCH in variant_to_nsstr.

[Jacek Caban]

On 11/18/21 7:43 PM, Gabriel Ivăncescu wrote:
> +    case VT_DISPATCH: {
> +        LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT);
> +        IDispatch *disp = V_DISPATCH(v);
> +
> +        if(!disp) {
> +            nsAString_InitDepend(nsstr, NULL);
> +            return S_OK;
> +        }
> +
> +        hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, lcid, DISPATCH_PROPERTYGET, NULL, &var, NULL, NULL);
> +        if(FAILED(hres))
> +            return hres;
> +
> +        if(V_VT(&var) == VT_BSTR) {
> +            nsAString_Init(nsstr, V_BSTR(&var));
> +            SysFreeString(V_BSTR(&var));
> +            break;
> +        }
> +        if(V_VT(&var) == (VT_BSTR | VT_BYREF)) {
> +            nsAString_Init(nsstr, *V_BSTRREF(&var));
> +            break;
> +        }
> +        hres = variant_to_nsstr(&var, hex_int, nsstr);
> +        VariantClear(&var);
> +        return hres;

Could you just use the same implementation as VT_R8 in this case? 
VariantChangeTypeEx should what you need.


Thanks,

Jacek

[PATCH v4 03/12] jscript: Use string hint when retrieving the default value primitive in IE10+.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/jscript/dispex.c             | 3 ++-
 dlls/jscript/jscript.h            | 3 ++-
 dlls/mshtml/script.c              | 5 ++++-
 dlls/mshtml/tests/documentmode.js | 1 -
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
index cc46ee0..e414790 100644
--- a/dlls/jscript/dispex.c
+++ b/dlls/jscript/dispex.c
@@ -1582,7 +1582,8 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc
 
         hres = E_FAIL;
         if(id == DISPID_VALUE)
-            hres = to_primitive(This->ctx, jsval_obj(This), &r, NO_HINT);
+            hres = to_primitive(This->ctx, jsval_obj(This), &r,
+                                This->ctx->version > SCRIPTLANGUAGEVERSION_ES5 ? HINT_STRING : NO_HINT);
         if(FAILED(hres))
             hres = prop_get(This, prop, &r);
 
diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h
index 69897cd..a1c8a00 100644
--- a/dlls/jscript/jscript.h
+++ b/dlls/jscript/jscript.h
@@ -45,7 +45,8 @@
  * This is Wine jscript extension for ES5 and ES6 compatible mode. Allowed only in HTML mode.
  */
 #define SCRIPTLANGUAGEVERSION_ES5  0x102
-#define SCRIPTLANGUAGEVERSION_ES6  0x103
+#define SCRIPTLANGUAGEVERSION_ES5b 0x103
+#define SCRIPTLANGUAGEVERSION_ES6  0x104
 
 typedef struct _jsval_t jsval_t;
 typedef struct _jsstr_t jsstr_t;
diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c
index 28e07a5..cc6a25b 100644
--- a/dlls/mshtml/script.c
+++ b/dlls/mshtml/script.c
@@ -65,7 +65,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(mshtml);
 /* See jscript.h in jscript.dll. */
 #define SCRIPTLANGUAGEVERSION_HTML 0x400
 #define SCRIPTLANGUAGEVERSION_ES5  0x102
-#define SCRIPTLANGUAGEVERSION_ES6  0x103
+#define SCRIPTLANGUAGEVERSION_ES5b 0x103
+#define SCRIPTLANGUAGEVERSION_ES6  0x104
 
 struct ScriptHost {
     IActiveScriptSite              IActiveScriptSite_iface;
@@ -156,6 +157,8 @@ static BOOL init_script_engine(ScriptHost *script_host)
     if(IsEqualGUID(&script_host->guid, &CLSID_JScript)) {
         if(compat_mode >= COMPAT_MODE_IE11)
             script_mode = SCRIPTLANGUAGEVERSION_ES6;
+        else if(compat_mode >= COMPAT_MODE_IE10)
+            script_mode = SCRIPTLANGUAGEVERSION_ES5b;
         else if(compat_mode >= COMPAT_MODE_IE9)
             script_mode = SCRIPTLANGUAGEVERSION_ES5;
         script_mode |= SCRIPTLANGUAGEVERSION_HTML;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 954dafd..38a9545 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1136,7 +1136,6 @@ sync_test("elem_attr", function() {
     ok(r === (v < 8 ? arr : (v < 9 ? "arrval" : null)), "testattr with custom valueOf = " + r);
     elem.setAttribute("testattr", arr);
     r = elem.getAttribute("testattr");
-    todo_wine_if(v >= 10).
     ok(r === (v < 8 ? arr : (v < 10 ? "arrval" : "42")), "testattr after setAttribute with custom valueOf = " + r);
     ok(elem.testattr === arr, "elem.testattr after setAttribute with custom valueOf = " + elem.testattr);
     r = elem.removeAttribute("testattr");
-- 
2.31.1

[PATCH v4 03/12] jscript: Use string hint when retrieving the default value primitive in IE10+.

[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=102248

Your paranoid android.


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

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

=== w10pro64 (64 bit report) ===

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

[PATCH v4 04/12] mshtml: Reset contentEditable to the default when removed (empty string).

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 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 d54f729..e8d108f 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);
 
-- 
2.31.1

[PATCH v4 05/12] mshtml: Convert the hex_int BOOL to flags in variant_to_nsstr.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 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        | 6 +++---
 7 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index e8d108f..f5500a3 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 40e7b4a..18056e7 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];
     HRESULT hres;
@@ -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;
 
@@ -1048,7 +1048,7 @@ HRESULT variant_to_nsstr(VARIANT *v, BOOL hex_int, nsAString *nsstr)
             nsAString_Init(nsstr, *V_BSTRREF(&var));
             break;
         }
-        hres = variant_to_nsstr(&var, hex_int, nsstr);
+        hres = variant_to_nsstr(&var, flags, nsstr);
         VariantClear(&var);
         return hres;
     }
-- 
2.31.1

[PATCH v4 06/12] mshtml: Stringify attribute values in IE8+ mode when using setAttribute.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---

Simplifies the next patch.

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

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index f5500a3..07dea49 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 18056e7..10d8bcf 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(&var));
+            break;
+        }
         nsAString_Init(nsstr, V_BSTR(&var));
         SysFreeString(V_BSTR(&var));
         break;
@@ -1040,11 +1051,22 @@ HRESULT variant_to_nsstr(VARIANT *v, DWORD flags, nsAString *nsstr)
             return hres;
 
         if(V_VT(&var) == VT_BSTR) {
+            if(flags & VARIANT_TO_NSSTR_BSTR_DEPEND) {
+                nsAString_InitDepend(nsstr, V_BSTR(&var));
+                break;
+            }
             nsAString_Init(nsstr, V_BSTR(&var));
             SysFreeString(V_BSTR(&var));
             break;
         }
         if(V_VT(&var) == (VT_BSTR | VT_BYREF)) {
+            if(flags & VARIANT_TO_NSSTR_BSTR_DEPEND) {
+                BSTR bstr = SysAllocString(*V_BSTRREF(&var));
+                if(!bstr)
+                    return E_OUTOFMEMORY;
+                nsAString_InitDepend(nsstr, bstr);
+                break;
+            }
             nsAString_Init(nsstr, *V_BSTRREF(&var));
             break;
         }
-- 
2.31.1

[PATCH v4 06/12] mshtml: Stringify attribute values in IE8+ mode when using setAttribute.

[Jacek Caban]

On 11/18/21 7:43 PM, Gabriel Ivăncescu wrote:
> +        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));

This looks ugly to me, the whole VARIANT_TO_NSSTR_BSTR_DEPEND idea does 
not seem right.


Thanks,

Jacek

[PATCH v4 06/12] mshtml: Stringify attribute values in IE8+ mode when using setAttribute.

[Gabriel Ivăncescu]

On 19/11/2021 13:15, Jacek Caban wrote:
> On 11/18/21 7:43 PM, Gabriel Ivăncescu wrote:
> > +        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));
>
> This looks ugly to me, the whole VARIANT_TO_NSSTR_BSTR_DEPEND idea does 
> not seem right.
>
> Thanks,
>
> Jacek

I can certainly get rid of it, but that will incur 2-3 extra allocations 
and copies, which seems a bit excessive to me. I agree it's a bit ugly, 
though.

Perhaps rewrite it into a helper that returns an allocated BSTR instead? 
And then have variant_to_nsstr a wrapper over it, except for VT_BSTR 
input, which will be specially handled to avoid allocations (as the 
comment above it says). Should be far better, I think.

[PATCH v4 06/12] mshtml: Stringify attribute values in IE8+ mode when using setAttribute.

[Jacek Caban]

On 11/19/21 3:04 PM, Gabriel Ivăncescu wrote:
> On 19/11/2021 13:15, Jacek Caban wrote:
> > On 11/18/21 7:43 PM, Gabriel Ivăncescu wrote:
> > > +        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));
> >
> > This looks ugly to me, the whole VARIANT_TO_NSSTR_BSTR_DEPEND idea 
> > does not seem right.
> >
> > Thanks,
> >
> > Jacek
>
> I can certainly get rid of it, but that will incur 2-3 extra 
> allocations and copies, which seems a bit excessive to me. I agree 
> it's a bit ugly, though.
>
> Perhaps rewrite it into a helper that returns an allocated BSTR 
> instead? And then have variant_to_nsstr a wrapper over it, except for 
> VT_BSTR input, which will be specially handled to avoid allocations 
> (as the comment above it says). Should be far better, I think.

You could probably just use VariantChangeTypeEx for IE8 case.


Jacek

[PATCH v4 07/12] 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 <gabrielopcode(a)gmail.com>
---

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 | 207 +++++++++++++++++++++++++++---
 4 files changed, 280 insertions(+), 31 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 4605fda..c30a44e 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;
+}
+
 HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
 {
     switch(get_dispid_type(id)) {
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 07dea49..a45b991 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)
             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 38a9545..81af28b 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=\""b"\">");
             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=\""b"\">.test returned " + e.test);
-            todo_wine_if(v == 8).
             ok(e.abcd === "\"b\"", "<" + tags[i] + " test='a\"' abcd=\""b"\">.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,23 +1133,19 @@ 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);
 
     arr.toString = function() { return 42; }
     elem.testattr = arr;
     r = elem.getAttribute("testattr");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? arr : (v < 9 ? "42" : null)), "testattr with custom toString = " + r);
     elem.setAttribute("testattr", arr);
     r = elem.getAttribute("testattr");
@@ -1126,13 +1153,11 @@ sync_test("elem_attr", function() {
     ok(elem.testattr === arr, "elem.testattr after setAttribute with custom toString = " + elem.testattr);
     r = elem.removeAttribute("testattr");
     ok(r === (v < 9 ? true : undefined), "testattr removeAttribute with custom toString returned " + r);
-    todo_wine_if(v === 8).
     ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr with custom toString = " + elem.testattr);
 
     arr.valueOf = function() { return "arrval"; }
     elem.testattr = arr;
     r = elem.getAttribute("testattr");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? arr : (v < 9 ? "arrval" : null)), "testattr with custom valueOf = " + r);
     elem.setAttribute("testattr", arr);
     r = elem.getAttribute("testattr");
@@ -1140,7 +1165,6 @@ sync_test("elem_attr", function() {
     ok(elem.testattr === arr, "elem.testattr after setAttribute with custom valueOf = " + elem.testattr);
     r = elem.removeAttribute("testattr");
     ok(r === (v < 9 ? true : undefined), "testattr removeAttribute with custom valueOf returned " + r);
-    todo_wine_if(v === 8).
     ok(elem.testattr === (v < 9 ? undefined : arr), "removed testattr with custom valueOf = " + elem.testattr);
     delete arr.valueOf;
     delete arr.toString;
@@ -1166,7 +1190,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");
@@ -1176,11 +1199,11 @@ sync_test("elem_attr", function() {
     ok(r === (v < 9 ? true : undefined), "removeAttribute after setAttribute returned " + r);
 
     /* IE11 returns an empty function, which we can't check directly */
-    todo_wine_if(v >= 8).
+    todo_wine_if(v >= 9).
     ok((v < 11) ? (elem.onclick === null) : (elem.onclick !== func), "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);
 
@@ -1190,15 +1213,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);
@@ -1218,6 +1239,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() {
-- 
2.31.1

[PATCH v4 07/12] 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=102252

Your paranoid android.


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

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

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

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

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

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

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/dispex.c              |  2 +-
 dlls/mshtml/htmlelem.c            | 29 ++++++++++++++++++++++++-----
 dlls/mshtml/mshtml_private.h      |  1 +
 dlls/mshtml/tests/documentmode.js |  2 --
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index c30a44e..e626f70 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1007,7 +1007,7 @@ static HRESULT get_builtin_func(dispex_data_t *data, DISPID id, func_info_t **re
     return DISP_E_UNKNOWNNAME;
 }
 
-static HRESULT get_builtin_id(DispatchEx *This, BSTR name, DWORD grfdex, DISPID *ret)
+HRESULT get_builtin_id(DispatchEx *This, BSTR name, DWORD grfdex, DISPID *ret)
 {
     int min, max, n, c;
 
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index a45b991..03bcbb7 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1110,15 +1110,34 @@ 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) {
+                DISPID tmp;
+                object_str = get_builtin_id(&This->node.event_target.dispex, strAttributeName,
+                                            fdexNameCaseInsensitive, &tmp) == S_OK;
+            }
+        }
 
-        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/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 9f10d68..444a4c0 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -384,6 +384,7 @@ extern void (__cdecl *note_cc_edge)(nsISupports*,const char*,nsCycleCollectionTr
 void init_dispatch(DispatchEx*,IUnknown*,dispex_static_data_t*,compat_mode_t) DECLSPEC_HIDDEN;
 void release_dispex(DispatchEx*) DECLSPEC_HIDDEN;
 BOOL dispex_query_interface(DispatchEx*,REFIID,void**) DECLSPEC_HIDDEN;
+HRESULT get_builtin_id(DispatchEx*,BSTR,DWORD,DISPID*) DECLSPEC_HIDDEN;
 HRESULT dispex_get_dprop_ref(DispatchEx*,const WCHAR*,BOOL,VARIANT**) DECLSPEC_HIDDEN;
 HRESULT get_dispids(tid_t,DWORD*,DISPID**) DECLSPEC_HIDDEN;
 HRESULT remove_attribute(DispatchEx*,DISPID,VARIANT_BOOL*) DECLSPEC_HIDDEN;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 81af28b..d01680b 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1171,7 +1171,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);
@@ -1224,7 +1223,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);
-- 
2.31.1

[PATCH v4 09/12] 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 <gabrielopcode(a)gmail.com>
---

This extends the hook callbacks with name, 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 e626f70..846a678 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, 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 03bcbb7..f7fd9ed 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1333,6 +1333,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;
     }
@@ -6689,13 +6692,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, 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(flags != DISPATCH_PROPERTYGET || !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);
 
@@ -6714,6 +6764,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..84cd0b1 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, 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, 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 444a4c0..d30c2af 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*,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;
@@ -1135,18 +1141,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 d01680b..9d2d71a 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1359,7 +1359,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);
@@ -1383,7 +1382,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..8b9cdde 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, 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 = {
-- 
2.31.1

[PATCH v4 09/12] 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=102254

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

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

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

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

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 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 846a678..b9d43e8 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 f7fd9ed..e2003ac 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1218,6 +1218,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);
 
@@ -1250,18 +1251,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;
     }
@@ -1292,6 +1301,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;
 
@@ -1304,10 +1314,12 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
 
     if(compat_mode >= COMPAT_MODE_IE8) {
         *pfSuccess = element_has_attribute(This, strAttributeName);
-        if(*pfSuccess)
-            return element_remove_attribute(This, strAttributeName);
         if(compat_mode >= COMPAT_MODE_IE9)
-            return S_OK;
+            return *pfSuccess ? element_remove_attribute(This, strAttributeName) : S_OK;
+
+        /* remove the prop without influencing the return value */
+        if(*pfSuccess)
+            pfSuccess = &tmp;
     }
 
     hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
@@ -1340,7 +1352,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)
@@ -6730,6 +6745,31 @@ static HRESULT ie8_ro_prop_hook(DispatchEx *dispex, BSTR name, LCID lcid, WORD f
     return S_OK;
 }
 
+static HRESULT ie8_rw_prop_hook(DispatchEx *dispex, 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[] = {
@@ -6744,6 +6784,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 d30c2af..ffdd445 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 9d2d71a..9608b41 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1277,7 +1277,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);
-- 
2.31.1

[PATCH v4 10/12] 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=102255

Your paranoid android.


=== 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: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

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

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

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

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

[PATCH v4 11/12] mshtml: Set the builtin prop to NULL even when removeAttribute returns false in IE8 mode.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/dispex.c              | 8 ++++++--
 dlls/mshtml/tests/documentmode.js | 1 -
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index b9d43e8..c586fb3 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1500,9 +1500,13 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success)
         if(FAILED(hres)) {
             VARIANT *ref;
             hres = dispex_get_dprop_ref(This, func->name, FALSE, &ref);
-            if(FAILED(hres) || V_VT(ref) != VT_BSTR)
+            if(FAILED(hres) || V_VT(ref) != VT_BSTR) {
                 *success = VARIANT_FALSE;
-            else
+                if(dispex_compat_mode(This) >= COMPAT_MODE_IE8) {
+                    V_VT(&var) = VT_NULL;
+                    builtin_propput(This, func, &dp, NULL);
+                }
+            }else
                 VariantClear(ref);
         }
         return S_OK;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 9608b41..08adc9e 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1183,7 +1183,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;
-- 
2.31.1

[PATCH v4 11/12] mshtml: Set the builtin prop to NULL even when removeAttribute returns false 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=102256

Your paranoid android.


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

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

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

[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 <gabrielopcode(a)gmail.com>
---
 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 a1c8a00..993b49a 100644
--- a/dlls/jscript/jscript.h
+++ b/dlls/jscript/jscript.h
@@ -516,6 +516,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 08adc9e..0781ee6 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1226,7 +1226,7 @@ sync_test("elem_attr", function() {
     r = elem.removeAttribute("ondblclick");
     ok(r === (v < 8 ? false : (v < 9 ? true : undefined)), "ondblclick removeAttribute returned " + r);
     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.
-- 
2.31.1

[PATCH v4 12/12] jscript: Return the correct string for Object.toString(null) in ES5+ 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=102257

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/18/21 7:43 PM, Gabriel Ivăncescu wrote:
> -        hres = prop_get(This, prop, &r);
> +        hres = E_FAIL;
> +        if(id == DISPID_VALUE)
> +            hres = to_primitive(This->ctx, jsval_obj(This), &r, NO_HINT);
> +        if(FAILED(hres))
> +            hres = prop_get(This, prop, &r);

You can't try to call to_primitive and ignore its failure. The call may 
have visible effects. The failure, for example, may be from a throw 
inside toString JavaScript code.


Why do you need that in the first place? The attached patch (on top of 
your patch) passes all tests. Unless there are some obscure special 
cases, it looks like we should be able to get rid of builtin 
DISPID_VALUE getters.


Thanks,

Jacek

[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=102302

Your paranoid android.


=== debiant2 (build log) ===

error: patch failed: dlls/jscript/dispex.c:1580
Task: Patch failed to apply

=== debiant2 (build log) ===

error: patch failed: dlls/jscript/dispex.c:1580
Task: Patch failed to apply

[Gabriel Ivăncescu]

On 19/11/2021 13:01, Jacek Caban wrote:
> Hi Gabriel,
>
> On 11/18/21 7:43 PM, Gabriel Ivăncescu wrote:
> > -        hres = prop_get(This, prop, &r);
> > +        hres = E_FAIL;
> > +        if(id == DISPID_VALUE)
> > +            hres = to_primitive(This->ctx, jsval_obj(This), &r, 
> > NO_HINT);
> > +        if(FAILED(hres))
> > +            hres = prop_get(This, prop, &r);
>
> You can't try to call to_primitive and ignore its failure. The call may 
> have visible effects. The failure, for example, may be from a throw 
> inside toString JavaScript code.
>
> Why do you need that in the first place? The attached patch (on top of 
> your patch) passes all tests. Unless there are some obscure special 
> cases, it looks like we should be able to get rid of builtin 
> DISPID_VALUE getters.
>
> Thanks,
>
> Jacek

Yeah, I was trying to be cautious. I'll test this some more and see what 
happens when the object doesn't expose toString or valueOf, neither in 
prototypes or anywhere.