[PATCH v3 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 -- v3: 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 | 81 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index a33c4f5c6f2..1428ae567e0 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -8845,6 +8845,86 @@ static void test_events(void) 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; + + /* Native 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"; @@ -17746,6 +17826,7 @@ START_TEST(domdoc) test_default_properties(); test_selectSingleNode(); test_events(); + test_onreadystatechange_sync(); test_put_nodeTypedValue(); test_get_xml(); test_insertBefore(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11076
Nikolay Sivov (@nsivov) commented about dlls/msxml3/tests/domdoc.c:
+ 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; + + /* Native 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); Please reword this one, and implementation comments, to get rid of "to match native" clarification. It's assumed that we are trying to match it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_142354
Nikolay Sivov (@nsivov) commented about dlls/msxml3/domdoc.c:
+ 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); + } Where does DispatchEx come from? Our test sink does not have it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_142355
Nikolay Sivov (@nsivov) commented about dlls/msxml3/domdoc.c:
if (SUCCEEDED(hr)) *result = VARIANT_TRUE;
+ if (*result == VARIANT_TRUE) + fire_doc_event(doc, EVENTID_READYSTATECHANGE); + return hr;
The tests cover only loadXML(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_142356
On Sat Jun 6 16:25:10 2026 +0000, Nikolay Sivov wrote:
Where does DispatchEx come from? Our test sink does not have it. DispatchEx would come from whatever is using msxml, in the tests case it only implement IDispatch so the if statement falls through to the else body and uses IDispatch_Invoke.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_142406
On Mon Jun 8 11:42:28 2026 +0000, Rose Hellsing wrote:
DispatchEx would come from whatever is using msxml, in the tests case it only implement IDispatch so the if statement falls through to the else body and uses IDispatch_Invoke. I can add a test for DispatchEx if that's wanted, it'd mostly duplicate the IDispatch patterns.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_142407
participants (3)
-
Nikolay Sivov (＠nsivov) -
Rose Hellsing
-
Rose Hellsing (＠axtlos)