Re: [PATCH v2 03/12] mshtml: Handle VT_DISPATCH in variant_to_nsstr.

[Jacek Caban]

Hi Gabriel,

On 11/16/21 6:58 PM, Gabriel Ivăncescu wrote:
> -
>    /* try toString() first */
> -
>    memcpy(buf, L"toString", sizeof(L"toString"));
> -
>    name = buf;
> -
>    hres = IDispatch_GetIDsOfNames(disp, &IID_NULL, &name, 1, 0, &dispid);
> -
>    if(SUCCEEDED(hres) && dispid != DISPID_UNKNOWN) {
> -
>        DISPPARAMS params = { &strv, NULL, 0, 0 };
> -
>        hres = IDispatch_Invoke(disp, dispid, &IID_NULL, lcid, DISPATCH_METHOD, &params, &strv, NULL, NULL);
> -
>        if(SUCCEEDED(hres)) {
> -
>            if(V_VT(&strv) == VT_BSTR) {
> -
>                nsAString_Init(nsstr, V_BSTR(&strv));
> -
>                SysFreeString(V_BSTR(&strv));
> -
>                break;
> -
>            }
> -
>            VariantClear(&strv);
> -
>        }
> -
>    }

I still hope to see a better solution for this. Calling toString() on an 
arbitrary IDispatch is almost surely not what we should do here. 
toString() is an implementation detail of how JavaScript converts 
objects to primitive values. It is exposed by ActiveScript engines using 
IVariantChangeType. We already use it in MSHTML IDispatchEx 
implementation and I'd expect those cases to already work correctly. It 
may be tricky to use it here, we don't really support using it outside 
InvokeEx yet.

Another thing that may be interesting to check is if current 
DISPID_VALUE behaviour is correct in jscript. I think that we don't have 
tests for that yet and maybe it should take care of calling toString in 
this case.

Thanks,

Jacek