[PATCH v12 6/7] mshtml: Set the builtin prop to NULL even when removeAttribute returns false in IE8 mode.

[Gabriel Ivăncescu]

On 02/12/2021 15:51, Jacek Caban wrote:
> On 12/2/21 2:20 PM, Gabriel Ivăncescu wrote:
>> On 01/12/2021 21:24, Jacek Caban wrote:
>>> Hi Gabriel,
>>>
>>> On 12/1/21 2:41 PM, Gabriel Ivăncescu wrote:
>>>> + if(dispex_compat_mode(This) >= COMPAT_MODE_IE8) {
>>>> +                    V_VT(&var) = VT_NULL;
>>>> +                    builtin_propput(This, func, &dp, NULL);
>>>> +                }
>>>
>>>
>>> That doesn't seem universally right, see the attached test for an 
>>> example.
>>>
>>>
>>> Thanks,
>>>
>>> Jacek
>>>
>>
>> Are you sure this is related to this patch? I did some digging but it 
>> seems to be a completely different issue to me. This patch is about 
>> removeAttribute returning false and setting to NULL, which happens 
>> with event handlers. 
> 
> 
> Yes, the existing code is already wrong, we probably shouldn't blindly 
> try to set value to VT_EMPTY in the first place. Trying to set the 
> property to another value (of type that doesn't make sense unless you 
> assume that those are event handlers) based on a previous error that we 
> know nothing about (it could be OOM?) is making it even worse. Is there 
> another reason for this patch than a todo_wine in tests?
> 
> 
> Thanks,
> 
> Jacek
> 

I think the VT_EMPTY is correct for pre IE8 modes, since I didn't get 
any failures there, just not for IE8.

Even if we assume that we do implement it correctly for IE8 to reset it 
to default values at some point, it would *still* not work without this 
patch for this particular case, since that would handle the case where 
it returns true, not false. Isn't this patch still needed, then? I can't 
see how else it would work.

As for the reason, well, prototype.js sets "onclick" to an array and 
later removeAttribute on it. Not sure to what extent it's useful though. 
But it was a pretty simple fix that I honestly see no downsides to—even 
in the future should we implement the rest properly.