Signed-off-by: Gabriel Ivăncescu gabrielopcode@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");
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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)
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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,
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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)
For IE9 and up we use proper attribute nodes without associated props.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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;
Signed-off-by: Jacek Caban jacek@codeweavers.com
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@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() {
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
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
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.
put_cssText implicitly sets the attribute, which must be removed since IE8 specially checks it.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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");
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
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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);
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
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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;
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
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@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.
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
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
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.
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