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

[Gabriel Ivăncescu]

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.