[PATCH v2 0/5] MR856: mshtml: url prop for Storage Events and improvements.
-- v2: mshtml: Silence a FIXME when parameter is missing. mshtml: Create non-gecko events properly from type string. mshtml: Implement url prop for StorageEvent. include/mshtml: Move some forward interface declarations to match Windows SDK. mshtml: Override document.URL's name when adding it from the mshtml typelib. https://gitlab.winehq.org/wine/wine/-/merge_requests/856
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> 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 | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index f08d4f6568c..171412acc90 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -240,6 +240,26 @@ static eventid_t attr_to_eid(const WCHAR *str) return EVENTID_LAST; } +static eventid_t get_non_gecko_event_type_id(const WCHAR *type) +{ + /* 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")) + return list[i]; + break; + } + } + return EVENTID_LAST; +} + const WCHAR *get_event_name(eventid_t eid) { return event_info[eid].name; @@ -3057,12 +3077,13 @@ 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 = get_non_gecko_event_type_id(type); nsIDOMEvent *nsevent; DOMEvent *event; nsAString nsstr; nsresult nsres; - nsAString_InitDepend(&nsstr, type); + nsAString_InitDepend(&nsstr, event_id == EVENTID_LAST ? type : event_types[event_info[event_id].type]); nsres = nsIDOMHTMLDocument_CreateEvent(doc->nsdoc, &nsstr, &nsevent); nsAString_Finish(&nsstr); if(NS_FAILED(nsres)) { @@ -3070,7 +3091,7 @@ 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); + 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> 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
On Fri Sep 16 15:15:53 2022 +0000, Gabriel Ivăncescu wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/856/diffs?diff_id=11221&start_sha=f64595c4df6460ae851e7db6f598c7c51c994dbc#b5fa643f0dd306b4a3dc41c7d2cf01b86dff3109_3088_3086) The worst part is that you're adding a special case where it's not needed at all. The fact that the special case makes Gecko object diverge from MSHTML counterpart also feels like step backwards. Improving event_types[] sounds better to me.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8767
On Fri Sep 16 13:49:04 2022 +0000, Gabriel Ivăncescu wrote:
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. It seems that alloc_event should just take event_type_t as an argument.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8768
On Sun Sep 18 12:12:46 2022 +0000, Jacek Caban wrote:
The worst part is that you're adding a special case where it's not needed at all. The fact that the special case makes Gecko object diverge from MSHTML counterpart also feels like step backwards. Improving event_types[] sounds better to me. The special case is simply mirroring the behavior of create_document_event; in my opinion, having these 2 functions differ in behavior is confusing and is actually a bug, even if irrelevant. At least it was confusing for me.
While it's true that event_types can be fixed for both MessageEvent and StorageEvent in this case, I didn't find a reason to do it, because we don't use Gecko's at all. In fact we can't even query their specific interfaces because they're not exposed as XPCOM (at least for MessageEvent, didn't bother to look at StorageEvent) so it's pointless. Are you sure you want to use them, then? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/856#note_8791
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)