[PATCH 0/1] MR3443: jscript: Call the getter with the proper 'this' in invoke_prop_func.

[PATCH 1/1] jscript: Call the getter with the proper 'this' in invoke_prop_func.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/jscript/dispex.c    | 11 ++++++++++-
 dlls/mshtml/tests/es5.js |  5 +++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c
index 2da2270fb0f..185a7954d03 100644
--- a/dlls/jscript/dispex.c
+++ b/dlls/jscript/dispex.c
@@ -579,7 +579,16 @@ static HRESULT invoke_prop_func(jsdisp_t *This, IDispatch *jsthis, dispex_prop_t
     case PROP_IDX: {
         jsval_t val;

-        hres = prop_get(This, prop, &val);
+        if(prop->type == PROP_IDX)
+            hres = This->builtin_info->idx_get(This, prop->u.idx, &val);
+        else if(prop->u.accessor.getter)
+            hres = jsdisp_call_value(prop->u.accessor.getter, jsval_disp(jsthis ? jsthis : (IDispatch*)&This->IDispatchEx_iface),
+                                     DISPATCH_METHOD, 0, NULL, &val);
+        else {
+            val = jsval_undefined();
+            hres = S_OK;
+        }
+
         if(FAILED(hres))
             return hres;

diff --git a/dlls/mshtml/tests/es5.js b/dlls/mshtml/tests/es5.js
index e86ab1dbe77..cef3c77c207 100644
--- a/dlls/mshtml/tests/es5.js
+++ b/dlls/mshtml/tests/es5.js
@@ -794,6 +794,7 @@ sync_test("defineProperty", function() {
     /* call prop with getter */
     desc = {
         get: function() {
+            ok(this === obj, "this != obj");
             return function(x) {
                 ok(x === 100, "x = " + x);
                 return 10;
@@ -804,6 +805,10 @@ sync_test("defineProperty", function() {
     test_accessor_prop_desc(obj, "funcprop", desc);
     ok(obj.funcprop(100) === 10, "obj.funcprop() = " + obj.funcprop(100));

+    Object.defineProperty(child.prototype, "funcprop_prot", desc);
+    test_accessor_prop_desc(child.prototype, "funcprop_prot", desc);
+    ok(obj.funcprop_prot(100) === 10, "obj.funcprop_prot() = " + obj.funcprop_prot(100));
+
     expect_exception(function() {
         Object.defineProperty(null, "funcprop", desc);
     }, JS_E_OBJECT_EXPECTED);

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/jscript/dispex.c:
>  case PROP_IDX: {
>      jsval_t val;
>
> -
>    hres = prop_get(This, prop, &val);
> -
>    if(prop->type == PROP_IDX)

I think it would be better to add additional `jsthis` argument to `prop_get` instead of duplicating it here.

[Gabriel Ivăncescu (＠insn)]

On Mon Jul 31 11:34:13 2023 +0000, Jacek Caban wrote:
> I think it would be better to add additional `jsthis` argument to
> `prop_get` instead of duplicating it here.

But this is the only place it's used in, and the prototype lookup in
prop_get only makes sense when we don't have a `jsthis` in the first
place, it feels like overcomplicating that helper.

[Gabriel Ivăncescu (＠insn)]

On Mon Jul 31 14:10:15 2023 +0000, Gabriel Ivăncescu wrote:
> But this is the only place it's used in, and the prototype lookup in
> prop_get only makes sense when we don't have a `jsthis` in the first
> place, it feels like overcomplicating that helper.

Hmm I found another somewhat related issue with our builtins. I guess
I'll go with this then and fix it in a separate commit.