[PATCH 0/6] MR856: mshtml: url prop for Storage Events and improvements.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Because the typelib should contain the lowercase `url` symbol instead. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/dispex.c | 18 +++++++++++------- dlls/mshtml/htmldoc.c | 2 ++ dlls/mshtml/mshtml_private.h | 1 + dlls/mshtml/tests/dom.js | 5 +++++ 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 5007f9a5d30..90650e5acc4 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -270,16 +270,20 @@ static BOOL is_arg_type_supported(VARTYPE vt) } static void add_func_info(dispex_data_t *data, tid_t tid, const FUNCDESC *desc, ITypeInfo *dti, - dispex_hook_invoke_t hook) + dispex_hook_invoke_t hook, const WCHAR *name_override) { func_info_t *info; BSTR name; HRESULT hres; - hres = ITypeInfo_GetDocumentation(dti, desc->memid, &name, NULL, NULL, NULL); - if(FAILED(hres)) { - WARN("GetDocumentation failed: %08lx\n", hres); - return; + if(name_override) + name = SysAllocString(name_override); + else { + hres = ITypeInfo_GetDocumentation(dti, desc->memid, &name, NULL, NULL, NULL); + if(FAILED(hres)) { + WARN("GetDocumentation failed: %08lx\n", hres); + return; + } } for(info = data->funcs; info < data->funcs+data->func_cnt; info++) { @@ -441,9 +445,9 @@ static HRESULT process_interface(dispex_data_t *data, tid_t tid, ITypeInfo *disp hook = NULL; } - if(!hook || hook->invoke) { + if(!hook || hook->invoke || hook->name) { add_func_info(data, tid, funcdesc, disp_typeinfo ? disp_typeinfo : typeinfo, - hook ? hook->invoke : NULL); + hook ? hook->invoke : NULL, hook ? hook->name : NULL); } ITypeInfo_ReleaseFuncDesc(typeinfo, funcdesc); diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index aa3efe27c68..0c96d73eb76 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -5975,6 +5975,7 @@ static const tid_t HTMLDocumentNode_iface_tids[] = { static void HTMLDocumentNode_init_dispex_info(dispex_data_t *info, compat_mode_t mode) { static const dispex_hook_t document2_hooks[] = { + {DISPID_IHTMLDOCUMENT2_URL, NULL, L"URL"}, {DISPID_IHTMLDOCUMENT2_LOCATION, HTMLDocumentNode_location_hook}, {DISPID_UNKNOWN} }; @@ -6307,6 +6308,7 @@ static const tid_t HTMLDocumentObj_iface_tids[] = { static void HTMLDocumentObj_init_dispex_info(dispex_data_t *info, compat_mode_t mode) { static const dispex_hook_t document2_hooks[] = { + {DISPID_IHTMLDOCUMENT2_URL, NULL, L"URL"}, {DISPID_IHTMLDOCUMENT2_LOCATION, HTMLDocumentObj_location_hook}, {DISPID_UNKNOWN} }; diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 18104afc10a..4ce90d3f17e 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -359,6 +359,7 @@ typedef HRESULT (*dispex_hook_invoke_t)(DispatchEx*,WORD,DISPPARAMS*,VARIANT*, typedef struct { DISPID dispid; dispex_hook_invoke_t invoke; + const WCHAR *name; } dispex_hook_t; struct DispatchEx { diff --git a/dlls/mshtml/tests/dom.js b/dlls/mshtml/tests/dom.js index e82424e5c55..43c52d89757 100644 --- a/dlls/mshtml/tests/dom.js +++ b/dlls/mshtml/tests/dom.js @@ -18,6 +18,11 @@ var tests = []; +sync_test("url", function() { + ok(document.URL === "http://winetest.example.org/index.html?dom.js", "document.URL = " + document.URL); + ok(!("documentURI" in document), "documentURI in document"); +}); + sync_test("input_selection", function() { var input = document.createElement("input"); input.type = "text"; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/856
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Currently, these forward declarations make widl add IHTMLDocument2's `URL` symbol before the other, lowercased, `url` symbols to the type library (from IHTMLMetaElement, IDOMStorageEvent, etc). Since the typelib symbols are case insensitive, `URL` ends up stored instead of `url`, which doesn't match native. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- include/mshtml.idl | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/include/mshtml.idl b/include/mshtml.idl index aa243897405..1abd3e99f77 100644 --- a/include/mshtml.idl +++ b/include/mshtml.idl @@ -395,9 +395,7 @@ interface IHTMLDOMConstructor : IDispatch [propget, id(DISPID_IHTMLDOMCONSTRUCTOR_CONSTRUCTOR), hidden] \ IDispatch *constructor() -interface IHTMLCaret; interface IHTMLComputedStyle; -interface IHTMLDocument2; interface IHTMLDocument7; interface IHTMLElement; interface IHTMLElement2; @@ -414,14 +412,6 @@ interface IHTMLWindow5; interface IHTMLWindow6; interface IHTMLWindow7; interface IDOMEvent; -interface IDisplayPointer; -interface IDisplayServices; -interface ILineInfo; -interface IMarkupPointer; -interface IMarkupContainer; -interface ISegmentList; -interface IHTMLChangeLog; -interface IHTMLChangeSink; interface ISVGSVGElement; interface IHTMLDOMRange; interface IHTMLStyleSheet; @@ -19574,6 +19564,8 @@ interface IHTMLFramesCollection2 : IDispatch HRESULT length([retval, out] LONG *p); } +interface IHTMLDocument2; + /***************************************************************************** * IHTMLScreen interface */ @@ -28841,6 +28833,16 @@ interface IHTMLComputedStyle : IUnknown HRESULT IsEqual([in] IHTMLComputedStyle *pComputedStyle,[out] VARIANT_BOOL *pfEqual); }; +interface IHTMLCaret; +interface IDisplayPointer; +interface IDisplayServices; +interface ILineInfo; +interface IMarkupPointer; +interface IMarkupContainer; +interface ISegmentList; +interface IHTMLChangeLog; +interface IHTMLChangeSink; + /***************************************************************************** * IDisplayPointer interface */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/856
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlevent.c | 18 +++++++++++++++--- dlls/mshtml/htmlevent.h | 2 +- dlls/mshtml/htmlstorage.c | 4 +++- dlls/mshtml/tests/documentmode.js | 8 +++++--- dlls/mshtml/tests/events.c | 5 +++++ 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index 7d13ac5b3fb..f08d4f6568c 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -2616,6 +2616,7 @@ typedef struct { BSTR key; BSTR old_value; BSTR new_value; + BSTR url; } DOMStorageEvent; static inline DOMStorageEvent *impl_from_IDOMStorageEvent(IDOMStorageEvent *iface) @@ -2710,8 +2711,13 @@ static HRESULT WINAPI DOMStorageEvent_get_newValue(IDOMStorageEvent *iface, BSTR static HRESULT WINAPI DOMStorageEvent_get_url(IDOMStorageEvent *iface, BSTR *p) { DOMStorageEvent *This = impl_from_IDOMStorageEvent(iface); - FIXME("(%p)->(%p)\n", This, p); - return E_NOTIMPL; + + TRACE("(%p)->(%p)\n", This, p); + + if(This->url) + return (*p = SysAllocStringLen(This->url, SysStringLen(This->url))) ? S_OK : E_OUTOFMEMORY; + *p = NULL; + return S_OK; } static HRESULT WINAPI DOMStorageEvent_get_storageArea(IDOMStorageEvent *iface, IHTMLStorage **p) @@ -2766,6 +2772,7 @@ static void DOMStorageEvent_destroy(DOMEvent *event) SysFreeString(storage_event->key); SysFreeString(storage_event->old_value); SysFreeString(storage_event->new_value); + SysFreeString(storage_event->url); } static const tid_t DOMEvent_iface_tids[] = { @@ -3120,7 +3127,7 @@ HRESULT create_message_event(HTMLDocumentNode *doc, VARIANT *data, DOMEvent **re } HRESULT create_storage_event(HTMLDocumentNode *doc, BSTR key, BSTR old_value, BSTR new_value, - BOOL commit, DOMEvent **ret) + const WCHAR *url, BOOL commit, DOMEvent **ret) { DOMStorageEvent *storage_event; DOMEvent *event; @@ -3140,6 +3147,11 @@ HRESULT create_storage_event(HTMLDocumentNode *doc, BSTR key, BSTR old_value, BS } } + if(url && !(storage_event->url = SysAllocString(url))) { + IDOMEvent_Release(&event->IDOMEvent_iface); + return E_OUTOFMEMORY; + } + *ret = event; return S_OK; } diff --git a/dlls/mshtml/htmlevent.h b/dlls/mshtml/htmlevent.h index ee0c7831009..fb2b395ca7a 100644 --- a/dlls/mshtml/htmlevent.h +++ b/dlls/mshtml/htmlevent.h @@ -114,7 +114,7 @@ HRESULT create_document_event(HTMLDocumentNode*,eventid_t,DOMEvent**) DECLSPEC_H HRESULT create_document_event_str(HTMLDocumentNode*,const WCHAR*,IDOMEvent**) DECLSPEC_HIDDEN; HRESULT create_event_from_nsevent(nsIDOMEvent*,compat_mode_t,DOMEvent**) DECLSPEC_HIDDEN; HRESULT create_message_event(HTMLDocumentNode*,VARIANT*,DOMEvent**) DECLSPEC_HIDDEN; -HRESULT create_storage_event(HTMLDocumentNode*,BSTR,BSTR,BSTR,BOOL,DOMEvent**) DECLSPEC_HIDDEN; +HRESULT create_storage_event(HTMLDocumentNode*,BSTR,BSTR,BSTR,const WCHAR*,BOOL,DOMEvent**) DECLSPEC_HIDDEN; void init_nsevents(HTMLDocumentNode*) DECLSPEC_HIDDEN; void release_nsevents(HTMLDocumentNode*) DECLSPEC_HIDDEN; diff --git a/dlls/mshtml/htmlstorage.c b/dlls/mshtml/htmlstorage.c index f411c6cb122..bf93e28fdf6 100644 --- a/dlls/mshtml/htmlstorage.c +++ b/dlls/mshtml/htmlstorage.c @@ -235,6 +235,7 @@ struct send_storage_event_ctx { BSTR key; BSTR old_value; BSTR new_value; + const WCHAR *url; }; static HRESULT push_storage_event_task(struct send_storage_event_ctx *ctx, HTMLInnerWindow *window, BOOL commit) @@ -243,7 +244,7 @@ static HRESULT push_storage_event_task(struct send_storage_event_ctx *ctx, HTMLI DOMEvent *event; HRESULT hres; - hres = create_storage_event(window->doc, ctx->key, ctx->old_value, ctx->new_value, commit, &event); + hres = create_storage_event(window->doc, ctx->key, ctx->old_value, ctx->new_value, ctx->url, commit, &event); if(FAILED(hres)) return hres; @@ -327,6 +328,7 @@ static HRESULT send_storage_event(HTMLStorage *storage, BSTR key, BSTR old_value goto done; get_top_window(window->base.outer_window, &top_window); + ctx.url = window->base.outer_window->url; ctx.key = key; ctx.old_value = old_value; ctx.new_value = new_value; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 9532f529332..90223d53f83 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -1322,7 +1322,7 @@ async_test("storage events", function() { } } - function test_event(e, key, oldValue, newValue) { + function test_event(e, idx, key, oldValue, newValue) { if(v < 9) { ok(e === undefined, "event not undefined in legacy mode: " + e); return; @@ -1333,6 +1333,8 @@ async_test("storage events", function() { ok(e.key === key, "key = " + e.key + ", expected " + key); ok(e.oldValue === oldValue, "oldValue = " + e.oldValue + ", expected " + oldValue); ok(e.newValue === newValue, "newValue = " + e.newValue + ", expected " + newValue); + s = (idx ? iframe.contentWindow : window)["location"]["href"]; + ok(e.url === s, "url = " + e.url + ", expected " + s); } function expect(idx, key, oldValue, newValue, quirk) { @@ -1352,10 +1354,10 @@ async_test("storage events", function() { (v < 9 ? document2 : window2)["onstorage"] = function(e) { (local && idx ? document2 : (local || v < 9 ? document : window))[local ? "onstoragecommit" : "onstorage"] = function(e) { - test_event(e, local ? "" : key, local ? "" : oldValue, local ? "" : newValue); + test_event(e, idx, local ? "" : key, local ? "" : oldValue, local ? "" : newValue); next(); } - test_event(e, key, oldValue, newValue); + test_event(e, idx, key, oldValue, newValue); } } } diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index ff64db7130b..1819bc6bdd9 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -2925,6 +2925,11 @@ static void test_storage_event(DISPPARAMS *params, BOOL doc_onstorage) "newValue = %s\n", wine_dbgstr_w(bstr)); SysFreeString(bstr); + hres = IDOMStorageEvent_get_url(event, &bstr); + ok_(__FILE__,line)(hres == S_OK, "get_url failed: %08lx\n", hres); + ok_(__FILE__,line)(!wcscmp(bstr, L"http://winetest.example.org/"), "url = %s\n", wine_dbgstr_w(bstr)); + SysFreeString(bstr); + IDOMStorageEvent_Release(event); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/856
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlevent.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index f08d4f6568c..1ee8ce3db34 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -3057,6 +3057,7 @@ HRESULT create_event_from_nsevent(nsIDOMEvent *nsevent, compat_mode_t compat_mod HRESULT create_document_event_str(HTMLDocumentNode *doc, const WCHAR *type, IDOMEvent **ret_event) { + eventid_t event_id = EVENTID_LAST; nsIDOMEvent *nsevent; DOMEvent *event; nsAString nsstr; @@ -3070,7 +3071,18 @@ HRESULT create_document_event_str(HTMLDocumentNode *doc, const WCHAR *type, IDOM return E_FAIL; } - event = alloc_event(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), EVENTID_LAST); + nsAString_Init(&nsstr, NULL); + nsres = nsIDOMEvent_GetType(nsevent, &nsstr); + if(NS_SUCCEEDED(nsres)) { + const WCHAR *t; + nsAString_GetData(&nsstr, &t); + event_id = str_to_eid(t); + if(event_id == EVENTID_LAST) + FIXME("unknown event type %s\n", debugstr_w(t)); + } + nsAString_Finish(&nsstr); + + event = alloc_event(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), event_id); nsIDOMEvent_Release(nsevent); if(!event) return E_OUTOFMEMORY; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/856
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> We don't use Gecko for these events, so we have to handle them ourselves. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlevent.c | 44 +++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index 1ee8ce3db34..219152430a1 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -240,6 +240,28 @@ static eventid_t attr_to_eid(const WCHAR *str) return EVENTID_LAST; } +static BOOL is_non_gecko_event_type(const WCHAR *type, eventid_t *event_id) +{ + /* Keep this in same order as the eventid_t enum */ + static const eventid_t list[] = { + EVENTID_MESSAGE, + EVENTID_STORAGE, + }; + unsigned i; + + for(i = 0; i < ARRAY_SIZE(list); i++) { + unsigned len = wcslen(event_info[list[i]].name); + if(!wcsnicmp(type, event_info[list[i]].name, len)) { + if(!wcsicmp(type + len, L"Event")) { + *event_id = list[i]; + return TRUE; + } + break; + } + } + return FALSE; +} + const WCHAR *get_event_name(eventid_t eid) { return event_info[eid].name; @@ -3063,7 +3085,7 @@ HRESULT create_document_event_str(HTMLDocumentNode *doc, const WCHAR *type, IDOM nsAString nsstr; nsresult nsres; - nsAString_InitDepend(&nsstr, type); + nsAString_InitDepend(&nsstr, is_non_gecko_event_type(type, &event_id) ? L"Event" : type); nsres = nsIDOMHTMLDocument_CreateEvent(doc->nsdoc, &nsstr, &nsevent); nsAString_Finish(&nsstr); if(NS_FAILED(nsres)) { @@ -3071,16 +3093,18 @@ HRESULT create_document_event_str(HTMLDocumentNode *doc, const WCHAR *type, IDOM return E_FAIL; } - nsAString_Init(&nsstr, NULL); - nsres = nsIDOMEvent_GetType(nsevent, &nsstr); - if(NS_SUCCEEDED(nsres)) { - const WCHAR *t; - nsAString_GetData(&nsstr, &t); - event_id = str_to_eid(t); - if(event_id == EVENTID_LAST) - FIXME("unknown event type %s\n", debugstr_w(t)); + if(event_id == EVENTID_LAST) { + nsAString_Init(&nsstr, NULL); + nsres = nsIDOMEvent_GetType(nsevent, &nsstr); + if(NS_SUCCEEDED(nsres)) { + const WCHAR *t; + nsAString_GetData(&nsstr, &t); + event_id = str_to_eid(t); + if(event_id == EVENTID_LAST) + FIXME("unknown event type %s\n", debugstr_w(t)); + } + nsAString_Finish(&nsstr); } - nsAString_Finish(&nsstr); event = alloc_event(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), event_id); nsIDOMEvent_Release(nsevent); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/856
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index ba631c9a7ef..02d6d374255 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3243,7 +3243,7 @@ static HRESULT WINAPI window_private_postMessage(IWineHTMLWindowPrivate *iface, TRACE("iface %p, msg %s, targetOrigin %s, transfer %s\n", iface, debugstr_variant(&msg), debugstr_w(targetOrigin), debugstr_variant(&transfer)); - if(V_VT(&transfer) != VT_EMPTY) + if(V_VT(&transfer) != VT_EMPTY && V_VT(&transfer) != VT_ERROR) FIXME("transfer not implemented, ignoring\n"); hres = check_target_origin(window, targetOrigin); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/856
Jacek Caban (@jacek) commented about dlls/mshtml/htmlstorage.c:
DOMEvent *event; HRESULT hres;
- hres = create_storage_event(window->doc, ctx->key, ctx->old_value, ctx->new_value, commit, &event); + hres = create_storage_event(window->doc, ctx->key, ctx->old_value, ctx->new_value, ctx->url, commit, &event);
Could you just retrieve it from window here instead of extending ctx? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8548
On Thu Sep 15 17:02:28 2022 +0000, Jacek Caban wrote:
Could you just retrieve it from window here instead of extending ctx? I would have to pass the window instead of the url, is that what you mean?
The url is for the window that triggered the change, while window->doc (first arg) is the window that receives the event, so they don't have the same URL in most cases, unless it's sending on itself. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8567
On Thu Sep 15 18:38:40 2022 +0000, Gabriel Ivăncescu wrote:
I would have to pass the window instead of the url, is that what you mean? The url is for the window that triggered the change, while window->doc (first arg) is the window that receives the event, so they don't have the same URL in most cases, unless it's sending on itself. I misinterpreted it, that's fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8568
Jacek Caban (@jacek) commented about dlls/mshtml/htmlevent.c:
return E_FAIL; }
- event = alloc_event(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), EVENTID_LAST); + nsAString_Init(&nsstr, NULL); + nsres = nsIDOMEvent_GetType(nsevent, &nsstr);
Could we just use the type passed to the function? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8570
Jacek Caban (@jacek) commented about dlls/mshtml/htmlevent.c:
nsAString nsstr; nsresult nsres;
- nsAString_InitDepend(&nsstr, type); + nsAString_InitDepend(&nsstr, is_non_gecko_event_type(type, &event_id) ? L"Event" : type);
What's wrong with passing the actual type for those events? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8571
On Thu Sep 15 18:54:03 2022 +0000, Jacek Caban wrote:
Could we just use the type passed to the function? Sorry, I don't understand what you mean. This function takes a string as input (e.g. "StorageEvent"), and creates an event from it. That part of the code just retrieves the event id from the Gecko event created from the string.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8576
On Thu Sep 15 18:54:04 2022 +0000, Jacek Caban wrote:
What's wrong with passing the actual type for those events? Well for several reasons:
1. Since we aren't using Gecko events for them, there's no point in doing that (just extra overhead). 2. I'm not even sure if Gecko supports creating some of them via CreateEvent (since it's deprecated). 3. The rest of the code (i.e. create_message_event, create_storage_event) create a normal "Event" from Gecko for nsevent, so this keeps it symmetrical. That's because their event type is "Event" since we don't use Gecko's. To be honest I don't see any benefit in creating the actual Gecko nsevent here for them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8578
On Thu Sep 15 19:38:54 2022 +0000, Gabriel Ivăncescu wrote:
Sorry, I don't understand what you mean. This function takes a string as input (e.g. "StorageEvent"), and creates an event from it. That part of the code just retrieves the event id from the Gecko event created from the string. You never initialize event type, so I think it will always be an empty string. The type (as in event.type, not the type passed to createEvent) is not known until the event is actually dispatched.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8598
On Thu Sep 15 19:42:32 2022 +0000, Gabriel Ivăncescu wrote: > Well for several reasons: > 1. Since we aren't using Gecko events for them, there's no point in > doing that (just extra overhead). > 2. I'm not even sure if Gecko supports creating some of them via > CreateEvent (since it's deprecated). > 3. The rest of the code (i.e. create_message_event, > create_storage_event) create a normal "Event" from Gecko for nsevent, so > this keeps it symmetrical. That's because their event type is "Event" > since we don't use Gecko's. > To be honest I don't see any benefit in creating the actual Gecko > nsevent here for them. 1. I'm not sure what you mean, if you suggest not creating the event at all, that's now what the patch is doing. 2. I'd suggest to know it before using it as an argument. 3. Is there anything depending on it? The alternative would be to change other callers to make Gecko match MSHTML. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8599
On Thu Sep 15 20:41:21 2022 +0000, Jacek Caban wrote:
You never initialize event type, so I think it will always be an empty string. The type (as in event.type, not the type passed to createEvent) is not known until the event is actually dispatched. I see. I used the same method from create_event_from_nsevent just above it, but I suppose that one is used for already dispatched nsevents.
Not sure how to handle that here. Should I drop this patch? The event id is mostly needed for non-gecko events to create the proper event in alloc_event. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8692
On Thu Sep 15 20:41:21 2022 +0000, Jacek Caban wrote:
1. I'm not sure what you mean, if you suggest not creating the event at all, that's now what the patch is doing. 2. I'd suggest to know it before using it as an argument. 3. Is there anything depending on it? The alternative would be to change other callers to make Gecko match MSHTML. Ok so it turns out Gecko does support creating Message or Storage events via CreateEvent, but I still don't see how this is useful at all considering we don't use them? What I really dislike is that in other parts we're not using them at all, so it's not symmetric and doesn't match the behavior, even if it doesn't change the outcome, it's still confusing and pointless.
To be fair, I can improve this by using `event_types[event_info[event_id].type]` for non-gecko events to obtain the "Event" string, so that it matches `create_document_event`. That should be fine, right? We do need at least a nsevent though, since DOMEvent uses it for generic event functionality. Hence the "Event" type. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8693
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)