[PATCH v4 0/4] MR5156: mshtml: Always use the event target dispex
This MR 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. -- v4: mshtml: Don't handle special case when doc != node->doc. mshtml: Always use the event target dispex. mshtml/tests: Add test for document mode after InitNew and Load. mshtml: Use generic event dispatcher for DOMContentLoaded. 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 | 38 +++++++++++++++++++++++++++++++++++++- dlls/mshtml/htmlevent.c | 14 +++++++++++++- dlls/mshtml/htmlevent.h | 1 + dlls/mshtml/nsevents.c | 22 ---------------------- 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 4267b13ffab..c46ab9a386f 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -6104,6 +6104,40 @@ static HRESULT HTMLDocumentNode_location_hook(DispatchEx *dispex, WORD flags, DI 0, flags, dp, res, ei, caller); } +static HRESULT HTMLDocumentNode_pre_handle_event(DispatchEx* dispex, DOMEvent *event) +{ + HTMLDocumentNode *doc = impl_from_DispatchEx(dispex); + switch(event->event_id) { + case EVENTID_DOMCONTENTLOADED: { + if(event->trusted && 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: { + cpp_bool trusted; + nsIDOMEvent_GetIsTrusted(event, &trusted); + if(trusted && 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 +6152,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, + .pre_handle_event = HTMLDocumentNode_pre_handle_event, + .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, }; static const NodeImplVtbl HTMLDocumentFragmentImplVtbl = { diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index 1ae3a0de384..e1a36dc5d90 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); + 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..5cb9de9046d 100644 --- a/dlls/mshtml/htmlevent.h +++ b/dlls/mshtml/htmlevent.h @@ -131,6 +131,7 @@ typedef struct { nsISupports *(*get_gecko_target)(DispatchEx*); void (*bind_event)(DispatchEx*,eventid_t); EventTarget *(*get_parent_event_target)(DispatchEx*); + HRESULT (*pre_handle_event)(DispatchEx*,DOMEvent*); HRESULT (*handle_event)(DispatchEx*,eventid_t,nsIDOMEvent*,BOOL*); ConnectionPointContainer *(*get_cp_container)(DispatchEx*); IHTMLEventObj *(*set_current_event)(DispatchEx*,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 | 6 +----- 2 files changed, 2 insertions(+), 6 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..8383f167cdd 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -12084,18 +12084,14 @@ 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); + 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/mshtml/nsevents.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/nsevents.c b/dlls/mshtml/nsevents.c index daac94dd368..c4843fa45b6 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)) @@ -451,7 +451,7 @@ static nsresult handle_htmlevent(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) } /* If we fine need for more special cases here, we may consider handling it in a more generic way. */ - if(event->event_id == EVENTID_FOCUS || event->event_id == EVENTID_BLUR) { + if((!node || doc == node->doc) && (event->event_id == EVENTID_FOCUS || event->event_id == EVENTID_BLUR)) { DOMEvent *focus_event; hres = create_document_event(doc, event->event_id == EVENTID_FOCUS ? EVENTID_FOCUSIN : EVENTID_FOCUSOUT, &focus_event); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5156
On Wed Feb 28 11:45:24 2024 +0000, Jacek Caban wrote:
Skipping the if makes it most likely correct, please drop the FIXME. OK, thanks. I've removed the FIXME comment and FIXME logging
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156#note_62998
On Wed Feb 28 11:44:42 2024 +0000, Jacek Caban wrote:
That doesn't prove that we should update times on custom events. To prove that, you'd need to use `dispatchEvent()` to dispatch a custom event and see if `domContentLoadedEventStart` value changes (and I'd expect not). Did you use `nsIDOMEvent` for that? I don't remember details, but there were reasons it's not used for our `IDOMEvent::get_isTrusted()`, so please try `DOMEvent.trusted` if you didn't. I wanted to investigate this failing test further, but I haven't been able to recreate it this morning. So I've just pushed the change as it was.
You can see I'm using `DOMEvent.trusted` in the `pre_handle_event`, but `nsIDOMEvent::GetIsTrusted` in `handle_event`. I assume we want to make the same check in `handle_event`? But I couldn't see a way to get the `DOMEvent` (it doesn't seem like anything we pass to `handle_event` has a reference to the `DOMEvent`?), so I resorted to using `nsIDOMEvent::GetIsTrusted`. But it doesn't feel right to do one thing in `pre_handle_event` and another in `handle_event`, so I'm happy to change `handle_event` (and all the existing implementations) to accept a `DOMEvent` instead of `nsIDOMEvent` if you think that's the right thing to do? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156#note_63007
But it doesn't feel right to do one thing in `pre_handle_event` and another in `handle_event`, so I'm happy to change `handle_event` (and all the existing implementations) to accept a `DOMEvent` instead of `nsIDOMEvent` if you think that's the right thing to do?
Yes, I think it's the right thing to do (in a separated commit). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5156#note_63008
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Jacek Caban (＠jacek)