On 16/11/2021 16:48, Gabriel Ivăncescu wrote:
Hi Jacek,
On 16/11/2021 16:57, Jacek Caban wrote:
Hi Gabriel,
On 11/16/21 3:29 PM, Gabriel Ivăncescu wrote:
diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 96a776d..bddb0e3 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -1466,6 +1466,11 @@ HRESULT remove_attribute(DispatchEx *This, DISPID id, VARIANT_BOOL *success) V_VT(&var) = VT_EMPTY; hres = builtin_propput(This, func, &dp, NULL); + if(hres == E_NOTIMPL) { + /* Setting event handlers to VT_EMPTY fails in quirks mode */ + V_VT(&var) = VT_NULL; + hres = builtin_propput(This, func, &dp, NULL); + } if(FAILED(hres)) return hres; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index d566223..c197f9b 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -1087,6 +1087,46 @@ sync_test("elem_attr", function() { ok(r === "cls2", "class attr = " + r); r = elem.getAttribute("className"); ok(r === "cls3", "className attr = " + r);
+ var func = function() { }; + elem.onclick = func; + ok(elem.onclick === func, "onclick = " + elem.onclick); + r = elem.getAttribute("onclick"); + ok(r === (v < 8 ? func : null), "onclick attr = " + r); + r = elem.removeAttribute("onclick"); + todo_wine_if(v < 9). + ok(r === (v < 9 ? false : undefined), "removeAttribute returned " + r); + todo_wine_if(v < 9). + ok(elem.onclick === (v != 8 ? func : null), "removed onclick = " + elem.onclick);
This test shows that removeAttribute() should not work in this case (although it should not throw), so making it work does not seem right.
Thanks,
Jacek
Well, in subsequent tests, it does work. The reason it doesn't work here is because the attribute is set to a non-string value (there's tests + fixes later in the series for that). Should I move the tests with todo_wine then?
I would hope for something cleaner instead. So far this patch and patch 5 is backed only on event handler tests and event handlers are likely to behave differently than other properties. I can't be sure without trying, but for those tests it seems to me that you could replace throwing by returning false from remove_attribute and try to use dispex_get_dprop_ref to clear the value (which would cover string values of event handler).
Thanks,
Jacek