[PATCH v11 01/10] mshtml: Stringify attribute values in setAttribute using string hints in IE10+.

[Gabriel Ivăncescu]

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

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 4605fda..9f56a56 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1043,7 +1043,7 @@ static HRESULT get_builtin_id(DispatchEx *This, BSTR name, DWORD grfdex, DISPID
     return DISP_E_UNKNOWNNAME;
 }
 
-static HRESULT change_type(VARIANT *dst, VARIANT *src, VARTYPE vt, IServiceProvider *caller)
+HRESULT change_type(VARIANT *dst, VARIANT *src, VARTYPE vt, IServiceProvider *caller)
 {
     V_VT(dst) = VT_EMPTY;
 
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index d54f729..3f7c60b 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -6585,8 +6585,42 @@ static IHTMLEventObj *HTMLElement_set_current_event(DispatchEx *dispex, IHTMLEve
     return default_set_current_event(This->node.doc->window, event);
 }
 
+static HRESULT IHTMLElement6_setAttribute_hook(DispatchEx *dispex, LCID lcid, WORD flags, DISPPARAMS *dp,
+        VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
+{
+    VARIANT args[2];
+    HRESULT hres;
+    DISPPARAMS new_dp = { args, NULL, 2, 0 };
+
+    if(!(flags & DISPATCH_METHOD) || dp->cArgs < 2 || dp->cNamedArgs)
+        return S_FALSE;
+
+    switch(V_VT(&dp->rgvarg[dp->cArgs - 2])) {
+    case VT_EMPTY:
+    case VT_BSTR:
+    case VT_NULL:
+        return S_FALSE;
+    default:
+        break;
+    }
+
+    hres = change_type(&args[0], &dp->rgvarg[dp->cArgs - 2], VT_BSTR, caller);
+    if(FAILED(hres))
+        return hres;
+    args[1] = dp->rgvarg[dp->cArgs - 1];
+
+    hres = IDispatchEx_InvokeEx(&dispex->IDispatchEx_iface, DISPID_IHTMLELEMENT6_IE9_SETATTRIBUTE,
+                                lcid, flags, &new_dp, res, ei, caller);
+    VariantClear(&args[0]);
+    return hres;
+}
+
 void HTMLElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
 {
+    static const dispex_hook_t elem6_ie10_hooks[] = {
+        {DISPID_IHTMLELEMENT6_IE9_SETATTRIBUTE, IHTMLElement6_setAttribute_hook},
+        {DISPID_UNKNOWN}
+    };
     static const dispex_hook_t elem2_ie11_hooks[] = {
         {DISPID_IHTMLELEMENT2_DOSCROLL, NULL},
         {DISPID_IHTMLELEMENT2_READYSTATE, NULL},
@@ -6601,7 +6635,7 @@ void HTMLElement_init_dispex_info(dispex_data_t *info, compat_mode_t mode)
         dispex_info_add_interface(info, IElementSelector_tid, NULL);
 
     if(mode >= COMPAT_MODE_IE9) {
-        dispex_info_add_interface(info, IHTMLElement6_tid, NULL);
+        dispex_info_add_interface(info, IHTMLElement6_tid, mode >= COMPAT_MODE_IE10 ? elem6_ie10_hooks : NULL);
         dispex_info_add_interface(info, IElementTraversal_tid, NULL);
     }
 
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 5cf53bb..6978ed7 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 change_type(VARIANT*,VARIANT*,VARTYPE,IServiceProvider*) 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 08fcde4..c2fd8bd 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 v11 02/10] mshtml: Restructure element setAttribute.

[Gabriel Ivăncescu]

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

Will be needed for IE8 to fallback to the gecko attribute nodes under
certain conditions.

 dlls/mshtml/htmlelem.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 3f7c60b..16357de 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1086,34 +1086,36 @@ 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);
+    nsAString name_str, value_str;
+    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, FALSE, &value_str);
+    if(compat_mode < COMPAT_MODE_IE8 || !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;
 
-        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);
+        return set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);
     }
 
-    hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
-            (lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive) | fdexNameEnsure, &dispid);
+    hres = variant_to_nsstr(&AttributeValue, FALSE, &value_str);
     if(FAILED(hres))
         return hres;
 
