On Wed Feb 26 15:59:27 2025 +0000, Gabriel Ivăncescu wrote:
Jacek Caban commented on a discussion on dlls/jscript/jsutils.c: https://gitlab.winehq.org/wine/wine/-/merge_requests/7425#note_96014
hres =IWineJSDispatchHost_GetJSDispatch(disp_host, &jsdisp_iface); IWineJSDispatchHost_Release(disp_host); if(SUCCEEDED(hres)) {
jsdisp_t *jsdisp = as_jsdisp((IDispatch*)jsdisp_iface);
if(jsdisp->ctx == ctx) {*r = jsval_obj(jsdisp);return S_OK;}else {jsdisp_release(jsdisp);}
*r = jsval_obj(as_jsdisp((IDispatch*)jsdisp_iface));
return S_OK;I can try to add a test, but it's obviously wrong the way it is now (see below).
Sure, the direction is clear. In this case it's not a question what Windows does, but to make sure Wine works properly and avoid regressions in the future.
Either way, I don't think the fix for lack of context propagation should be done here, it feels like it's unrelated and a bit of a hack, even if it's missing (the propagation).
Well, `IDispatchEx` code path gets it right, so it would be a regression.
Wait, how would it be a regression? The important parts (that end up in Invoke) will still end up through the IDispatch since they have different contexts. I don't see why mshtml objects (that should be jsdisps) should be treated differently than "native" jscript objects? It's not like making them jsdisps will forcefully ignore the ctx checks and always end up in the jsdisp code path. Those will still be done where necessary, just like for generic jscript objects.
Actually I found another problem, while adding these tests (it's a fragile issue that the tests somehow triggered, not sure why exactly). Obviously I'll have to fix it before adding the tests since otherwise existing test will fail.
The failure is in `window_open_self_test` on the `iframe.contentWindow === iframe_window` check. So what happens is that a navigation creates a new jsdisp, new ctx and all. But same outer window and it's supposed to be the same.
Obviously the jsdisp needs to be tied, in theory, to the outer window, but it's not that simple, because script ctx will also be different. This issue is probably avoided now because we just use the outer window's Dispatch, but that's not right either (any code path that relies on jsdisp will fail, since outer window will be treated like a "host object disp" instead of a jsdisp). The simplest case is e.g. Object.prototype.toString which will return [object Object] by *design* and not due to a FIXME. I added such test now.
On my Proton patches from last year, I had to deal with this specifically as well. What I did was re-use the existing jsdisp from the older inner window, if any, when updating it to a new one.
But on top of that I had to add code to deal with this migration and to move it from one context to another (on jscript side), while unlinking all the props, which basically reset the jsdisp while keeping its memory address. I think I'll have to do something similar now.
Good thing I caught it now, might explain some weird things I saw with complex iframes and navigations.