[PATCH 1/2] jscript: Handle detached scope objects 'this' in Object.toString.

[Gabriel Ivăncescu]

On 05/05/2022 18:18, Jacek Caban wrote:
> Hi Gabriel,
> 
> On 5/4/22 18:10, Gabriel Ivăncescu wrote:
>> diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c
>> index a675e45..453ea95 100644
>> --- a/dlls/jscript/object.c
>> +++ b/dlls/jscript/object.c
>> @@ -73,8 +73,9 @@ static HRESULT Object_toString(script_ctx_t *ctx, 
>> jsval_t vthis, WORD flags, uns
>>           str = L"[object Object]";
>>       }else if(names[jsdisp->builtin_info->class]) {
>>           str = names[jsdisp->builtin_info->class];
>> +    }else if(jsdisp->builtin_info->class == JSCLASS_NONE) {
>> +        hres = JS_E_OBJECT_EXPECTED;
>>       }else {
>> -        assert(jsdisp->builtin_info->class != JSCLASS_NONE);
>>           FIXME("jsdisp->builtin_info->class = %d\n", 
>> jsdisp->builtin_info->class);
>>           hres = E_FAIL;
>>       }
> 
> 
> It seems like something is still not working as intended and I'm not 
> sure if it's the right place to fix it. Why would it be specific to 
> detached scopes? I was wondering why you need so complicated test and 
> tried the attached one. It works fine on Windows, but fails with your 
> patch.
> 
> 
> Thanks,
> 
> Jacek

Interesting, I need to investigate it more, but I'm pretty sure that 
detached scopes are not supposed to be actual objects ever exposed to 
jscript code, which is why it's the way it is.

Honestly, I don't particularly care about toString, it's just the only 
builtin I could think off the top of my head that could be tested for 
this. Because right now we handle it in exec_source, which is only for 
interpreted functions... but it's a good test case of such behavior.

Do you think a fix like in 2/2 is more appropriate (also for quirks 
modes, but with null instead of undefined)? And perhaps fix whatever is 
causing toString to fail, which seems unrelated I guess.

That might mean moving it out of exec_source though.

Thanks,
Gabriel