[PATCH v2 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 -- v2: msxml3/tests: Test synchronous invocation of onreadystatechange from loadXML(). msxml3: Invoke onreadystatechange after a successful synchronous load. 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..cb4eef13cf5 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_expectedcall == 0, "handler must not fire on parse failure (got %d calls).\n", g_expectedcall); + + 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
participants (2)
-
Rose Hellsing -
Rose Hellsing (＠axtlos)