-    return set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);
+    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);
+    hres = map_nsresult(nsres);
+
+    return hres;
 }
 
 HRESULT get_elem_attr_value_by_dispid(HTMLElement *elem, DISPID dispid, VARIANT *ret)
-- 
2.31.1

[PATCH v11 03/10] mshtml: Restructure element getAttribute.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/htmlelem.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index 16357de..b6c9db0 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1158,6 +1158,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;
 
@@ -1166,33 +1169,28 @@ 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;
-
-        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);
-    }
+    if(compat_mode < COMPAT_MODE_IE8 || !This->dom_element) {
+        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+                                     lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &dispid);
+        if(FAILED(hres)) {
+            V_VT(AttributeValue) = VT_NULL;
+            return (hres == DISP_E_UNKNOWNNAME) ? S_OK : hres;
+        }
 
-    hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
-            lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &dispid);
-    if(hres == DISP_E_UNKNOWNNAME) {
-        V_VT(AttributeValue) = VT_NULL;
-        return S_OK;
-    }
+        hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
+        if(FAILED(hres))
+            return hres;
 
-    if(FAILED(hres)) {
-        V_VT(AttributeValue) = VT_NULL;
+        if(lFlags & ATTRFLAG_ASSTRING)
+            hres = attr_value_to_string(AttributeValue);
         return hres;
     }
 
-    hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
-    if(SUCCEEDED(hres) && (lFlags & ATTRFLAG_ASSTRING))
-        hres = attr_value_to_string(AttributeValue);
-    return hres;
+    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);
 }
 
 static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strAttributeName,
-- 
2.31.1

[PATCH v11 04/10] mshtml: Restructure element removeAttribute.

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/htmlelem.c | 55 +++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index b6c9db0..d718069 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1197,46 +1197,47 @@ 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) {
-        *pfSuccess = element_has_attribute(This, strAttributeName);
-        if(*pfSuccess)
-            return element_remove_attribute(This, strAttributeName);
-        return S_OK;
-    }
+    if(compat_mode < COMPAT_MODE_IE8 || !This->dom_element) {
+        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+                                     lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &id);
+        if(hres == DISP_E_UNKNOWNNAME) {
+            *pfSuccess = VARIANT_FALSE;
+            return S_OK;
+        }
+        if(FAILED(hres))
+            return hres;
 
-    hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
-            lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &id);
-    if(hres == DISP_E_UNKNOWNNAME) {
-        *pfSuccess = VARIANT_FALSE;
-        return S_OK;
-    }
-    if(FAILED(hres))
-        return hres;
+        if(id == DISPID_IHTMLELEMENT_STYLE) {
+            IHTMLStyle *style;
 
-    if(id == DISPID_IHTMLELEMENT_STYLE) {
-        IHTMLStyle *style;
+            TRACE("Special case: style\n");
 
-        TRACE("Special case: style\n");
+            hres = IHTMLElement_get_style(&This->IHTMLElement_iface, &style);
+            if(FAILED(hres))
+                return hres;
 
-        hres = IHTMLElement_get_style(&This->IHTMLElement_iface, &style);
-        if(FAILED(hres))
-            return hres;
+            hres = IHTMLStyle_put_cssText(style, NULL);
+            IHTMLStyle_Release(style);
+            if(FAILED(hres))
+                return hres;
 
-        hres = IHTMLStyle_put_cssText(style, NULL);
-        IHTMLStyle_Release(style);
-        if(FAILED(hres))
-            return hres;
+            *pfSuccess = VARIANT_TRUE;
+            return S_OK;
+        }
 
-        *pfSuccess = VARIANT_TRUE;
-        return S_OK;
+        return remove_attribute(&This->node.event_target.dispex, id, pfSuccess);
     }
 
-    return remove_attribute(&This->node.event_target.dispex, id, pfSuccess);
+    *pfSuccess = element_has_attribute(This, strAttributeName);
+    if(*pfSuccess)
+        return element_remove_attribute(This, strAttributeName);
+    return S_OK;
 }
 
 static HRESULT WINAPI HTMLElement_put_className(IHTMLElement *iface, BSTR v)
-- 
2.31.1

