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@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 | 117 ++++++++++++++--- dlls/mshtml/mshtml_private.h | 1 + dlls/mshtml/tests/documentmode.js | 207 +++++++++++++++++++++++++++--- 4 files changed, 303 insertions(+), 32 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 e8d108f..cff6e4b 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -1086,18 +1086,60 @@ 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); + VARIANT val = AttributeValue; + BOOL needs_free = FALSE; 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) { + /* 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; + + 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; + } + } + + /* 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, &val); + if(FAILED(hres) && compat_mode < COMPAT_MODE_IE8) + goto done; + }else + hres = E_FAIL; + + if(hres == E_FAIL && This->dom_element) { nsAString name_str, value_str; nsresult nsres;
- 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); @@ -1105,15 +1147,13 @@ static HRESULT WINAPI HTMLElement_setAttribute(IHTMLElement *iface, BSTR strAttr nsAString_Finish(&value_str); if(NS_FAILED(nsres)) WARN("SetAttribute failed: %08x\n", nsres); - return map_nsresult(nsres); + hres = map_nsresult(nsres); }
- hres = IDispatchEx_GetDispID(&This->node.event_target.dispex.IDispatchEx_iface, strAttributeName, - (lFlags&ATTRFLAG_CASESENSITIVE ? fdexNameCaseSensitive : fdexNameCaseInsensitive) | fdexNameEnsure, &dispid); - if(FAILED(hres)) - return hres; - - return set_elem_attr_value_by_dispid(This, dispid, &AttributeValue); +done: + if(needs_free) + SysFreeString(V_BSTR(&val)); + return hres; }
HRESULT get_elem_attr_value_by_dispid(HTMLElement *elem, DISPID dispid, VARIANT *ret) @@ -1156,6 +1196,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;
@@ -1164,10 +1207,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); @@ -1187,8 +1232,38 @@ 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 && V_VT(AttributeValue) != VT_NULL) { + LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH,SUBLANG_ENGLISH_US),SORT_DEFAULT); + + 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; } @@ -1197,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, @@ -6463,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 5cf53bb..101f005 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 c2fd8bd..27e6431 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() {