[PATCH v4 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 -- v4: 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 | 130 +++++++++++++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 6 deletions(-) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index a33c4f5c6f2..6959642f033 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -1518,12 +1518,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); @@ -8845,6 +8839,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"; @@ -17746,6 +17862,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
I reworded the comments and added a new test function for domdoc_load. Didn't add tests for DispatchEx though yet, can do that as well if needed -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11076#note_142412
participants (2)
-
Rose Hellsing -
Rose Hellsing (＠axtlos)