[PATCH v4 0/1] MR3391: mshtml: implement window.MutationObserver with MutationObserver stub
This get us pass the "Update your browser" blocker in adobe's sign-in page. The page itself doesn't make use of `window.MutationObserver`. However the sign-in page is still broken. -- v4: mshtml: implement window.MutationObserver with MutationObserver stub https://gitlab.winehq.org/wine/wine/-/merge_requests/3391
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/mshtml/htmlwindow.c | 14 +++ dlls/mshtml/mshtml_private.h | 6 +- dlls/mshtml/mshtml_private_iface.idl | 13 +++ dlls/mshtml/mutation.c | 143 +++++++++++++++++++++++++++ dlls/mshtml/tests/documentmode.js | 1 + 5 files changed, 176 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index c6072c7e9fa..324da23ed68 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3330,6 +3330,19 @@ static HRESULT WINAPI window_private_get_console(IWineHTMLWindowPrivate *iface, return S_OK; } +static HRESULT WINAPI window_private_MutationObserver(IWineHTMLWindowPrivate *iface, IDispatch *callback, IDispatch **mutation_observer) +{ + HTMLWindow *This = impl_from_IWineHTMLWindowPrivateVtbl(iface); + IWineMSHTMLMutationObserver *ret = NULL; + + FIXME("iface %p, mutation_observer %p, stub.\n", iface, mutation_observer); + + create_mutation_observer(dispex_compat_mode(&This->inner_window->event_target.dispex), callback, &ret); + *mutation_observer = (IDispatch *)ret; + + return S_OK; +} + static const IWineHTMLWindowPrivateVtbl WineHTMLWindowPrivateVtbl = { window_private_QueryInterface, window_private_AddRef, @@ -3343,6 +3356,7 @@ static const IWineHTMLWindowPrivateVtbl WineHTMLWindowPrivateVtbl = { window_private_get_console, window_private_matchMedia, window_private_postMessage, + window_private_MutationObserver }; static inline HTMLWindow *impl_from_IWineHTMLWindowCompatPrivateVtbl(IWineHTMLWindowCompatPrivate *iface) diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 59d652a828b..dc6b38dbc6a 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -294,7 +294,8 @@ typedef struct EventTarget EventTarget; XIID(IWinePageTransitionEvent) \ XIID(IWineXMLHttpRequestPrivate) \ XIID(IWineMSHTMLConsole) \ - XIID(IWineMSHTMLMediaQueryList) + XIID(IWineMSHTMLMediaQueryList) \ + XIID(IWineMSHTMLMutationObserver) typedef enum { #define XIID(iface) iface ## _tid, @@ -1479,3 +1480,6 @@ IInternetSecurityManager *get_security_manager(void); extern HINSTANCE hInst; void create_console(compat_mode_t compat_mode, IWineMSHTMLConsole **ret); HRESULT create_media_query_list(HTMLWindow *window, BSTR media_query, IDispatch **ret); + +void create_mutation_observer(compat_mode_t compat_mode, IDispatch *callback, + IWineMSHTMLMutationObserver **ret); diff --git a/dlls/mshtml/mshtml_private_iface.idl b/dlls/mshtml/mshtml_private_iface.idl index c0bb30fbbc8..90e8846adb1 100644 --- a/dlls/mshtml/mshtml_private_iface.idl +++ b/dlls/mshtml/mshtml_private_iface.idl @@ -76,6 +76,17 @@ interface IWineMSHTMLConsole : IDispatch HRESULT warn([in, optional] VARIANT *varargStart); } +[ + odl, + oleautomation, + dual, + hidden, + uuid(6ac5491e-1758-4b82-98a2-83e31a7c8871) +] +interface IWineMSHTMLMutationObserver : IDispatch +{ +} + [ odl, oleautomation, @@ -114,6 +125,8 @@ interface IWineHTMLWindowPrivate : IDispatch HRESULT matchMedia([in] BSTR media_query, [retval, out] IDispatch **media_query_list); [id(54)] HRESULT postMessage([in] VARIANT msg, [in] BSTR targetOrigin, [in, optional] VARIANT transfer); + [id(55)] + HRESULT MutationObserver([in] IDispatch *callback, [retval, out] IDispatch **observer); } [ diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c index 83ab623aea8..af019d9c056 100644 --- a/dlls/mshtml/mutation.c +++ b/dlls/mshtml/mutation.c @@ -1076,3 +1076,146 @@ void init_mutation(nsIComponentManager *component_manager) if(NS_FAILED(nsres)) ERR("Could not create nsIContentUtils instance: %08lx\n", nsres); } + +struct mutation_observer { + IWineMSHTMLMutationObserver IWineMSHTMLMutationObserver_iface; + + LONG ref; + DispatchEx dispex; + IDispatch *callback; + + HTMLDOMNode *node; +}; + +static inline struct mutation_observer *impl_from_IWineMSHTMLMutationObserver(IWineMSHTMLMutationObserver *iface) +{ + return CONTAINING_RECORD(iface, struct mutation_observer, IWineMSHTMLMutationObserver_iface); +} + +static HRESULT WINAPI MutationObserver_QueryInterface(IWineMSHTMLMutationObserver *iface, REFIID riid, void **ppv) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + + TRACE("(%p)->(%s %p)\n", This, debugstr_mshtml_guid(riid), ppv); + + if(IsEqualGUID(&IID_IUnknown, riid) || IsEqualGUID(&IID_IWineMSHTMLMutationObserver, riid)) { + *ppv = &This->IWineMSHTMLMutationObserver_iface; + }else { + WARN("(%p)->(%s %p)\n", This, debugstr_mshtml_guid(riid), ppv); + *ppv = NULL; + return E_NOINTERFACE; + } + + IUnknown_AddRef((IUnknown*)*ppv); + return S_OK; +} + +static ULONG WINAPI MutationObserver_AddRef(IWineMSHTMLMutationObserver *iface) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + LONG ref = InterlockedIncrement(&This->ref); + + TRACE("(%p) ref=%ld\n", This, ref); + + return ref; +} + +static ULONG WINAPI MutationObserver_Release(IWineMSHTMLMutationObserver *iface) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + LONG ref = InterlockedDecrement(&This->ref); + + TRACE("(%p) ref=%ld\n", This, ref); + + if(!ref) { + if(This->node) { + IHTMLDOMNode_Release(&This->node->IHTMLDOMNode_iface); + This->node = NULL; + } + release_dispex(&This->dispex); + IDispatch_Release(This->callback); + This->callback = NULL; + free(This); + } + + return ref; +} + +static HRESULT WINAPI MutationObserver_GetTypeInfoCount(IWineMSHTMLMutationObserver *iface, UINT *pctinfo) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + FIXME("(%p)->(%p)\n", This, pctinfo); + return E_NOTIMPL; +} + +static HRESULT WINAPI MutationObserver_GetTypeInfo(IWineMSHTMLMutationObserver *iface, UINT iTInfo, + LCID lcid, ITypeInfo **ppTInfo) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + + return IDispatchEx_GetTypeInfo(&This->dispex.IDispatchEx_iface, iTInfo, lcid, ppTInfo); +} + +static HRESULT WINAPI MutationObserver_GetIDsOfNames(IWineMSHTMLMutationObserver *iface, REFIID riid, + LPOLESTR *rgszNames, UINT cNames, LCID lcid, + DISPID *rgDispId) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + + return IDispatchEx_GetIDsOfNames(&This->dispex.IDispatchEx_iface, riid, rgszNames, cNames, lcid, + rgDispId); +} + +static HRESULT WINAPI MutationObserver_Invoke(IWineMSHTMLMutationObserver *iface, DISPID dispIdMember, + REFIID riid, LCID lcid, WORD wFlags, DISPPARAMS *pDispParams, + VARIANT *pVarResult, EXCEPINFO *pExcepInfo, UINT *puArgErr) +{ + struct mutation_observer *This = impl_from_IWineMSHTMLMutationObserver(iface); + + return IDispatchEx_Invoke(&This->dispex.IDispatchEx_iface, dispIdMember, riid, lcid, wFlags, + pDispParams, pVarResult, pExcepInfo, puArgErr); +} + +static const IWineMSHTMLMutationObserverVtbl WineMSHTMLMutationObserverVtbl = { + MutationObserver_QueryInterface, + MutationObserver_AddRef, + MutationObserver_Release, + MutationObserver_GetTypeInfoCount, + MutationObserver_GetTypeInfo, + MutationObserver_GetIDsOfNames, + MutationObserver_Invoke, +}; + +static const tid_t mutation_observer_iface_tids[] = { + IWineMSHTMLMutationObserver_tid, + 0 +}; +static dispex_static_data_t console_dispex = { + L"MutationObserver", + NULL, + IWineMSHTMLMutationObserver_tid, + mutation_observer_iface_tids +}; + +void create_mutation_observer(compat_mode_t compat_mode, IDispatch *callback, + IWineMSHTMLMutationObserver **ret) { + struct mutation_observer *obj; + + TRACE("(compat_mode = %d, callback = %p, ret = %p)\n", compat_mode, callback, ret); + + obj = calloc(1, sizeof(*obj)); + if(!obj) + { + ERR("No memory.\n"); + return; + } + + obj->IWineMSHTMLMutationObserver_iface.lpVtbl = &WineMSHTMLMutationObserverVtbl; + obj->ref = 1; + init_dispatch(&obj->dispex, (IUnknown*)&obj->IWineMSHTMLMutationObserver_iface.lpVtbl, + &console_dispex, compat_mode); + + IDispatch_AddRef(callback); + obj->callback = callback; + *ret = &obj->IWineMSHTMLMutationObserver_iface; +} diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 00a6eded3fe..9f8955d87d3 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -475,6 +475,7 @@ sync_test("window_props", function() { test_exposed("performance", true); test_exposed("console", v >= 10); test_exposed("matchMedia", v >= 10); + test_exposed("MutationObserver", v >= 11); }); sync_test("domimpl_props", function() { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3391
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=135159 Your paranoid android. === w10pro64 (64 bit report) === mshtml: htmldoc.c:351: Test failed: expected Exec_SETTITLE === debian11 (32 bit report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit ar:MA report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit de report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit fr report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit he:IL report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit hi:IN report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit ja:JP report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit zh:CN report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11b (32 bit WoW report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11b (64 bit WoW report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window."
BTW, it's not exactly needed for your use case, but I'd add a stub construction. i.e. add a: ```c static const dispex_static_data_vtbl_t mutation_observer_ctor_dispex_vtbl = { mutation_observer_ctor_value }; ``` and the `mutation_observer_ctor_value` would just be a FIXME "stub". This will help debugging easier to see if it's *actually* used to construct anything. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39953
On Mon Jul 24 14:42:16 2023 +0000, **** wrote:
Marvin replied on the mailing list: ``` Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=135159 Your paranoid android. === w10pro64 (64 bit report) === mshtml: htmldoc.c:351: Test failed: expected Exec_SETTITLE === debian11 (32 bit report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit ar:MA report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit de report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit fr report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit he:IL report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit hi:IN report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit ja:JP report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11 (32 bit zh:CN report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11b (32 bit WoW report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." === debian11b (64 bit WoW report) === mshtml: script.c:1126: Test failed: L"/index.html?10:window_props: MutationObserver found in window." ``` How do I make sure `MutationObserver` is only exposed for compat mode IE11?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39955
Note that the result should likely be cached in the inner window, so that you return the same constructor everytime
This function is the function that constructs `MutationObserver`s, and each `MutationObserver` is independent from each other, that's why I didn't use a cache. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39956
On Mon Jul 24 14:56:49 2023 +0000, Yuxuan Shui wrote:
Note that the result should likely be cached in the inner window, so that you return the same constructor everytime This function is the function that constructs `MutationObserver`s, (note it's not `propget`), and each `MutationObserver` is independent from each other, that's why I didn't use a cache. Thanks for the tip about compat mode!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39957
On Mon Jul 24 13:43:06 2023 +0000, Gabriel Ivăncescu wrote:
I don't think this is needed here? Since you only add the constructor? This (empty) interface is for the _instance_ created from the constructor. So you don't need `IWineMSHTMLMutationObserver`. No, I implemented the constructor function, I didn't just return an empty constructor from `MutationObserver`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39958
On Mon Jul 24 13:43:06 2023 +0000, Gabriel Ivăncescu wrote:
Likewise to prevent confusion I'd rename this to make it known it's a constructor so maybe `create_mutation_observer_ctor`? Also in this case you don't pass the callback when creating the constructor, you pass it when instantiating something from the constructor (but since you make it a stub, you don't need that now). Same as above.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39959
On Mon Jul 24 13:43:06 2023 +0000, Gabriel Ivăncescu wrote:
Since it's a getter now that gets the constructor, name it `window_private_get_MutationObserver`. Ditto
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39960
On Mon Jul 24 13:43:07 2023 +0000, Gabriel Ivăncescu wrote:
Ok so I will comment in general here for the entire thing. First name it `mutation_observer_ctor` or something like that to make it known it's a constructor and **not** the instance created from it. Secondly, this is not a DOM node (and anyway what you did was wrong and you didn't even make use of it), so you can simply remove the entire node thing. For an example of constructors you can look at e.g. `HTMLXMLHttpRequestFactory`. You just need the dispex, a ref and probably an IUnknown iface to implement it (and the dispex qi). IUnknown because I don't think this constructor exposes any props or methods… (IUnknown instead of your IWineMSHTMLMutationObserver basically, since that's for the _instance_ not the constructor). If you make it IUnknown you'll have to return the dispex's IDispatchEx iface when returning the constructor as an IDispatch, obviously. This will avoid having to add forwarding methods like GetIDsOfNames and so on. If you want to actually implement construction you'll have to use the dispex vtbl's value (see the other constructors like XMLHttpRequestFactory) but you can set that aside for now since you just expose it as stub. I think there is some misunderstanding of my intention. I didn't want to just return an empty constructor, I did implement the constructor. (hence the `callback` parameter, and the lack of `propget`). It's the constructed `MutationObserver` that is a stub.
This should resolve the stub construction comment below as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3391#note_39961
participants (4)
-
Gabriel Ivăncescu -
Marvin -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)