[PATCH v11 05/10] mshtml: Don't populate the props at all on IE9+ modes.

[Gabriel Ivăncescu]

For IE9 and up we use proper attribute nodes without associated props.

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/mshtml/htmlelem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index d718069..bc2a28f 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -6467,6 +6467,9 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex)
     if(!This->dom_element)
         return S_FALSE;
 
+    if(dispex_compat_mode(dispex) >= COMPAT_MODE_IE9)
+        return S_OK;
+
     nsres = nsIDOMElement_GetAttributes(This->dom_element, &attrs);
     if(NS_FAILED(nsres))
         return E_FAIL;
-- 
2.31.1

[PATCH v11 05/10] mshtml: Don't populate the props at all on IE9+ modes.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

[PATCH v11 06/10] mshtml: Associate props with attributes for IE8 mode, but keep them stringified.

[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. setAttribute also stringifies the value if it's a builtin only.

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

This patch is impossible to split without introducing test failures,
since it depends on the behavior *between* getAttribute, setAttribute and
removeAttribute.

We also *have* to stringify in this patch, because the existing tests already
test that and succeed, since it used to be stringified by variant_to_nsstr
even though we don't use it now for most props in IE8 mode.

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

 dlls/mshtml/htmlelem.c            | 88 +++++++++++++++++++++++++------
 dlls/mshtml/tests/documentmode.js | 77 +++++++++++++++++++++------
 2 files changed, 133 insertions(+), 32 deletions(-)

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index bc2a28f..aaa5bc3 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1067,6 +1067,15 @@ static HRESULT WINAPI HTMLElement_Invoke(IHTMLElement *iface, DISPID dispIdMembe
             wFlags, pDispParams, pVarResult, pExcepInfo, puArgErr);
 }
 
+static inline WCHAR *translate_attr_name(WCHAR *attr_name, compat_mode_t compat_mode)
+{
+    WCHAR *ret = attr_name;
+
+    if(compat_mode >= COMPAT_MODE_IE8 && !wcsicmp(attr_name, L"class"))
+        ret = (WCHAR*)L"className";
+    return ret;
+}
+
 static HRESULT set_elem_attr_value_by_dispid(HTMLElement *elem, DISPID dispid, VARIANT *v)
 {
     DISPID propput_dispid = DISPID_PROPERTYPUT;
@@ -1088,24 +1097,47 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
     HTMLElement *This = impl_from_IHTMLElement(iface);
     compat_mode_t compat_mode = dispex_compat_mode(&This->node.event_target.dispex);
     nsAString name_str, value_str;
+    VARIANT val = AttributeValue;
+    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(compat_mode < COMPAT_MODE_IE8 || !This->dom_element) {
-        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+    if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) {
+        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, translate_attr_name(strAttributeName, compat_mode),
                 (lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive) | fdexNameEnsure, &dispid);
         if(FAILED(hres))
             return hres;
 
-        return set_elem_attr_value_by_dispid(This, dispid, &AttributeValue);
+        if(compat_mode >= COMPAT_MODE_IE8 && get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
+            if(V_VT(&val) != VT_BSTR && V_VT(&val) != VT_NULL) {
+                LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT);
+
+                V_VT(&val) = VT_EMPTY;
+                hres = VariantChangeTypeEx(&val, &AttributeValue, lcid, 0, VT_BSTR);
+                if(FAILED(hres))
+                    return hres;
+
+                if(V_BSTR(&val))
+                    needs_free = TRUE;
+                else
+                    V_VT(&val) = VT_NULL;
+            }
+        }
+
+        /* className and style are special cases */
+        if(compat_mode != COMPAT_MODE_IE8 || !This->dom_element ||
+           (dispid != DISPID_IHTMLELEMENT_CLASSNAME && dispid != DISPID_IHTMLELEMENT_STYLE)) {
+            hres = set_elem_attr_value_by_dispid(This, dispid, &val);
+            goto done;
+        }
     }
 
-    hres = variant_to_nsstr(&AttributeValue, FALSE, &value_str);
+    hres = variant_to_nsstr(&val, FALSE, &value_str);
     if(FAILED(hres))
