[PATCH v6 0/2] MR11076: msxml3: Invoke onreadystatechange after a successful synchronous load.
Native MSXML fires the onreadystatechange handler synchronously from within IXMLDOMDocument::load()/loadXML() once parsing completes and readyState reaches 4. Hosts that set the handler before calling load() and expect it to have run by the time control returns to the caller rely on this ordering; Wine currently invokes the handler only as part of the asynchronous parse path, so synchronous loads silently skip it. This is MR 1 out of 4 for https://bugs.winehq.org/show_bug.cgi?id=33474 -- v6: msxml3/tests: Test synchronous invocation of onreadystatechange from loadXML(). https://gitlab.winehq.org/wine/wine/-/merge_requests/11076
From: Rose Hellsing <rose@pinkro.se> Native MSXML fires the onreadystatechange handler synchronously from within IXMLDOMDocument::load()/loadXML() once parsing completes and readyState reaches 4. Hosts that set the handler before calling load() and expect it to have run by the time control returns to the caller rely on this ordering; Wine currently invokes the handler only as part of the asynchronous parse path, so synchronous loads silently skip it. Signed-off-by: Rose Hellsing <rose@pinkro.se> --- dlls/msxml3/domdoc.c | 49 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c index 4185229a097..05ba1ad2054 100644 --- a/dlls/msxml3/domdoc.c +++ b/dlls/msxml3/domdoc.c @@ -134,6 +134,41 @@ static HRESULT set_doc_event(domdoc *doc, eventid_t eid, const VARIANT *v) return S_OK; } +/* Invoke a registered event handler (e.g. onreadystatechange) with no arguments. + * Called after a successful synchronous load to match native MSXML behavior. */ +static void fire_doc_event(domdoc *doc, eventid_t eid) +{ + DISPPARAMS dp = {NULL, NULL, 0, 0}; + IDispatchEx *dispex; + IDispatch *disp; + HRESULT hr; + + disp = doc->events[eid]; + if (!disp) return; + + IDispatch_AddRef(disp); + + TRACE("firing event %d on %p\n", eid, disp); + + hr = IDispatch_QueryInterface(disp, &IID_IDispatchEx, (void **)&dispex); + if (SUCCEEDED(hr)) + { + hr = IDispatchEx_InvokeEx(dispex, DISPID_VALUE, LOCALE_SYSTEM_DEFAULT, + DISPATCH_METHOD, &dp, NULL, NULL, NULL); + IDispatchEx_Release(dispex); + } + else + { + hr = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, LOCALE_SYSTEM_DEFAULT, + DISPATCH_METHOD, &dp, NULL, NULL, NULL); + } + + if (FAILED(hr)) + TRACE("event handler returned %#lx\n", hr); + + IDispatch_Release(disp); +} + static inline ConnectionPoint *impl_from_IConnectionPoint(IConnectionPoint *iface) { return CONTAINING_RECORD(iface, ConnectionPoint, IConnectionPoint_iface); @@ -1283,6 +1318,9 @@ static HRESULT WINAPI domdoc_load(IXMLDOMDocument3 *iface, VARIANT source, VARIA TRACE("failed to parse document\n"); } + if (*result == VARIANT_TRUE) + fire_doc_event(doc, EVENTID_READYSTATECHANGE); + return doc->error == S_OK ? S_OK : S_FALSE; default: FIXME("unhandled SAFEARRAY dim: %d\n", dim); @@ -1317,6 +1355,9 @@ static HRESULT WINAPI domdoc_load(IXMLDOMDocument3 *iface, VARIANT source, VARIA if (SUCCEEDED(hr)) *result = VARIANT_TRUE; + if (*result == VARIANT_TRUE) + fire_doc_event(doc, EVENTID_READYSTATECHANGE); + return hr; } } @@ -1331,6 +1372,8 @@ static HRESULT WINAPI domdoc_load(IXMLDOMDocument3 *iface, VARIANT source, VARIA if (hr == S_OK) *result = VARIANT_TRUE; ISequentialStream_Release(stream); + if (*result == VARIANT_TRUE) + fire_doc_event(doc, EVENTID_READYSTATECHANGE); return hr; } @@ -1388,6 +1431,9 @@ static HRESULT WINAPI domdoc_load(IXMLDOMDocument3 *iface, VARIANT source, VARIA hr = S_FALSE; } + if (*result == VARIANT_TRUE) + fire_doc_event(doc, EVENTID_READYSTATECHANGE); + TRACE("hr %#lx.\n", hr); return hr; @@ -1511,6 +1557,9 @@ static HRESULT WINAPI domdoc_loadXML(IXMLDOMDocument3 *iface, BSTR data, VARIANT hr = S_FALSE; } + if (*result == VARIANT_TRUE) + fire_doc_event(doc, EVENTID_READYSTATECHANGE); + return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11076
From: Rose Hellsing <rose@pinkro.se> Verify that put_onreadystatechange on a freshly created DOMDocument is invoked synchronously from loadXML() before returning to the caller, that readyState reaches 4, and that a failed parse on a separate document does not invoke the handler. Signed-off-by: Rose Hellsing <rose@pinkro.se> --- dlls/msxml3/tests/domdoc.c | 153 ++++++++++++++++++++++++++++++++++--- 1 file changed, 143 insertions(+), 10 deletions(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index 5464a76aa38..336c6289e79 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -157,6 +157,8 @@ typedef struct LONG ref; } dispevent; +static int g_expectedcall, g_unexpectedcall; + static inline dispevent *impl_from_IDispatch( IDispatch *iface ) { return CONTAINING_RECORD(iface, dispevent, IDispatch_iface); @@ -198,7 +200,7 @@ static ULONG WINAPI dispevent_Release(IDispatch *iface) static HRESULT WINAPI dispevent_GetTypeInfoCount(IDispatch *iface, UINT *pctinfo) { - ok(0, "unexpected call\n"); + g_unexpectedcall++; *pctinfo = 0; return S_OK; } @@ -206,14 +208,14 @@ static HRESULT WINAPI dispevent_GetTypeInfoCount(IDispatch *iface, UINT *pctinfo static HRESULT WINAPI dispevent_GetTypeInfo(IDispatch *iface, UINT iTInfo, LCID lcid, ITypeInfo **ppTInfo) { - ok(0, "unexpected call\n"); + g_unexpectedcall++; return S_OK; } static HRESULT WINAPI dispevent_GetIDsOfNames(IDispatch *iface, REFIID riid, LPOLESTR *rgszNames, UINT cNames, LCID lcid, DISPID *rgDispId) { - ok(0, "unexpected call\n"); + g_unexpectedcall++; return S_OK; } @@ -221,7 +223,20 @@ static HRESULT WINAPI dispevent_Invoke(IDispatch *iface, DISPID member, REFIID r LCID lcid, WORD flags, DISPPARAMS *params, VARIANT *result, EXCEPINFO *excepInfo, UINT *argErr) { - ok(0, "unexpected call\n"); + ok(!member, "expected 0 member, got %ld.\n", member); + ok(lcid == LOCALE_SYSTEM_DEFAULT, "expected LOCALE_SYSTEM_DEFAULT, got lcid %lx.\n", lcid); + ok(flags == DISPATCH_METHOD, "expected DISPATCH_METHOD, got %d\n", flags); + + ok(params->cArgs == 0, "got %d\n", params->cArgs); + ok(params->cNamedArgs == 0, "got %d\n", params->cNamedArgs); + ok(params->rgvarg == NULL, "got %p\n", params->rgvarg); + ok(params->rgdispidNamedArgs == NULL, "got %p\n", params->rgdispidNamedArgs); + + ok(result == NULL, "got %p\n", result); + ok(excepInfo == NULL, "got %p\n", excepInfo); + ok(argErr == NULL, "got %p\n", argErr); + + g_expectedcall++; return E_FAIL; } @@ -1509,12 +1524,6 @@ static void test_domdoc( void ) doc = create_document(&IID_IXMLDOMDocument); if (!doc) return; -if (0) -{ - /* crashes on native */ - IXMLDOMDocument_loadXML( doc, (BSTR)0x1, NULL ); -} - /* try some stupid things */ hr = IXMLDOMDocument_loadXML( doc, NULL, NULL ); ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); @@ -8956,6 +8965,128 @@ static void test_events(void) IXMLDOMDocument_Release(doc); } +static void test_onreadystatechange_load_stream(void) +{ + IXMLDOMDocument *doc; + IStream *stream; + HGLOBAL mem; + IDispatch *event; + VARIANT v_src; + VARIANT v; + VARIANT_BOOL b; + HRESULT hr; + char xml[] = "<?xml version=\"1.0\"?><root/>"; + size_t len = strlen(xml); + + doc = create_document(&IID_IXMLDOMDocument); + event = create_dispevent(); + + V_VT(&v) = VT_DISPATCH; + V_DISPATCH(&v) = event; + hr = IXMLDOMDocument_put_onreadystatechange(doc, v); + ok(hr == S_OK, "put_onreadystatechange returned %#lx.\n", hr); + + mem = GlobalAlloc(0, len); + memcpy(mem, xml, len); + hr = CreateStreamOnHGlobal(mem, TRUE, &stream); + ok(hr == S_OK, "CreateStreamOnHGlobal returned %#lx.\n", hr); + + g_expectedcall = 0; + g_unexpectedcall = 0; + VariantInit(&v_src); + V_VT(&v_src) = VT_UNKNOWN; + V_UNKNOWN(&v_src) = (IUnknown*)stream; + hr = IXMLDOMDocument_load(doc, v_src, &b); + ok(hr == S_OK, "load returned %#lx.\n", hr); + ok(b == VARIANT_TRUE, "load set b to %#hx.\n", b); + ok(g_expectedcall >= 1, "expected onreadystatechange to fire at least once, got %d.\n", g_expectedcall); + ok(g_unexpectedcall == 0, "got %d unexpected IDispatch calls.\n", g_unexpectedcall); + + IStream_Release(stream); + IDispatch_Release(event); + IXMLDOMDocument_Release(doc); +} + +static void test_onreadystatechange_sync(void) +{ + static const WCHAR xml[] = L"<?xml version=\"1.0\"?><root/>"; + static const WCHAR garbage[] = L"this is not xml at all"; + IXMLDOMDocument *doc; + IDispatch *event; + VARIANT_BOOL b; + HRESULT hr; + VARIANT v; + LONG state; + BSTR str; + + /* MSXML invokes onreadystatechange synchronously from loadXML() + * before returning to the caller. The handler may fire at every + * readyState transition (UNINITIALIZED>LOADING>LOADED>INTERACTIVE> + * COMPLETED) so we only assert that it fired at least once and that + * readyState reached 4 by the time loadXML() returns. */ + doc = create_document(&IID_IXMLDOMDocument); + + event = create_dispevent(); + V_VT(&v) = VT_DISPATCH; + V_DISPATCH(&v) = event; + hr = IXMLDOMDocument_put_onreadystatechange(doc, v); + ok(hr == S_OK, "put_onreadystatechange returned %#lx.\n", hr); + + g_expectedcall = 0; + g_unexpectedcall = 0; + + str = SysAllocString(xml); + b = VARIANT_FALSE; + hr = IXMLDOMDocument_loadXML(doc, str, &b); + SysFreeString(str); + ok(hr == S_OK, "loadXML returned %#lx.\n", hr); + ok(b == VARIANT_TRUE, "loadXML set success to %d.\n", b); + + ok(g_expectedcall >= 1, "expected onreadystatechange to fire at least once, got %d.\n", g_expectedcall); + ok(g_unexpectedcall == 0, "got %d unexpected IDispatch calls.\n", g_unexpectedcall); + + state = -1; + hr = IXMLDOMDocument_get_readyState(doc, &state); + ok(hr == S_OK, "get_readyState returned %#lx.\n", hr); + ok(state == 4, "expected readyState 4 after successful loadXML, got %ld.\n", state); + + V_VT(&v) = VT_DISPATCH; + V_DISPATCH(&v) = NULL; + hr = IXMLDOMDocument_put_onreadystatechange(doc, v); + ok(hr == S_OK, "put_onreadystatechange(NULL) returned %#lx.\n", hr); + + IDispatch_Release(event); + IXMLDOMDocument_Release(doc); + + /* A failed parse on a fresh document must not invoke the handler. */ + doc = create_document(&IID_IXMLDOMDocument); + + event = create_dispevent(); + V_VT(&v) = VT_DISPATCH; + V_DISPATCH(&v) = event; + hr = IXMLDOMDocument_put_onreadystatechange(doc, v); + ok(hr == S_OK, "put_onreadystatechange returned %#lx.\n", hr); + + g_expectedcall = 0; + g_unexpectedcall = 0; + + str = SysAllocString(garbage); + b = VARIANT_TRUE; + hr = IXMLDOMDocument_loadXML(doc, str, &b); + SysFreeString(str); + ok(hr == S_FALSE, "loadXML on invalid xml returned %#lx.\n", hr); + ok(b == VARIANT_FALSE, "loadXML on invalid xml set success to %d.\n", b); + ok(g_unexpectedcall == 0, "got %d unexpected IDispatch calls.\n", g_unexpectedcall); + + V_VT(&v) = VT_DISPATCH; + V_DISPATCH(&v) = NULL; + hr = IXMLDOMDocument_put_onreadystatechange(doc, v); + ok(hr == S_OK, "put_onreadystatechange(NULL) returned %#lx.\n", hr); + + IDispatch_Release(event); + IXMLDOMDocument_Release(doc); +} + static void test_createProcessingInstruction(void) { static const WCHAR xml1[] = L"<?xml version=\"1.0\"?>\r\n<test/>\r\n"; @@ -17857,6 +17988,8 @@ START_TEST(domdoc) test_default_properties(); test_selectSingleNode(); test_events(); + test_onreadystatechange_load_stream(); + test_onreadystatechange_sync(); test_put_nodeTypedValue(); test_get_xml(); test_insertBefore(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11076
Native MSXML fires the onreadystatechange handler synchronously from within IXMLDOMDocument::load()/loadXML() once parsing completes and readyState reaches 4. Hosts that set the handler before calling load() and expect it to have run by the time control returns to the caller rely on this ordering; Wine currently invokes the handler only as part of the asynchronous parse path, so synchronous loads silently skip it.
I don't get this. Was this description partially generated? We don't use handlers at all at the moment. And there is no "async parse path". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_149075
participants (3)
-
Nikolay Sivov (＠nsivov) -
Rose Hellsing
-
Rose Hellsing (＠axtlos)