[PATCH 0/4] MR5156: mshtml: Always use the event target dispex
This PR ensures that document mode isn't updated on the wrong document when a DOMContentLoaded event occurs. This is done by always using the event target dispex (instead of the dispex associated with doc associated with the event listener). This fixes the launcher for Swords of Legends Online. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Use generic event dispatcher instead of nsevent for the DOMContentLoaded event. Also allow processing before dispatching event. --- dlls/mshtml/htmldoc.c | 36 +++++++++++++++++++++++++++++++++++- dlls/mshtml/htmlevent.c | 14 +++++++++++++- dlls/mshtml/htmlevent.h | 1 + dlls/mshtml/nsevents.c | 22 ---------------------- 4 files changed, 49 insertions(+), 24 deletions(-) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 4267b13ffab..6e4da4e77df 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -6104,6 +6104,38 @@ static HRESULT HTMLDocumentNode_location_hook(DispatchEx *dispex, WORD flags, DI 0, flags, dp, res, ei, caller); } +static HRESULT HTMLDocumentNode_pre_handle_event(DispatchEx* dispex, eventid_t eid, nsIDOMEvent *event) +{ + HTMLDocumentNode *doc = impl_from_DispatchEx(dispex); + switch(eid) { + case EVENTID_DOMCONTENTLOADED: { + if(doc->window) + doc->window->dom_content_loaded_event_start_time = get_time_stamp(); + break; + } + default: + break; + } + + return S_OK; +} + +static HRESULT HTMLDocumentNode_handle_event(DispatchEx* dispex, eventid_t eid, nsIDOMEvent *event, BOOL *prevent_default) +{ + HTMLDocumentNode *doc = impl_from_DispatchEx(dispex); + switch(eid) { + case EVENTID_DOMCONTENTLOADED: { + if(doc->window) + doc->window->dom_content_loaded_event_end_time = get_time_stamp(); + break; + } + default: + break; + } + + return S_OK; +} + static const event_target_vtbl_t HTMLDocumentNode_event_target_vtbl = { { .query_interface = HTMLDocumentNode_query_interface, @@ -6118,8 +6150,10 @@ static const event_target_vtbl_t HTMLDocumentNode_event_target_vtbl = { .get_gecko_target = HTMLDocumentNode_get_gecko_target, .bind_event = HTMLDocumentNode_bind_event, .get_parent_event_target = HTMLDocumentNode_get_parent_event_target, + .handle_event = HTMLDocumentNode_handle_event, .get_cp_container = HTMLDocumentNode_get_cp_container, - .set_current_event = HTMLDocumentNode_set_current_event + .set_current_event = HTMLDocumentNode_set_current_event, + .pre_handle_event = HTMLDocumentNode_pre_handle_event, }; static const NodeImplVtbl HTMLDocumentFragmentImplVtbl = { diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index 1ae3a0de384..ec3653136b3 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -118,7 +118,7 @@ typedef struct { /* Keep these sorted case sensitively */ static const event_info_t event_info[] = { {L"DOMContentLoaded", EVENT_TYPE_EVENT, 0, - EVENT_BUBBLES | EVENT_CANCELABLE}, + EVENT_DEFAULTLISTENER | EVENT_HASDEFAULTHANDLERS | EVENT_BUBBLES | EVENT_CANCELABLE }, {L"abort", EVENT_TYPE_EVENT, DISPID_EVMETH_ONABORT, EVENT_BIND_TO_TARGET}, {L"afterprint", EVENT_TYPE_EVENT, DISPID_EVMETH_ONAFTERPRINT, @@ -5100,6 +5100,18 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, IEventTarget_AddRef(&event_target->IEventTarget_iface); event->phase = DEP_CAPTURING_PHASE; + + if(event_info[event->event_id].flags & EVENT_HASDEFAULTHANDLERS) { + for(i = 0; i < chain_cnt; i++) { + vtbl = dispex_get_vtbl(&target_chain[i]->dispex); + if(!vtbl->pre_handle_event) + continue; + hres = vtbl->pre_handle_event(&target_chain[i]->dispex, event->event_id, event->nsevent); + if(FAILED(hres) || event->stop_propagation) + break; + } + } + i = chain_cnt-1; while(!event->stop_propagation && i) call_event_handlers(target_chain[i--], event, dispatch_mode); diff --git a/dlls/mshtml/htmlevent.h b/dlls/mshtml/htmlevent.h index 645e99a4c19..e0bb6609e7b 100644 --- a/dlls/mshtml/htmlevent.h +++ b/dlls/mshtml/htmlevent.h @@ -134,6 +134,7 @@ typedef struct { HRESULT (*handle_event)(DispatchEx*,eventid_t,nsIDOMEvent*,BOOL*); ConnectionPointContainer *(*get_cp_container)(DispatchEx*); IHTMLEventObj *(*set_current_event)(DispatchEx*,IHTMLEventObj*); + HRESULT (*pre_handle_event)(DispatchEx*,eventid_t,nsIDOMEvent*); } event_target_vtbl_t; IHTMLEventObj *default_set_current_event(HTMLInnerWindow*,IHTMLEventObj*); diff --git a/dlls/mshtml/nsevents.c b/dlls/mshtml/nsevents.c index 482d3196770..ec4d1f6694a 100644 --- a/dlls/mshtml/nsevents.c +++ b/dlls/mshtml/nsevents.c @@ -48,7 +48,6 @@ typedef struct { static nsresult handle_blur(HTMLDocumentNode*,nsIDOMEvent*); static nsresult handle_focus(HTMLDocumentNode*,nsIDOMEvent*); static nsresult handle_keypress(HTMLDocumentNode*,nsIDOMEvent*); -static nsresult handle_dom_content_loaded(HTMLDocumentNode*,nsIDOMEvent*); static nsresult handle_pageshow(HTMLDocumentNode*,nsIDOMEvent*); static nsresult handle_pagehide(HTMLDocumentNode*,nsIDOMEvent*); static nsresult handle_load(HTMLDocumentNode*,nsIDOMEvent*); @@ -68,7 +67,6 @@ static const struct { { EVENTID_BLUR, 0, handle_blur }, { EVENTID_FOCUS, 0, handle_focus }, { EVENTID_KEYPRESS, BUBBLES, handle_keypress }, - { EVENTID_DOMCONTENTLOADED, OVERRIDE, handle_dom_content_loaded }, { EVENTID_PAGESHOW, OVERRIDE, handle_pageshow }, { EVENTID_PAGEHIDE, OVERRIDE, handle_pagehide }, { EVENTID_LOAD, OVERRIDE, handle_load }, @@ -234,26 +232,6 @@ static nsresult handle_keypress(HTMLDocumentNode *doc, nsIDOMEvent *event) return NS_OK; } -static nsresult handle_dom_content_loaded(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) -{ - DOMEvent *event; - HRESULT hres; - - if(doc->window) - doc->window->dom_content_loaded_event_start_time = get_time_stamp(); - - hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), &event); - if(SUCCEEDED(hres)) { - dispatch_event(&doc->node.event_target, event); - IDOMEvent_Release(&event->IDOMEvent_iface); - } - - if(doc->window) - doc->window->dom_content_loaded_event_end_time = get_time_stamp(); - - return NS_OK; -} - static nsresult handle_pageshow(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) { HTMLInnerWindow *window; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/mshtml/tests/dom.c | 76 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index de3ff54966c..0b0d826aaa1 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -12029,6 +12029,81 @@ static void test_document_mode_lock(void) IHTMLDocument2_Release(doc); } +static void test_document_mode_after_initnew(void) +{ + IHTMLDocument2 *doc; + IHTMLDocument6 *doc6; + IEventTarget *event_target; + IPersistStreamInit *init; + IStream *stream; + HRESULT hres; + HGLOBAL mem; + VARIANT var; + SIZE_T len; + MSG msg; + + notif_doc = doc = create_document(); + if(!doc) + return; + doc_complete = FALSE; + + hres = IHTMLDocument2_QueryInterface(doc, &IID_IEventTarget, (void**)&event_target); + ok(hres == E_NOINTERFACE, "QueryInterface(IID_IEventTarget) returned %08lx.\n", hres); + ok(event_target == NULL, "event_target != NULL\n"); + + len = strlen(doc_blank_ie9); + mem = GlobalAlloc(0, len); + memcpy(mem, doc_blank_ie9, len); + hres = CreateStreamOnHGlobal(mem, TRUE, &stream); + ok(hres == S_OK, "Failed to create stream: %08lx.\n", hres); + + hres = IHTMLDocument2_QueryInterface(doc, &IID_IPersistStreamInit, (void**)&init); + ok(hres == S_OK, "QueryInterface(IID_IPersistStreamInit) failed: %08lx.\n", hres); + + IPersistStreamInit_InitNew(init); + IPersistStreamInit_Load(init, stream); + IPersistStreamInit_Release(init); + IStream_Release(stream); + + set_client_site(doc, TRUE); + do_advise((IUnknown*)doc, &IID_IPropertyNotifySink, (IUnknown*)&PropertyNotifySink); + + hres = IHTMLDocument2_QueryInterface(doc, &IID_IHTMLDocument6, (void**)&doc6); + ok(hres == S_OK, "QueryInterface(IID_IHTMLDocument6) failed: %08lx\n", hres); + + V_VT(&var) = VT_EMPTY; + hres = IHTMLDocument6_get_documentMode(doc6, &var); + ok(hres == S_OK, "get_documentMode failed: %08lx\n", hres); + ok(V_VT(&var) == VT_R4, "V_VT(documentMode) = %u\n", V_VT(&var)); + ok(V_R4(&var) == 5, "documentMode = %f, expected 5\n", V_R4(&var)); + VariantClear(&var); + + while(!doc_complete && GetMessageW(&msg, NULL, 0, 0)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + + hres = IHTMLDocument2_QueryInterface(doc, &IID_IEventTarget, (void**)&event_target); + todo_wine + ok(hres == S_OK, "QueryInterface(IID_IEventTarget) returned %08lx.\n", hres); + todo_wine + ok(event_target != NULL, "event_target == NULL\n"); + if (event_target != NULL) + IEventTarget_Release(event_target); + + V_VT(&var) = VT_EMPTY; + hres = IHTMLDocument6_get_documentMode(doc6, &var); + ok(hres == S_OK, "get_documentMode failed: %08lx\n", hres); + ok(V_VT(&var) == VT_R4, "V_VT(documentMode) = %u\n", V_VT(&var)); + todo_wine + ok(V_R4(&var) == 9, "documentMode = %f, expected 9\n", V_R4(&var)); + IHTMLDocument6_Release(doc6); + VariantClear(&var); + + set_client_site(doc, FALSE); + IHTMLDocument2_Release(doc); +} + static DWORD WINAPI create_document_proc(void *param) { IHTMLDocument2 *doc = NULL; @@ -12152,6 +12227,7 @@ START_TEST(dom) test_quirks_mode(); test_document_mode_lock(); + test_document_mode_after_initnew(); test_threads(); /* Run this last since it messes with the process-wide user agent */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
From: Brendan McGrath <bmcgrath(a)codeweavers.com> The event target may be from a different document to the document associated with the event handler. --- dlls/mshtml/nsevents.c | 2 +- dlls/mshtml/tests/dom.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/mshtml/nsevents.c b/dlls/mshtml/nsevents.c index ec4d1f6694a..daac94dd368 100644 --- a/dlls/mshtml/nsevents.c +++ b/dlls/mshtml/nsevents.c @@ -444,7 +444,7 @@ static nsresult handle_htmlevent(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) target = &node->event_target; } - hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), &event); + hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&target->dispex), &event); if(FAILED(hres)) { IEventTarget_Release(&target->IEventTarget_iface); return NS_OK; diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index 0b0d826aaa1..06860f5a553 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -12084,9 +12084,7 @@ static void test_document_mode_after_initnew(void) } hres = IHTMLDocument2_QueryInterface(doc, &IID_IEventTarget, (void**)&event_target); - todo_wine ok(hres == S_OK, "QueryInterface(IID_IEventTarget) returned %08lx.\n", hres); - todo_wine ok(event_target != NULL, "event_target == NULL\n"); if (event_target != NULL) IEventTarget_Release(event_target); @@ -12095,7 +12093,6 @@ static void test_document_mode_after_initnew(void) hres = IHTMLDocument6_get_documentMode(doc6, &var); ok(hres == S_OK, "get_documentMode failed: %08lx\n", hres); ok(V_VT(&var) == VT_R4, "V_VT(documentMode) = %u\n", V_VT(&var)); - todo_wine ok(V_R4(&var) == 9, "documentMode = %f, expected 9\n", V_R4(&var)); IHTMLDocument6_Release(doc6); VariantClear(&var); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/mshtml/nsevents.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/nsevents.c b/dlls/mshtml/nsevents.c index daac94dd368..90d2a40db11 100644 --- a/dlls/mshtml/nsevents.c +++ b/dlls/mshtml/nsevents.c @@ -416,6 +416,7 @@ static nsresult handle_htmlevent(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) nsIDOMEventTarget *event_target; EventTarget *target; nsIDOMNode *nsnode; + HTMLDOMNode *node = NULL; DOMEvent *event; nsresult nsres; HRESULT hres; @@ -436,7 +437,6 @@ static nsresult handle_htmlevent(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) target = &doc->window->event_target; IHTMLWindow2_AddRef(&doc->window->base.IHTMLWindow2_iface); }else { - HTMLDOMNode *node; hres = get_node(nsnode, TRUE, &node); nsIDOMNode_Release(nsnode); if(FAILED(hres)) @@ -454,6 +454,9 @@ static nsresult handle_htmlevent(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) if(event->event_id == EVENTID_FOCUS || event->event_id == EVENTID_BLUR) { DOMEvent *focus_event; + if (node && doc != node->doc) + FIXME("doc %p is not the same as node->doc %p\n", doc, node->doc); + /* FIXME: look to use HTMLDocumentNode_handle_event instead */ hres = create_document_event(doc, event->event_id == EVENTID_FOCUS ? EVENTID_FOCUSIN : EVENTID_FOCUSOUT, &focus_event); if(SUCCEEDED(hres)) { dispatch_event(target, focus_event); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=143408 Your paranoid android. === debian11b (64 bit WoW report) === Report validation errors: shell32:shlfolder crashed (c0000005)
Gabriel Ivăncescu (@insn) commented about dlls/mshtml/htmldoc.c:
.get_gecko_target = HTMLDocumentNode_get_gecko_target, .bind_event = HTMLDocumentNode_bind_event, .get_parent_event_target = HTMLDocumentNode_get_parent_event_target, + .handle_event = HTMLDocumentNode_handle_event, .get_cp_container = HTMLDocumentNode_get_cp_container, - .set_current_event = HTMLDocumentNode_set_current_event + .set_current_event = HTMLDocumentNode_set_current_event, + .pre_handle_event = HTMLDocumentNode_pre_handle_event,
Nit: since we use designated initializers now, you can just put `pre_handle_event` before `handle_event` for consistency (same in the vtbl). You shouldn't have to change the others, regardless. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156#note_62655
Gabriel Ivăncescu (@insn) commented about dlls/mshtml/htmlevent.h:
HRESULT (*handle_event)(DispatchEx*,eventid_t,nsIDOMEvent*,BOOL*); ConnectionPointContainer *(*get_cp_container)(DispatchEx*); IHTMLEventObj *(*set_current_event)(DispatchEx*,IHTMLEventObj*); + HRESULT (*pre_handle_event)(DispatchEx*,eventid_t,nsIDOMEvent*);
Same here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156#note_62656
Gabriel Ivăncescu (@insn) commented about dlls/mshtml/tests/dom.c:
}
hres = IHTMLDocument2_QueryInterface(doc, &IID_IEventTarget, (void**)&event_target); - todo_wine ok(hres == S_OK, "QueryInterface(IID_IEventTarget) returned %08lx.\n", hres); - todo_wine ok(event_target != NULL, "event_target == NULL\n"); if (event_target != NULL)
Nit: You can remove the if condition now and unconditionally release it, since it should never fail now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156#note_62657
participants (4)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Gabriel Ivăncescu
-
Marvin