[PATCH 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 -- 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 | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c index 4185229a097..8b31098d718 100644 --- a/dlls/msxml3/domdoc.c +++ b/dlls/msxml3/domdoc.c @@ -134,6 +134,44 @@ 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; + EXCEPINFO ei; + HRESULT hr; + + disp = doc->events[eid]; + if (!disp) return; + + IDispatch_AddRef(disp); + memset(&ei, 0, sizeof(ei)); + + 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, &ei, NULL); + IDispatchEx_Release(dispex); + } + else + { + UINT argerr; + hr = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, LOCALE_SYSTEM_DEFAULT, + DISPATCH_METHOD, &dp, NULL, &ei, &argerr); + } + + 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 +1321,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 +1358,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 +1375,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 +1434,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 +1560,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() once the parse completes and readyState reaches 4, and that a failed parse does not invoke the handler. Signed-off-by: Rose Hellsing <rose@pinkro.se> --- dlls/msxml3/tests/domdoc.c | 60 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c index a33c4f5c6f2..e0dbe1097fe 100644 --- a/dlls/msxml3/tests/domdoc.c +++ b/dlls/msxml3/tests/domdoc.c @@ -8845,6 +8845,65 @@ 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; + + 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); + + /* Native MSXML invokes onreadystatechange synchronously from loadXML(), + * before returning to the caller, when parsing succeeds. */ + 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 invocation count 1, 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); + + /* A failed parse must not invoke the handler. */ + g_expectedcall = 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 +17805,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)