[PATCH 0/6] MR7970: mshtml: Misc prop fixes & improvements.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/dispex.c | 8 ++++++++ dlls/mshtml/htmlwindow.c | 4 +++- dlls/mshtml/tests/documentmode.js | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 21927315443..43896729a9e 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -803,6 +803,14 @@ HRESULT dispex_define_property(DispatchEx *dispex, const WCHAR *name, DWORD flag dynamic_prop_t *prop; HRESULT hres; + if(flags & PROPF_CONFIGURABLE) { + prop = NULL; + hres = get_dynamic_prop(dispex, name, 0, &prop); + assert(FAILED(hres)); + if(prop) + return hres; + } + hres = alloc_dynamic_prop(dispex, name, NULL, &prop); if(FAILED(hres)) return hres; diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index c8c0c7c1ea6..8913c4e511c 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3816,7 +3816,9 @@ static HRESULT HTMLWindow_find_dispid(DispatchEx *dispex, const WCHAR *name, DWO V_VT(&v) = VT_DISPATCH; V_DISPATCH(&v) = (IDispatch *)&constr->IWineJSDispatchHost_iface; - return dispex_define_property(&This->event_target.dispex, name, PROPF_WRITABLE | PROPF_CONFIGURABLE, &v, dispid); + hres = dispex_define_property(&This->event_target.dispex, name, PROPF_WRITABLE | PROPF_CONFIGURABLE, &v, dispid); + if(hres != DISP_E_UNKNOWNNAME) + return hres; } } } diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 557c46920ab..1987112166d 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -4058,7 +4058,6 @@ sync_test("constructors", function() { old = window.HTMLMetaElement; r = (delete window.HTMLMetaElement); ok(r === true, "delete HTMLMetaElement returned " + r); - todo_wine. ok(!window.hasOwnProperty("HTMLMetaElement"), "HTMLMetaElement still a property of window"); window.HTMLMetaElement = old; }); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7970
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/function.c | 1 + dlls/mshtml/tests/documentmode.js | 3 +++ 2 files changed, 4 insertions(+) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 2e811b857cd..8f4b836ff76 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -978,6 +978,7 @@ HRESULT create_source_function(script_ctx_t *ctx, bytecode_t *code, function_cod static const builtin_prop_t HostFunction_props[] = { {L"arguments", NULL, 0, Function_get_arguments}, + {L"caller", NULL, 0, Function_get_caller}, }; static const builtin_info_t HostFunction_info = { diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 58c04800696..513fa775b4f 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -399,6 +399,7 @@ sync_test("builtin_obj", function() { ok(e === "[object Window]", "window.toString with nullDisp context = " + e); ok(f.hasOwnProperty("arguments"), "arguments not a prop of createElement"); + ok(f.hasOwnProperty("caller"), "caller not a prop of createElement"); ok(!f.hasOwnProperty("length"), "length is a prop of createElement"); } @@ -4047,6 +4048,8 @@ sync_test("constructors", function() { } todo_wine. ok(XMLHttpRequest.create.hasOwnProperty("arguments"), "arguments not a prop of XMLHttpRequest.create"); + todo_wine. + ok(XMLHttpRequest.create.hasOwnProperty("caller"), "caller not a prop of XMLHttpRequest.create"); ok(!XMLHttpRequest.create.hasOwnProperty("length"), "length is a prop of XMLHttpRequest.create"); r = Object.getOwnPropertyDescriptor(HTMLMetaElement, "prototype"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7970
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/function.c | 6 ++++++ dlls/mshtml/tests/documentmode.js | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 567e2596969..2e811b857cd 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -976,10 +976,16 @@ HRESULT create_source_function(script_ctx_t *ctx, bytecode_t *code, function_cod return S_OK; } +static const builtin_prop_t HostFunction_props[] = { + {L"arguments", NULL, 0, Function_get_arguments}, +}; + static const builtin_info_t HostFunction_info = { .class = JSCLASS_FUNCTION, .call = Function_value, .destructor = Function_destructor, + .props_cnt = ARRAY_SIZE(HostFunction_props), + .props = HostFunction_props, .gc_traverse = Function_gc_traverse }; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 1987112166d..58c04800696 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -387,12 +387,19 @@ sync_test("builtin_obj", function() { window.toString.call(null); ok(false, "expected exception calling window.toString with null context"); }catch(ex) {} + + ok(!Object.hasOwnProperty.call(f, "arguments"), "arguments is a prop of createElement"); + ok(!Object.hasOwnProperty.call(f, "caller"), "caller is a prop of createElement"); + ok(!Object.hasOwnProperty.call(f, "length"), "length is a prop of createElement"); }else { ok(Object.getPrototypeOf(f) === Function.prototype, "unexpected document.createElement prototype"); e = window.toString.call(null); ok(e === "[object Window]", "window.toString with null context = " + e); e = window.toString.call(external.nullDisp); ok(e === "[object Window]", "window.toString with nullDisp context = " + e); + + ok(f.hasOwnProperty("arguments"), "arguments not a prop of createElement"); + ok(!f.hasOwnProperty("length"), "length is a prop of createElement"); } e = 0; @@ -4038,6 +4045,9 @@ sync_test("constructors", function() { }catch(e) { ok(e.number === 0x0ffff - 0x80000000, "new XMLHttpRequest.create() threw " + e.number); } + todo_wine. + ok(XMLHttpRequest.create.hasOwnProperty("arguments"), "arguments not a prop of XMLHttpRequest.create"); + ok(!XMLHttpRequest.create.hasOwnProperty("length"), "length is a prop of XMLHttpRequest.create"); r = Object.getOwnPropertyDescriptor(HTMLMetaElement, "prototype"); ok(r.value === HTMLMetaElement.prototype, "HTMLMetaElement.prototype value = " + r.value); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7970
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 2 ++ dlls/mshtml/tests/documentmode.js | 25 ++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 03b400a383f..b1af74abc79 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -659,6 +659,8 @@ static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) TRACE("no prop_put\n"); return S_OK; } + if(!(prop->flags & PROPF_WRITABLE)) + return S_OK; hres = This->builtin_info->prop_put(This, prop->u.id, val); if(hres != S_FALSE) return hres; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 634f4ad3420..557c46920ab 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -4012,7 +4012,7 @@ sync_test("prototype props", function() { }); sync_test("constructors", function() { - var v = document.documentMode, i, r; + var v = document.documentMode, i, r, old; if(v < 9) return; @@ -4038,4 +4038,27 @@ sync_test("constructors", function() { }catch(e) { ok(e.number === 0x0ffff - 0x80000000, "new XMLHttpRequest.create() threw " + e.number); } + + r = Object.getOwnPropertyDescriptor(HTMLMetaElement, "prototype"); + ok(r.value === HTMLMetaElement.prototype, "HTMLMetaElement.prototype value = " + r.value); + ok(!("get" in r), "HTMLMetaElement.prototype has getter"); + ok(!("set" in r), "HTMLMetaElement.prototype has setter"); + ok(r.writable === false, "HTMLMetaElement.prototype writable = " + r.writable); + ok(r.enumerable === false, "HTMLMetaElement.prototype enumerable = " + r.enumerable); + ok(r.configurable === false, "HTMLMetaElement.prototype configurable = " + r.configurable); + + old = HTMLMetaElement.prototype; + HTMLMetaElement.prototype = Object.prototype; + ok(HTMLMetaElement.prototype === old, "HTMLMetaElement.prototype = " + HTMLMetaElement.prototype); + + r = (delete HTMLMetaElement.prototype); + ok(r === false, "delete HTMLMetaElement.prototype returned " + r); + ok(HTMLMetaElement.hasOwnProperty("prototype"), "prototype not a prop anymore of HTMLMetaElement"); + + old = window.HTMLMetaElement; + r = (delete window.HTMLMetaElement); + ok(r === true, "delete HTMLMetaElement returned " + r); + todo_wine. + ok(!window.hasOwnProperty("HTMLMetaElement"), "HTMLMetaElement still a property of window"); + window.HTMLMetaElement = old; }); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7970
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/function.c | 2 ++ dlls/mshtml/tests/documentmode.js | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 8f4b836ff76..54b8f5abe2f 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -1124,6 +1124,8 @@ static const builtin_info_t HostConstructor_info = { .release = HostConstructor_release, .call = Function_value, .destructor = Function_destructor, + .props_cnt = ARRAY_SIZE(HostFunction_props), + .props = HostFunction_props, .gc_traverse = Function_gc_traverse, .lookup_prop = HostConstructor_lookup_prop, }; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 513fa775b4f..0ac22d2236e 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -4032,6 +4032,11 @@ sync_test("constructors", function() { ok(window.hasOwnProperty(r), r + " not prop of window"); ok(!(r in Window.prototype), r + " is a prop of window's prototype"); ok(window[r].toString() === "\nfunction " + r + "() {\n [native code]\n}\n", r + ".toString() = " + window[r].toString()); + + ok(window[r].hasOwnProperty("arguments"), "arguments not a prop of " + r); + ok(window[r].hasOwnProperty("caller"), "caller not a prop of " + r); + ok(window[r].hasOwnProperty("prototype"), "prototype not a prop of " + r); + ok(!window[r].hasOwnProperty("length"), "length is a prop of " + r); } ok(window.Image.prototype === window.HTMLImageElement.prototype, "Image.prototype != HTMLImageElement.prototype"); ok(window.Option.prototype === window.HTMLOptionElement.prototype, "Option.prototype != HTMLOptionElement.prototype"); @@ -4046,9 +4051,7 @@ sync_test("constructors", function() { }catch(e) { ok(e.number === 0x0ffff - 0x80000000, "new XMLHttpRequest.create() threw " + e.number); } - todo_wine. ok(XMLHttpRequest.create.hasOwnProperty("arguments"), "arguments not a prop of XMLHttpRequest.create"); - todo_wine. ok(XMLHttpRequest.create.hasOwnProperty("caller"), "caller not a prop of XMLHttpRequest.create"); ok(!XMLHttpRequest.create.hasOwnProperty("length"), "length is a prop of XMLHttpRequest.create"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7970
Jacek Caban (@jacek) commented about dlls/mshtml/tests/documentmode.js:
window.toString.call(null); ok(false, "expected exception calling window.toString with null context"); }catch(ex) {} + + ok(!Object.hasOwnProperty.call(f, "arguments"), "arguments is a prop of createElement"); + ok(!Object.hasOwnProperty.call(f, "caller"), "caller is a prop of createElement"); + ok(!Object.hasOwnProperty.call(f, "length"), "length is a prop of createElement");
We already test "arguments" and "length" a few lines above using "in" expression. For the negative tests like this, "in" expressions are stronger tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7970#note_102575
Jacek Caban (@jacek) commented about dlls/mshtml/tests/documentmode.js:
old = window.HTMLMetaElement; r = (delete window.HTMLMetaElement); ok(r === true, "delete HTMLMetaElement returned " + r); - todo_wine.
It would be nice to test the other use of `dispex_define_property` too, something like `delete HTMLMetaElement.prototype.constructor`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7970#note_102574
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/tests/dom.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dlls/mshtml/tests/dom.js b/dlls/mshtml/tests/dom.js index 256cab60d7c..300e22cd684 100644 --- a/dlls/mshtml/tests/dom.js +++ b/dlls/mshtml/tests/dom.js @@ -147,9 +147,18 @@ async_test("iframe_location", function() { iframe.onload = function() { ok(iframe.contentWindow.location.pathname === "/emptyfile", "path = " + iframe.contentWindow.location.pathname); + ok(iframe.contentWindow.Image !== undefined, "Image is undefined"); + ok(iframe.contentWindow.VBArray !== undefined, "VBArray is undefined"); + iframe.contentWindow.Image = undefined; + iframe.contentWindow.VBArray = undefined; + iframe.contentWindow.foobar = 1234; iframe.onload = function () { ok(iframe.contentWindow.location.pathname === "/empty/file", "path = " + iframe.contentWindow.location.pathname); + ok(iframe.contentWindow.Image !== undefined, "Image is undefined (2)"); + ok(iframe.contentWindow.VBArray !== undefined, "VBArray is undefined (2)"); + ok(!Object.prototype.hasOwnProperty.call(iframe.contentWindow, "foobar"), + "contentWindow has foobar"); next_test(); } iframe.src = "empty/file"; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7970
Jacek Caban (@jacek) commented about dlls/mshtml/tests/documentmode.js:
window.toString.call(null); ok(false, "expected exception calling window.toString with null context"); }catch(ex) {} + + ok(!Object.hasOwnProperty.call(f, "arguments"), "arguments is a prop of createElement"); + ok(!Object.hasOwnProperty.call(f, "caller"), "caller is a prop of createElement"); + ok(!Object.hasOwnProperty.call(f, "length"), "length is a prop of createElement"); }else { ok(Object.getPrototypeOf(f) === Function.prototype, "unexpected document.createElement prototype"); e = window.toString.call(null); ok(e === "[object Window]", "window.toString with null context = " + e); e = window.toString.call(external.nullDisp); ok(e === "[object Window]", "window.toString with nullDisp context = " + e); + + ok(f.hasOwnProperty("arguments"), "arguments not a prop of createElement"); + ok(!f.hasOwnProperty("length"), "length is a prop of createElement");
For testing exposed properties, using `test_own_props` would be better. Additionally, it would be good to test the actual value of the property (probably `null` in this case). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7970#note_102576
On Mon May 5 16:42:52 2025 +0000, Jacek Caban wrote:
For testing exposed properties, using `test_own_props` would be better. Additionally, it would be good to test the actual value of the property (probably `null` in this case). I had to add a fix for builtin props filling, else we wouldn't be able to test the patch. There's more prop filling fixes I planned to send on next MR (but I moved this one now since it was needed).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7970#note_102673
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)