[PATCH 4/8] jscript: Implement Array.prototype.toLocaleString.

[Gabriel Ivăncescu]

Hi Jacek,

Sorry for delay on this, been a bit swamped.

On 15/04/2022 21:02, Jacek Caban wrote:
> Hi Gabriel,
> 
> On 4/15/22 15:00, Gabriel Ivăncescu wrote:
>> +static HRESULT to_locale_string(script_ctx_t *ctx, jsval_t val, 
>> jsstr_t **str)
>> +{
>> +    DISPPARAMS dp = { 0 };
>> +    IDispatchEx *dispex;
>> +    jsdisp_t *jsdisp;
>> +    IDispatch *obj;
>> +    EXCEPINFO ei;
>> +    HRESULT hres;
>> +    UINT err = 0;
>> +    VARIANT var;
>> +    DISPID id;
>> +    BSTR bstr;
>> +
>> +    switch(jsval_type(val)) {
>> +    case JSV_OBJECT:
>> +        obj = get_object(val);
>> +        if((jsdisp = to_jsdisp(obj))) {
>> +            hres = jsdisp_call_name(jsdisp, L"toLocaleString", 
>> DISPATCH_METHOD, 0, NULL, &val);
>> +            if(FAILED(hres)) {
>> +                if(hres == JS_E_INVALID_PROPERTY && ctx->version >= 
>> SCRIPTLANGUAGEVERSION_ES5)
>> +                    hres = JS_E_FUNCTION_EXPECTED;
>> +                return hres;
>> +            }
>> +            break;
>> +        }
>> +
>> +        if(!(bstr = SysAllocString(L"toLocaleString")))
>> +            return E_OUTOFMEMORY;
>> +
>> +        V_VT(&var) = VT_EMPTY;
>> +        hres = IDispatch_QueryInterface(obj, &IID_IDispatchEx, 
>> (void**)&dispex);
>> +        if(SUCCEEDED(hres) && dispex) {
>> +            hres = IDispatchEx_GetDispID(dispex, bstr, 
>> make_grfdex(ctx, fdexNameCaseSensitive), &id);
>> +            if(SUCCEEDED(hres)) {
>> +                hres = IDispatchEx_InvokeEx(dispex, id, ctx->lcid, 
>> DISPATCH_METHOD, &dp, &var, &ei, &ctx->jscaller->IServiceProvider_iface);
>> +                if(hres == DISP_E_EXCEPTION)
>> +                    disp_fill_exception(ctx, &ei);
>> +            }
>> +            IDispatchEx_Release(dispex);
>> +        }else {
>> +            hres = IDispatch_GetIDsOfNames(obj, &IID_NULL, &bstr, 1, 
>> 0, &id);
>> +            if(SUCCEEDED(hres)) {
>> +                hres = IDispatch_Invoke(obj, id, &IID_NULL, 
>> ctx->lcid, DISPATCH_METHOD, &dp, &var, &ei, &err);
>> +                if(hres == DISP_E_EXCEPTION)
>> +                    disp_fill_exception(ctx, &ei);
>> +            }
>> +        }
>> +        SysFreeString(bstr);
>> +        if(FAILED(hres))
>> +            return hres;
>> +
>> +        hres = variant_to_jsval(ctx, &var, &val);
>> +        VariantClear(&var);
>> +        break;
> 
> 
> Shouldn't it just use disp_call();
> 

disp_call needs a DISPID though. I still need to obtain the DISPID, so 
just using Invoke after doesn't seem like much, considering I already 
QueryInterface for IDispatchEx for the DISPID...

> 
>> +    case JSV_NUMBER:
>> +        if(ctx->version >= SCRIPTLANGUAGEVERSION_ES5) {
>> +            hres = Number_toLocaleString(ctx, val, 0, 0, NULL, &val);
>> +            if(SUCCEEDED(hres))
>> +                *str = get_string(val);
>> +            return hres;
>> +        }
>> +        /* fall through */
> 
> 
> Should it just use the default case? If not, then please don't do direct 
> calls to functions like Number_toLocaleString like this, a separated 
> helper would be better.
> 

According to the tests, it uses Number.prototype.toLocaleString on ES5 
for some reason. See the /* primitive number value not affected */ part.