[PATCH v4 06/12] mshtml: Stringify attribute values in IE8+ mode when using setAttribute.

[Jacek Caban]

On 11/19/21 3:04 PM, Gabriel Ivăncescu wrote:
> On 19/11/2021 13:15, Jacek Caban wrote:
>> On 11/18/21 7:43 PM, Gabriel Ivăncescu wrote:
>>> +        hres = variant_to_nsstr(&AttributeValue, 
>>> VARIANT_TO_NSSTR_BSTR_DEPEND, &value_str);
>>>           if(FAILED(hres))
>>>               return hres;
>>> +        if((V_VT(&AttributeValue) & ~VT_BYREF) != VT_BSTR)
>>> +            needs_free = TRUE;
>>> +
>>> +        V_VT(&AttributeValue) = VT_BSTR;
>>> +        nsAString_GetData(&value_str, (const 
>>> WCHAR**)&V_BSTR(&AttributeValue));
>>
>>
>> This looks ugly to me, the whole VARIANT_TO_NSSTR_BSTR_DEPEND idea 
>> does not seem right.
>>
>>
>> Thanks,
>>
>> Jacek
>>
>
> I can certainly get rid of it, but that will incur 2-3 extra 
> allocations and copies, which seems a bit excessive to me. I agree 
> it's a bit ugly, though.
>
> Perhaps rewrite it into a helper that returns an allocated BSTR 
> instead? And then have variant_to_nsstr a wrapper over it, except for 
> VT_BSTR input, which will be specially handled to avoid allocations 
> (as the comment above it says). Should be far better, I think.


You could probably just use VariantChangeTypeEx for IE8 case.


Jacek