[PATCH 01/14] mshtml: Retry with VT_NULL when removing an attribute.

[Gabriel Ivăncescu]

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?