On Wed Jun 5 21:57:54 2024 +0000, Jacek Caban wrote:
But it was still ambiguous as I explained.
If you mean the "zoom" example, I'd appreciate for some details. That one should already use `IHTMLCSSStyleDeclaration` interface, not `IHTMLStyle*` ones.
But since we pass the mshtml-internal tid (called iid in args) which
is opaque to jscript, I don't really see why this is better than passing the `func_info_t` directly? Well IMO passing the pair is already more complex and requires more lookups, more args, etc, but that's just me. `get_dispex_from_this` is one thing, going through the interface take cares of that. For that part, you could just replace the argument in my branch. But we should also somehow make sure that the hook is safe to call. By binding function info to the exact object instance, we guarantee that it's safe to call `impl_from_*` from hooks. If you accept any function info as an input, you need to somehow validate that it's safe to call it. Something like moving QI earlier would only partially mitigate that. I'm open to other approaches, but unless I'm missing something, there is no such validation in your current branch. It seems to me that things like 1ffeaa446613 come from some variation of solution to that problem. Maybe it was needed in earlier version, but it shouldn't be needed with "this" being resolved on jscript side. It's no longer arbitrary when it's used for a `IJSDispatchHost` call. I would suggest to try removing it.
Yeah, I forgot to validate it in my branch. In BuiltinFuncCall, something like this would work:
```c if(func->hook) { hres = IWineJSDispatchHost_QueryInterface(&This->IWineJSDispatchHost_iface, tid_ids[func->tid], (void**)&unk); if(FAILED(hres)) return hres; IUnknown_Release(unk);
hres = func->hook(This, setter ? DISPATCH_PROPERTYPUT : DISPATCH_PROPERTYGET, dp, ret, ei, caller); if(hres != S_FALSE) return hres; } ```
Alternatively, we can pass around `func_info_t` instead of dispid and tid, but still look it up like in your branch. We can obtain the dispid and tid from `func_info_t` so that's all that's needed. I'm not sure if that's an improvement over my approach, but I think it's definitely a simplification over yours. What do you think?
BTW for `zoom`, the assignment should fail as it does on native (as the test does), are you sure it's supposed to be using `IHTMLCSSStyleDeclaration` which doesn't fail? I mean the test will simply start failing if it picks `IHTMLCSSStyleDeclaration` because the assignment does not throw exception anymore (when it should be!).
-
Gabriel Ivăncescu (＠insn)