[PATCH v2 07/12] mshtml: Fix setting and retrieving attributes in IE8 mode.

Gabriel Ivăncescu gabrielopcode at gmail.com
Tue Nov 16 11:58:22 CST 2021


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 at 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 | 205 ++++++++++++++++++++++++++++--
 4 files changed, 280 insertions(+), 29 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 138806f..ca3ca54 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 "testarray"; }
     elem.testattr = arr;
     r = elem.getAttribute("testattr");
-    todo_wine_if(v === 8).
     ok(r === (v < 8 ? arr : (v < 9 ? "testarray" : null)), "testattr with custom toString = " + r);
     elem.setAttribute("testattr", arr);
     r = elem.getAttribute("testattr");
@@ -1126,7 +1153,6 @@ 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);
     delete arr.toString;
 
@@ -1151,7 +1177,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");
@@ -1161,11 +1186,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);
 
@@ -1175,15 +1200,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);
@@ -1203,6 +1226,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




More information about the wine-devel mailing list