-        return hres;
+        goto done;
 
     nsAString_InitDepend(&name_str, strAttributeName);
     nsres = nsIDOMElement_SetAttribute(This->dom_element, &name_str, &value_str);
@@ -1115,6 +1147,9 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
         WARN("SetAttribute failed: %08x\n", nsres);
     hres = map_nsresult(nsres);
 
+done:
+    if(needs_free)
+        SysFreeString(V_BSTR(&val));
     return hres;
 }
 
@@ -1169,21 +1204,41 @@ static HRESULT WINAPI HTMLElement_getAttribute(IHTMLElement *iface, BSTR strAttr
     if(lFlags & ~(ATTRFLAG_CASESENSITIVE|ATTRFLAG_ASSTRING))
         FIXME("Unsupported flags %x\n", lFlags);
 
-    if(compat_mode < COMPAT_MODE_IE8 || !This->dom_element) {
-        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+    if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) {
+        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, translate_attr_name(strAttributeName, compat_mode),
                                      lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &dispid);
         if(FAILED(hres)) {
             V_VT(AttributeValue) = VT_NULL;
             return (hres == DISP_E_UNKNOWNNAME) ? S_OK : hres;
         }
 
-        hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
-        if(FAILED(hres))
-            return hres;
+        /* className and style are special cases */
+        if(compat_mode != COMPAT_MODE_IE8 || !This->dom_element ||
+           (dispid != DISPID_IHTMLELEMENT_CLASSNAME && dispid != DISPID_IHTMLELEMENT_STYLE)) {
+            hres = get_elem_attr_value_by_dispid(This, dispid, AttributeValue);
+            if(FAILED(hres))
+                return hres;
 
-        if(lFlags & ATTRFLAG_ASSTRING)
-            hres = attr_value_to_string(AttributeValue);
-        return hres;
+            if(compat_mode >= COMPAT_MODE_IE8 && V_VT(AttributeValue) != VT_BSTR && V_VT(AttributeValue) != VT_NULL) {
+                LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT);
+
+                if(get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
+                    VariantClear(AttributeValue);
+                    V_VT(AttributeValue) = VT_NULL;
+                    return S_OK;
+                }
+
+                hres = VariantChangeTypeEx(AttributeValue, AttributeValue, lcid, 0, VT_BSTR);
+                if(FAILED(hres)) {
+                    VariantClear(AttributeValue);
+                    return hres;
+                }
+                if(!V_BSTR(AttributeValue))
+                    V_VT(AttributeValue) = VT_NULL;
+            }else if(lFlags & ATTRFLAG_ASSTRING)
+                hres = attr_value_to_string(AttributeValue);
+            return hres;
+        }
     }
 
     nsAString_InitDepend(&name_str, strAttributeName);
@@ -1203,8 +1258,8 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
 
     TRACE("(%p)->(%s %x %p)\n", This, debugstr_w(strAttributeName), lFlags, pfSuccess);
 
-    if(compat_mode < COMPAT_MODE_IE8 || !This->dom_element) {
-        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName,
+    if(compat_mode < COMPAT_MODE_IE9 || !This->dom_element) {
+        hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, translate_attr_name(strAttributeName, compat_mode),
                                      lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive, &id);
         if(hres == DISP_E_UNKNOWNNAME) {
             *pfSuccess = VARIANT_FALSE;
@@ -1231,7 +1286,8 @@ static HRESULT WINAPI HTMLElement_removeAttribute(IHTMLElement *iface, BSTR strA
             return S_OK;
         }
 
-        return remove_attribute(&This->node.event_target.dispex, id, pfSuccess);
+        if(compat_mode != COMPAT_MODE_IE8 || !This->dom_element || id != DISPID_IHTMLELEMENT_CLASSNAME)
+            return remove_attribute(&This->node.event_target.dispex, id, pfSuccess);
     }
 
     *pfSuccess = element_has_attribute(This, strAttributeName);
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index c2fd8bd..2ef9173 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,30 @@ 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) {
+        /* style is a 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 v11 06/10] mshtml: Associate props with attributes for IE8 mode, but keep them stringified.

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

Your paranoid android.


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

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

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

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

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

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

[PATCH v11 06/10] mshtml: Associate props with attributes for IE8 mode, but keep them stringified.

[Jacek Caban]

Hi Gabriel,

On 11/29/21 5:31 PM, Gabriel Ivăncescu wrote:
> +static inline WCHAR *translate_attr_name(WCHAR *attr_name, compat_mode_t compat_mode)
> +{
> +    WCHAR *ret = attr_name;
> +
> +    if(compat_mode >= COMPAT_MODE_IE8 && !wcsicmp(attr_name, L"class"))
> +        ret = (WCHAR*)L"className";
> +    return ret;
> +}

Please avoid this cast.
> +                if(get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
> +                    VariantClear(AttributeValue);
> +                    V_VT(AttributeValue) = VT_NULL;
> +                    return S_OK;
> +                }

It's not right in general, see the attached test. I'm not expecting that 
you implement all those IE8-specific things (in fact, I don't think we 
need to care too much about it), but the patch in its current shape can 
break things. Could you just remove this if() and live todo_wines?


Thanks,

Jacek

[PATCH v11 06/10] mshtml: Associate props with attributes for IE8 mode, but keep them stringified.

[Gabriel Ivăncescu]

On 30/11/2021 22:59, Jacek Caban wrote:
> Hi Gabriel,
>
> On 11/29/21 5:31 PM, Gabriel Ivăncescu wrote:
> > +static inline WCHAR *translate_attr_name(WCHAR *attr_name, 
> > compat_mode_t compat_mode)
> > +{
> > +    WCHAR *ret = attr_name;
> > +
> > +    if(compat_mode >= COMPAT_MODE_IE8 && !wcsicmp(attr_name, L"class"))
> > +        ret = (WCHAR*)L"className";
> > +    return ret;
> > +}
>
> Please avoid this cast.
> > +                if(get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
> > +                    VariantClear(AttributeValue);
> > +                    V_VT(AttributeValue) = VT_NULL;
> > +                    return S_OK;
> > +                }
>
> It's not right in general, see the attached test. I'm not expecting that 
> you implement all those IE8-specific things (in fact, I don't think we 
> need to care too much about it), but the patch in its current shape can 
> break things. Could you just remove this if() and live todo_wines?

Sure, not a problem. My only fear was that it *adds* a todo_wine without 
that code to existing tests, but as you said that's not very important 
and it was also a side-effect of a former coincidence just working 
right, so I hope adding it isn't too much of a problem.

[PATCH v11 07/10] mshtml: Fix style attribute removal in IE8 mode.

[Gabriel Ivăncescu]

put_cssText implicitly sets the attribute, which must be removed since IE8
specially checks it.

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

diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index aaa5bc3..ac7ea47 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1282,6 +1282,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;
         }
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index 2ef9173..02cdf29 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -1255,7 +1255,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");
-- 
2.31.1

[PATCH v11 07/10] mshtml: Fix style attribute removal 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=102930

Your paranoid android.


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

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

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

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

[PATCH v11 08/10] 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            | 16 ++++++++++++++--
 dlls/mshtml/mshtml_private.h      |  1 +
 dlls/mshtml/tests/documentmode.js |  2 --
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c
index 9f56a56..5cb7e96 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 ac7ea47..0f0b7e3 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1099,9 +1099,9 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
     nsAString name_str, value_str;
     VARIANT val = AttributeValue;
     BOOL needs_free = FALSE;
+    HRESULT hres = S_OK;
     nsresult nsres;
     DISPID dispid;
-    HRESULT hres;
 
     TRACE("(%p)->(%s %s %08x)\n", This, debugstr_w(strAttributeName), debugstr_variant(&AttributeValue), lFlags);
 
@@ -1112,7 +1112,12 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
             return hres;
 
         if(compat_mode >= COMPAT_MODE_IE8 && get_dispid_type(dispid) == DISPEXPROP_BUILTIN) {
-            if(V_VT(&val) != VT_BSTR && V_VT(&val) != VT_NULL) {
+            if(V_VT(&val) == VT_DISPATCH && compat_mode < COMPAT_MODE_IE10) {
+                if(!(V_BSTR(&val) = SysAllocString(L"[object]")))
+                    return E_OUTOFMEMORY;
+                V_VT(&val) = VT_BSTR;
+                needs_free = TRUE;
+            }else if(V_VT(&val) != VT_BSTR && V_VT(&val) != VT_NULL) {
                 LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT);
 
                 V_VT(&val) = VT_EMPTY;
@@ -1133,6 +1138,13 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr
             hres = set_elem_attr_value_by_dispid(This, dispid, &val);
             goto done;
         }
+    }else if(compat_mode < COMPAT_MODE_IE10 && V_VT(&val) == VT_DISPATCH) {
+        if(get_builtin_id(&This->node.event_target.dispex, strAttributeName, fdexNameCaseInsensitive, &dispid) == S_OK) {
+            if(!(V_BSTR(&val) = SysAllocString(L"[object]")))
+                return E_OUTOFMEMORY;
+            V_VT(&val) = VT_BSTR;
+            needs_free = TRUE;
+        }
     }
 
     hres = variant_to_nsstr(&val, FALSE, &value_str);
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index 6978ed7..86ea21e 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -385,6 +385,7 @@ void init_dispatch(DispatchEx*,IUnknown*,dispex_static_data_t*,compat_mode_t) DE
 void release_dispex(DispatchEx*) DECLSPEC_HIDDEN;
 BOOL dispex_query_interface(DispatchEx*,REFIID,void**) DECLSPEC_HIDDEN;
 HRESULT change_type(VARIANT*,VARIANT*,VARTYPE,IServiceProvider*) 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 02cdf29..01cd2b7 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 v11 08/10] mshtml: Set dispatch values to "[object]" for builtin attributes in IE8 and IE9 modes.

[Marvin]

Hi,

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

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

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

[PATCH v11 09/10] 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 5cb7e96..7ac98e6 100644
--- a/dlls/mshtml/dispex.c
+++ b/dlls/mshtml/dispex.c
@@ -1470,9 +1470,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 01cd2b7..bc6d1ee 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 v11 09/10] 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=102932

Your paranoid android.


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

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

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

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

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

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

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

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

[PATCH v11 10/10] 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 3eef1aa..c0eb847 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 c192ec7..8a9bcf4 100644
--- a/dlls/jscript/jscript.h
+++ b/dlls/jscript/jscript.h
@@ -514,6 +514,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 24692f8..c008469 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 bc6d1ee..1a52140 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 58b0d43..84b5f36 100644
--- a/dlls/mshtml/tests/es5.js
+++ b/dlls/mshtml/tests/es5.js
@@ -797,7 +797,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 v11 10/10] 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=102933

Your paranoid android.


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

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

=== w1064_tsign (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_ar (64 bit report) ===

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

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

[Jacek Caban]

On 11/29/21 5:31 PM, Gabriel Ivăncescu wrote:
> +    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;
> +    }

This is ugly. I think that what you observe is a change in how ES5 
treats 'this' argument in builtin functions. It allows 'this' to be of 
any type for builtin function. The conversion to object (or using a 
global object in place of null) happens only when we enter JS function, 
if I read it right. To support this properly, I'm afraid that we'd need 
to pass 'this' as jsval to builtin functions. We wouldn't need any 
special casing for toString() then.


Thanks,

Jacek

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

[Gabriel Ivăncescu]

On 30/11/2021 23:20, Jacek Caban wrote:
> On 11/29/21 5:31 PM, Gabriel Ivăncescu wrote:
> > +    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;
> > +    }
>
> This is ugly. I think that what you observe is a change in how ES5 
> treats 'this' argument in builtin functions. It allows 'this' to be of 
> any type for builtin function. The conversion to object (or using a 
> global object in place of null) happens only when we enter JS function, 
> if I read it right. To support this properly, I'm afraid that we'd need 
> to pass 'this' as jsval to builtin functions. We wouldn't need any 
> special casing for toString() then.

Interesting, I did not realize builtin functions were special in regards 
to the "this" context. I'll send this separately later then, but passing 
jsval to builtin functions would also help with fixing the undefined 
case (since toString reports [object Undefined] then), so it's on the 
right track for sure.

[Jacek Caban]

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

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

Your paranoid android.


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

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

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

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

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

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

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

mshtml:
htmldoc.c:2541: Test failed: unexpected call UpdateUI
htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
htmldoc.c: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