[PATCH 1/7] jscript: Fix some refcount leaks.
In particular, most leak when jsdisp->ctx is not the current ctx. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index c7e4ba9..4c57a69 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -2028,6 +2028,7 @@ HRESULT disp_call(script_ctx_t *ctx, IDispatch *disp, DISPID id, WORD flags, uns if(jsdisp && jsdisp->ctx == ctx) { if(flags & DISPATCH_PROPERTYPUT) { FIXME("disp_call(propput) on builtin object\n"); + jsdisp_release(jsdisp); return E_FAIL; } @@ -2037,6 +2038,8 @@ HRESULT disp_call(script_ctx_t *ctx, IDispatch *disp, DISPID id, WORD flags, uns jsdisp_release(jsdisp); return hres; } + if(jsdisp) + jsdisp_release(jsdisp); flags &= ~DISPATCH_JSCRIPT_INTERNAL_MASK; if(ret && argc) @@ -2106,6 +2109,8 @@ HRESULT disp_call_value(script_ctx_t *ctx, IDispatch *disp, IDispatch *jsthis, W jsdisp_release(jsdisp); return hres; } + if(jsdisp) + jsdisp_release(jsdisp); flags &= ~DISPATCH_JSCRIPT_INTERNAL_MASK; if(r && argc && flags == DISPATCH_METHOD) @@ -2205,6 +2210,8 @@ HRESULT disp_propput(script_ctx_t *ctx, IDispatch *disp, DISPID id, jsval_t val) VARIANT var; DISPPARAMS dp = {&var, &dispid, 1, 1}; + if(jsdisp) + jsdisp_release(jsdisp); hres = jsval_to_variant(val, &var); if(FAILED(hres)) return hres; @@ -2230,6 +2237,8 @@ HRESULT disp_propput_name(script_ctx_t *ctx, IDispatch *disp, const WCHAR *name, BSTR str; DISPID id; + if(jsdisp) + jsdisp_release(jsdisp); if(!(str = SysAllocString(name))) return E_OUTOFMEMORY; @@ -2248,7 +2257,9 @@ HRESULT disp_propput_name(script_ctx_t *ctx, IDispatch *disp, const WCHAR *name, return disp_propput(ctx, disp, id, val); } - return jsdisp_propput_name(jsdisp, name, val); + hres = jsdisp_propput_name(jsdisp, name, val); + jsdisp_release(jsdisp); + return hres; } HRESULT jsdisp_propget_name(jsdisp_t *obj, const WCHAR *name, jsval_t *val) @@ -2312,6 +2323,8 @@ HRESULT disp_propget(script_ctx_t *ctx, IDispatch *disp, DISPID id, jsval_t *val jsdisp_release(jsdisp); return hres; } + if(jsdisp) + jsdisp_release(jsdisp); V_VT(&var) = VT_EMPTY; hres = disp_invoke(ctx, disp, id, INVOKE_PROPERTYGET, &dp, &var); -- 2.31.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/object.c | 13 +++++++++++-- dlls/jscript/tests/lang.js | 13 +++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c index 169b47c..8da0ff4 100644 --- a/dlls/jscript/object.c +++ b/dlls/jscript/object.c @@ -196,8 +196,17 @@ static HRESULT Object_propertyIsEnumerable(script_ctx_t *ctx, vdisp_t *jsthis, W static HRESULT Object_isPrototypeOf(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *jsdisp; + BOOL ret = FALSE; + + if(!r) + return S_OK; + + if(argc && is_jsdisp(jsthis) && is_object_instance(argv[0]) && (jsdisp = to_jsdisp(get_object(argv[0])))) + ret = (jsdisp->prototype == jsthis->u.jsdisp); + + *r = jsval_bool(ret); + return S_OK; } static HRESULT Object_get_value(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t *r) diff --git a/dlls/jscript/tests/lang.js b/dlls/jscript/tests/lang.js index ad1217d..bcfe88b 100644 --- a/dlls/jscript/tests/lang.js +++ b/dlls/jscript/tests/lang.js @@ -1679,14 +1679,27 @@ tmp = new instanceOfTest(); ok((tmp instanceof instanceOfTest) === true, "tmp is not instance of instanceOfTest"); ok((tmp instanceof Object) === true, "tmp is not instance of Object"); ok((tmp instanceof String) === false, "tmp is instance of String"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest, tmp) === false, "instanceOfTest is prototype of tmp"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest.prototype, tmp) === true, "instanceOfTest.prototype is not prototype of tmp"); instanceOfTest.prototype = new Object(); ok((tmp instanceof instanceOfTest) === false, "tmp is instance of instanceOfTest"); ok((tmp instanceof Object) === true, "tmp is not instance of Object"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest.prototype, tmp) === false, "instanceOfTest.prototype is prototype of tmp"); ok((1 instanceof Object) === false, "1 is instance of Object"); ok((false instanceof Boolean) === false, "false is instance of Boolean"); ok(("" instanceof Object) === false, "'' is instance of Object"); +ok(Object.prototype.isPrototypeOf.call(Number.prototype, 1) === false, "Number.prototype is prototype of 1"); +ok(Object.prototype.isPrototypeOf.call(String.prototype, "") === false, "String.prototype is prototype of ''"); + +ok(Object.prototype.isPrototypeOf.call(tmp) === false, "tmp is prototype of no argument"); +ok(Object.prototype.isPrototypeOf.call(tmp, null) === false, "tmp is prototype of null"); +ok(Object.prototype.isPrototypeOf.call(tmp, undefined) === false, "tmp is prototype of undefined"); +ok(Object.prototype.isPrototypeOf.call(test, Object) === false, "test is prototype of Object"); +ok(Object.prototype.isPrototypeOf.call(Object.prototype, test) === false, "Object.prototype is prototype of test"); +ok(Object.prototype.isPrototypeOf.call(testObj, Object) === false, "testObj is prototype of Object"); +ok(Object.prototype.isPrototypeOf.call(Object.prototype, testObj) === false, "Object.prototype is prototype of testObj"); (function () { ok((arguments instanceof Object) === true, "argument is not instance of Object"); -- 2.31.1
On 10/25/21 3:30 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/object.c | 13 +++++++++++-- dlls/jscript/tests/lang.js | 13 +++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c index 169b47c..8da0ff4 100644 --- a/dlls/jscript/object.c +++ b/dlls/jscript/object.c @@ -196,8 +196,17 @@ static HRESULT Object_propertyIsEnumerable(script_ctx_t *ctx, vdisp_t *jsthis, W static HRESULT Object_isPrototypeOf(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *jsdisp; + BOOL ret = FALSE; + + if(!r) + return S_OK; + + if(argc && is_jsdisp(jsthis) && is_object_instance(argv[0]) && (jsdisp = to_jsdisp(get_object(argv[0])))) + ret = (jsdisp->prototype == jsthis->u.jsdisp);
That's not what spec says we should do.
diff --git a/dlls/jscript/tests/lang.js b/dlls/jscript/tests/lang.js index ad1217d..bcfe88b 100644 --- a/dlls/jscript/tests/lang.js +++ b/dlls/jscript/tests/lang.js @@ -1679,14 +1679,27 @@ tmp = new instanceOfTest(); ok((tmp instanceof instanceOfTest) === true, "tmp is not instance of instanceOfTest"); ok((tmp instanceof Object) === true, "tmp is not instance of Object"); ok((tmp instanceof String) === false, "tmp is instance of String"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest, tmp) === false, "instanceOfTest is prototype of tmp"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest.prototype, tmp) === true, "instanceOfTest.prototype is not prototype of tmp");
Why do you use call() instead of doing regular calls? Thanks, Jacek
On 01/11/2021 15:46, Jacek Caban wrote:
On 10/25/21 3:30 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/object.c | 13 +++++++++++-- dlls/jscript/tests/lang.js | 13 +++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c index 169b47c..8da0ff4 100644 --- a/dlls/jscript/object.c +++ b/dlls/jscript/object.c @@ -196,8 +196,17 @@ static HRESULT Object_propertyIsEnumerable(script_ctx_t *ctx, vdisp_t *jsthis, W static HRESULT Object_isPrototypeOf(script_ctx_t *ctx, vdisp_t *jsthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *jsdisp; + BOOL ret = FALSE; + + if(!r) + return S_OK; + + if(argc && is_jsdisp(jsthis) && is_object_instance(argv[0]) && (jsdisp = to_jsdisp(get_object(argv[0])))) + ret = (jsdisp->prototype == jsthis->u.jsdisp);
That's not what spec says we should do.
Oh yeah, oops, I see it's supposed to check the whole chain.
diff --git a/dlls/jscript/tests/lang.js b/dlls/jscript/tests/lang.js index ad1217d..bcfe88b 100644 --- a/dlls/jscript/tests/lang.js +++ b/dlls/jscript/tests/lang.js @@ -1679,14 +1679,27 @@ tmp = new instanceOfTest(); ok((tmp instanceof instanceOfTest) === true, "tmp is not instance of instanceOfTest"); ok((tmp instanceof Object) === true, "tmp is not instance of Object"); ok((tmp instanceof String) === false, "tmp is instance of String"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest, tmp) === false, "instanceOfTest is prototype of tmp"); +ok(Object.prototype.isPrototypeOf.call(instanceOfTest.prototype, tmp) === true, "instanceOfTest.prototype is not prototype of tmp");
Why do you use call() instead of doing regular calls?
Not a specific reason, I just use it in general to make sure it's the proper one called, so I end up copy pasting it.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/dispex.c | 23 ++++++++++++- dlls/jscript/jscript.h | 2 ++ dlls/jscript/object.c | 28 ++++++++++++++++ dlls/mshtml/tests/documentmode.js | 55 +++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 4c57a69..e64a520 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -461,7 +461,8 @@ static HRESULT prop_put(jsdisp_t *This, dispex_prop_t *prop, jsval_t val) prop_iter = prototype_iter->props + prop_iter->u.ref; } while(prop_iter->type == PROP_PROTREF); - if(prop_iter->type == PROP_ACCESSOR) + if(prop_iter->type == PROP_ACCESSOR || + (prop_iter->type == PROP_BUILTIN && prop_iter->u.p->setter)) prop = prop_iter; } @@ -2654,6 +2655,26 @@ HRESULT jsdisp_define_data_property(jsdisp_t *obj, const WCHAR *name, unsigned f return jsdisp_define_property(obj, name, &prop_desc); } +HRESULT jsdisp_change_prototype(jsdisp_t *obj, jsdisp_t *proto) +{ + DWORD i; + + if(obj->prototype == proto) + return S_OK; + + if(obj->prototype) { + for(i = 0; i < obj->prop_cnt; i++) + if(obj->props[i].type == PROP_PROTREF) + obj->props[i].type = PROP_DELETED; + jsdisp_release(obj->prototype); + } + + obj->prototype = proto; + if(proto) + jsdisp_addref(proto); + return S_OK; +} + void jsdisp_freeze(jsdisp_t *obj, BOOL seal) { unsigned int i; diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 90a5e15..69897cd 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -104,6 +104,7 @@ HRESULT get_dispatch_typeinfo(ITypeInfo**) DECLSPEC_HIDDEN; #define PROPF_VERSION_SHIFT 16 #define PROPF_HTML (SCRIPTLANGUAGEVERSION_HTML << PROPF_VERSION_SHIFT) #define PROPF_ES5 ((SCRIPTLANGUAGEVERSION_HTML|SCRIPTLANGUAGEVERSION_ES5) << PROPF_VERSION_SHIFT) +#define PROPF_ES6 ((SCRIPTLANGUAGEVERSION_HTML|SCRIPTLANGUAGEVERSION_ES6) << PROPF_VERSION_SHIFT) /* * This is our internal dispatch flag informing calee that it's called directly from interpreter. @@ -326,6 +327,7 @@ HRESULT jsdisp_define_property(jsdisp_t*,const WCHAR*,property_desc_t*) DECLSPEC HRESULT jsdisp_define_data_property(jsdisp_t*,const WCHAR*,unsigned,jsval_t) DECLSPEC_HIDDEN; HRESULT jsdisp_next_prop(jsdisp_t*,DISPID,enum jsdisp_enum_type,DISPID*) DECLSPEC_HIDDEN; HRESULT jsdisp_get_prop_name(jsdisp_t*,DISPID,jsstr_t**); +HRESULT jsdisp_change_prototype(jsdisp_t*,jsdisp_t*) DECLSPEC_HIDDEN; void jsdisp_freeze(jsdisp_t*,BOOL) DECLSPEC_HIDDEN; BOOL jsdisp_is_frozen(jsdisp_t*,BOOL) DECLSPEC_HIDDEN; diff --git a/dlls/jscript/object.c b/dlls/jscript/object.c index 8da0ff4..d638cb6 100644 --- a/dlls/jscript/object.c +++ b/dlls/jscript/object.c @@ -209,6 +209,33 @@ static HRESULT Object_isPrototypeOf(script_ctx_t *ctx, vdisp_t *jsthis, WORD fla return S_OK; } +static HRESULT Object_get_proto_(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t *r) +{ + TRACE("%p\n", jsthis); + + if(r) + *r = jsthis->prototype + ? jsval_obj(jsdisp_addref(jsthis->prototype)) + : jsval_null(); + return S_OK; +} + +static HRESULT Object_set_proto_(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t value) +{ + jsdisp_t *proto; + + TRACE("%p\n", jsthis); + + if(is_undefined(value) || is_null(value)) + proto = NULL; + else if(!is_object_instance(value) || !(proto = to_jsdisp(get_object(value)))) { + FIXME("not an object\n"); + return E_FAIL; + } + + return jsdisp_change_prototype(jsthis, proto); +} + static HRESULT Object_get_value(script_ctx_t *ctx, jsdisp_t *jsthis, jsval_t *r) { jsstr_t *ret; @@ -229,6 +256,7 @@ static void Object_destructor(jsdisp_t *dispex) } static const builtin_prop_t Object_props[] = { + {L"__proto__", NULL, PROPF_ES6, Object_get_proto_, Object_set_proto_}, {L"hasOwnProperty", Object_hasOwnProperty, PROPF_METHOD|1}, {L"isPrototypeOf", Object_isPrototypeOf, PROPF_METHOD|1}, {L"propertyIsEnumerable", Object_propertyIsEnumerable, PROPF_METHOD|1}, diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 3307f2c..02a5f5f 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -1025,3 +1025,58 @@ sync_test("elem_attr", function() { r = elem.getAttribute("className"); ok(r === "cls3", "className attr = " + r); }); + +sync_test("__proto__", function() { + var v = document.documentMode; + var r, x = 42; + + if(v < 11) { + ok(x.__proto__ === undefined, "x.__proto__ = " + x.__proto__); + ok(!("__proto__" in Object), "Object.__proto__ = " + Object.__proto__); + return; + } + + ok(x.__proto__ === Number.prototype, "x.__proto__ = " + x.__proto__); + ok(Object.__proto__ === Function.prototype, "Object.__proto__ = " + Object.__proto__); + ok(Object.prototype.__proto__ === null, "Object.prototype.__proto__ = " + Object.prototype.__proto__); + ok(Object.prototype.hasOwnProperty("__proto__"), "__proto__ is not a property of Object.prototype"); + ok(!Object.prototype.hasOwnProperty.call(x, "__proto__"), "__proto__ is a property of x"); + + x.__proto__ = Object.prototype; + ok(x.__proto__ === Number.prototype, "x.__proto__ set to Object.prototype = " + x.__proto__); + ok(!Object.prototype.hasOwnProperty.call(x, "__proto__"), "__proto__ is a property of x after set to Object.prototype"); + x = {}; + x.__proto__ = null; + r = Object.getPrototypeOf(x); + ok(x.__proto__ === undefined, "x.__proto__ after set to null = " + x.__proto__); + ok(r === null, "getPrototypeOf(x) after set to null = " + r); + + function check(expect, msg) { + var r = Object.getPrototypeOf(x); + ok(x.__proto__ === expect, "x.__proto__ " + msg + " = " + x.__proto__); + ok(r === expect, "getPrototypeOf(x) " + msg + " = " + r); + ok(!Object.prototype.hasOwnProperty.call(x, "__proto__"), "__proto__ is a property of x " + msg); + } + + x = {}; + check(Object.prototype, "after x set to {}"); + x.__proto__ = Number.prototype; + check(Number.prototype, "after set to Number.prototype"); + x.__proto__ = Object.prototype; + check(Object.prototype, "after re-set to Object.prototype"); + + function ctor() { } + var obj = new ctor(); + x.__proto__ = obj; + check(obj, "after set to obj"); + x.__proto__ = ctor.prototype; + check(obj.__proto__, "after set to ctor.prototype"); + ok(obj.__proto__ === ctor.prototype, "obj.__proto__ !== ctor.prototype"); + + r = (delete x.__proto__); + todo_wine. + ok(r, "delete x.__proto__ returned " + r); + ok(Object.prototype.hasOwnProperty("__proto__"), "__proto__ is not a property of Object.prototype after delete"); + r = Object.getPrototypeOf(x); + ok(r === ctor.prototype, "x.__proto__ after delete = " + r); +}); -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=100703 Your paranoid android. === w8adm (32 bit report) === mshtml: events.c:1089: Test failed: unexpected call img_onerror events: Timeout === w8adm (32 bit report) === mshtml: htmldoc.c:3084: Test failed: Incorrect error code: -2146697211 htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:5861: Test failed: expected OnChanged_1012 htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV htmldoc.c:5864: Test failed: expected Exec_SETTITLE htmldoc.c:5905: Test failed: expected FireNavigateComplete2 === w10pro64 (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS === w7u_adm (32 bit report) === mshtml: script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1635173154344 expected 1635173154407"
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlstylesheet.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/mshtml/htmlstylesheet.c b/dlls/mshtml/htmlstylesheet.c index cbc2adf..52eaa21 100644 --- a/dlls/mshtml/htmlstylesheet.c +++ b/dlls/mshtml/htmlstylesheet.c @@ -540,6 +540,7 @@ static HRESULT WINAPI HTMLStyleSheetsCollection_item(IHTMLStyleSheetsCollection } hres = create_style_sheet(nsstylesheet, dispex_compat_mode(&This->dispex), &stylesheet); + nsIDOMStyleSheet_Release(nsstylesheet); if(FAILED(hres)) return hres; -- 2.31.1
Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlstylesheet.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlstylesheet.c b/dlls/mshtml/htmlstylesheet.c index 52eaa21..c31c780 100644 --- a/dlls/mshtml/htmlstylesheet.c +++ b/dlls/mshtml/htmlstylesheet.c @@ -348,6 +348,7 @@ static HRESULT WINAPI HTMLStyleSheetRulesCollection_item(IHTMLStyleSheetRulesCol HTMLStyleSheetRulesCollection *This = impl_from_IHTMLStyleSheetRulesCollection(iface); nsIDOMCSSRule *nsstylesheetrule; nsresult nsres; + HRESULT hres; TRACE("(%p)->(%d %p)\n", This, index, p); @@ -357,7 +358,9 @@ static HRESULT WINAPI HTMLStyleSheetRulesCollection_item(IHTMLStyleSheetRulesCol if(!nsstylesheetrule) return E_INVALIDARG; - return create_style_sheet_rule(nsstylesheetrule, dispex_compat_mode(&This->dispex), p); + hres = create_style_sheet_rule(nsstylesheetrule, dispex_compat_mode(&This->dispex), p); + nsIDOMCSSRule_Release(nsstylesheetrule); + return hres; } static const IHTMLStyleSheetRulesCollectionVtbl HTMLStyleSheetRulesCollectionVtbl = { -- 2.31.1
Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>
This is implemented by creating a dummy HTML element and parsing the inline tag there, under a dummy tag name, so that it's filled with the parsed attributes. After that, its attributes are copied into a proper element with the proper tag name. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 154 +++++++++++++++++++++++++++++- dlls/mshtml/tests/documentmode.js | 69 +++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index de69e2f..d5463f5 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -355,6 +355,154 @@ static inline HTMLElement *impl_from_IHTMLElement(IHTMLElement *iface) return CONTAINING_RECORD(iface, HTMLElement, IHTMLElement_iface); } +static HRESULT create_nselem_with_elem_attrs(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement *elem_with_attr, nsIDOMElement **ret) +{ + nsIDOMMozNamedAttrMap *attrs; + nsAString name_str, val_str; + nsresult nsres, nsres2; + nsIDOMElement *elem; + nsIDOMAttr *attr; + UINT32 i, length; + + nsAString_InitDepend(&name_str, tag); + nsres = nsIDOMHTMLDocument_CreateElement(doc->nsdoc, &name_str, &elem); + nsAString_Finish(&name_str); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + nsres = nsIDOMElement_GetAttributes(elem_with_attr, &attrs); + if(NS_FAILED(nsres)) + goto fail; + + nsres = nsIDOMMozNamedAttrMap_GetLength(attrs, &length); + if(NS_FAILED(nsres)) { + nsIDOMMozNamedAttrMap_Release(attrs); + goto fail; + } + + nsAString_Init(&name_str, NULL); + nsAString_Init(&val_str, NULL); + for(i = 0; i < length; i++) { + nsres = nsIDOMMozNamedAttrMap_Item(attrs, i, &attr); + if(NS_FAILED(nsres)) + continue; + + nsres = nsIDOMAttr_GetNodeName(attr, &name_str); + nsres2 = nsIDOMAttr_GetNodeValue(attr, &val_str); + nsIDOMAttr_Release(attr); + if(NS_FAILED(nsres) || NS_FAILED(nsres2)) + continue; + + nsres = nsIDOMElement_SetAttribute(elem, &name_str, &val_str); + } + nsAString_Finish(&name_str); + nsAString_Finish(&val_str); + + nsIDOMMozNamedAttrMap_Release(attrs); + *ret = elem; + return S_OK; + +fail: + nsIDOMElement_Release(elem); + return E_FAIL; +} + +static HRESULT create_nselem_parse(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement **ret) +{ + static const WCHAR prefix[15] = L"<BODY><DUMMYTAG"; + static const WCHAR suffix[] = L"/></BODY>"; + WCHAR *p = wcschr(tag + 1, '>'); + UINT32 i, len, name_len, length; + nsIDOMHTMLCollection *nscol; + nsIDOMHTMLElement *nshtml; + HRESULT hres = E_FAIL; + nsresult nsres; + nsAString str; + + if(!p || p[1] || wcschr(tag + 1, '<')) + return E_FAIL; + if(!doc->nsdoc) { + WARN("NULL nsdoc\n"); + return E_UNEXPECTED; + } + if(p - tag >= 0x40000000) + return E_OUTOFMEMORY; + + /* Ignore the > or /> end token */ + len = p - tag - (p[-1] == '/'); + + /* Get the tag name using HTML whitespace rules */ + name_len = len - 1; + for(i = 1; i < len; i++) { + if((tag[i] >= 0x09 && tag[i] <= 0x0d) || tag[i] == ' ') { + name_len = i - 1; + break; + } + } + if(!name_len) + return E_FAIL; + len -= name_len + 1; + + /* Create a temporary html element and parse it there */ + nsAString_InitDepend(&str, L"HTML"); + nsres = nsIDOMHTMLDocument_CreateElement(doc->nsdoc, &str, (nsIDOMElement**)&nshtml); + nsAString_Finish(&str); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + length = ARRAY_SIZE(prefix) + ARRAY_SIZE(suffix) + len; + if(!(p = heap_alloc(length * sizeof(WCHAR)))) { + hres = E_OUTOFMEMORY; + goto fail; + } + memcpy(p, prefix, sizeof(prefix)); + memcpy(p + ARRAY_SIZE(prefix), tag + 1 + name_len, len * sizeof(WCHAR)); + memcpy(p + ARRAY_SIZE(prefix) + len, suffix, sizeof(suffix)); + + nsAString_InitDepend(&str, p); + nsres = nsIDOMHTMLElement_SetInnerHTML(nshtml, &str); + nsAString_Finish(&str); + heap_free(p); + if(NS_FAILED(nsres)) { + hres = map_nsresult(nsres); + goto fail; + } + + /* Get the dummy element and copy its attributes into the proper element */ + nsAString_InitDepend(&str, L"DUMMYTAG"); + nsres = nsIDOMHTMLElement_GetElementsByTagName(nshtml, &str, &nscol); + nsAString_Finish(&str); + if(NS_FAILED(nsres)) + goto fail; + + length = 0; + nsres = nsIDOMHTMLCollection_GetLength(nscol, &length); + if(NS_SUCCEEDED(nsres) && length == 1) { + nsIDOMElement *nselem; + nsIDOMNode *nsnode; + + nsIDOMHTMLCollection_Item(nscol, 0, &nsnode); + nsres = nsIDOMNode_QueryInterface(nsnode, &IID_nsIDOMElement, (void**)&nselem); + nsIDOMNode_Release(nsnode); + if(NS_SUCCEEDED(nsres)) { + if(!(p = heap_alloc((name_len + 1) * sizeof(WCHAR)))) + hres = E_OUTOFMEMORY; + else { + memcpy(p, tag + 1, name_len * sizeof(WCHAR)); + p[name_len] = '\0'; + hres = create_nselem_with_elem_attrs(doc, p, nselem, ret); + heap_free(p); + } + nsIDOMElement_Release(nselem); + } + } + nsIDOMHTMLCollection_Release(nscol); + +fail: + nsIDOMHTMLElement_Release(nshtml); + return hres; +} + HRESULT create_nselem(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement **ret) { nsAString tag_str; @@ -385,7 +533,11 @@ HRESULT create_element(HTMLDocumentNode *doc, const WCHAR *tag, HTMLElement **re if(!doc->nsdoc) doc = doc->node.doc; - hres = create_nselem(doc, tag, &nselem); + /* IE8 and below allow creating elements with attributes, such as <div class="a"> */ + if(tag[0] == '<' && dispex_compat_mode(&doc->node.event_target.dispex) <= COMPAT_MODE_IE8) + hres = create_nselem_parse(doc, tag, &nselem); + else + hres = create_nselem(doc, tag, &nselem); if(FAILED(hres)) return hres; diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 02a5f5f..7e85af6 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -488,6 +488,75 @@ sync_test("style_props", function() { } }); +sync_test("createElement_inline_props", function() { + var v = document.documentMode, e, s; + + if(v < 9) { + s = document.createElement("<div>").tagName; + ok(s === "DIV", "<div>.tagName returned " + s); + s = document.createElement("<div >").tagName; + ok(s === "DIV", "<div >.tagName returned " + s); + s = document.createElement("<div/>").tagName; + ok(s === "DIV", "<div/>.tagName returned " + s); + e = 0; + try { + document.createElement("<div"); + }catch(ex) { + e = ex.number; + } + ok(e === 0x4005 - 0x80000000, "<div e = " + e); + e = 0; + try { + document.createElement("<div test=1"); + }catch(ex) { + e = ex.number; + } + ok(e === 0x4005 - 0x80000000, "<div test=1 e = " + e); + + e = document.createElement("<div test='a\"' abcd=\""b"\">"); + ok(e.tagName === "DIV", "<div test=\"a\" abcd=\"b\">.tagName returned " + e.tagName); + todo_wine_if(v == 8). + ok(e.test === "a\"", "<div test='a\"' abcd=\""b"\">.test returned " + e.test); + todo_wine_if(v == 8). + ok(e.abcd === "\"b\"", "<div test='a\"' abcd=\""b"\">.abcd returned " + e.abcd); + + e = document.createElement("<html test='<'>"); + ok(e.tagName === "HTML", "<html test='<'>.tagName returned " + e.tagName); + todo_wine_if(v == 8). + ok(e.test === "<", "<html test='<'>.test returned " + e.test); + + e = document.createElement("<body test='>'>"); + ok(e.tagName === "BODY", "<body test='>'>.tagName returned " + e.tagName); + todo_wine_if(v == 8). + ok(e.test === ">", "<body test='>'>.test returned " + e.test); + }else { + s = ""; + e = 0; + try { + document.createElement("<div>"); + }catch(ex) { + s = ex.toString(); + e = ex.number; + } + todo_wine. + ok(e === undefined, "<div> e = " + e); + todo_wine. + ok(s === "InvalidCharacterError", "<div> s = " + s); + s = ""; + e = 0; + try { + document.createElement("<div test=\"a\">"); + }catch(ex) { + s = ex.toString(); + e = ex.number; + } + todo_wine. + ok(e === undefined, "<div test=\"a\"> e = " + e); + todo_wine. + ok(s === "InvalidCharacterError", "<div test=\"a\"> s = " + s); + } +}); + sync_test("JS objs", function() { var g = window; -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=100706 Your paranoid android. === w8adm (32 bit report) === mshtml: htmldoc.c:3084: Test failed: Incorrect error code: -2146697211 htmldoc.c:3089: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:5861: Test failed: expected OnChanged_1012 htmldoc.c:5862: Test failed: expected Exec_HTTPEQUIV htmldoc.c:5864: Test failed: expected Exec_SETTITLE htmldoc.c:5905: Test failed: expected FireNavigateComplete2 === w7u_adm (32 bit report) === mshtml: script.c:624: Test failed: L"/index.html?es5.js:date_now: unexpected Date.now() result 1635177239283 expected 1635177239345"
On 10/25/21 3:30 PM, Gabriel Ivăncescu wrote:
This is implemented by creating a dummy HTML element and parsing the inline tag there, under a dummy tag name, so that it's filled with the parsed attributes. After that, its attributes are copied into a proper element with the proper tag name.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 154 +++++++++++++++++++++++++++++- dlls/mshtml/tests/documentmode.js | 69 +++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-)
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index de69e2f..d5463f5 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -355,6 +355,154 @@ static inline HTMLElement *impl_from_IHTMLElement(IHTMLElement *iface) return CONTAINING_RECORD(iface, HTMLElement, IHTMLElement_iface); }
+static HRESULT create_nselem_with_elem_attrs(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement *elem_with_attr, nsIDOMElement **ret) +{ + nsIDOMMozNamedAttrMap *attrs; + nsAString name_str, val_str; + nsresult nsres, nsres2; + nsIDOMElement *elem; + nsIDOMAttr *attr; + UINT32 i, length; + + nsAString_InitDepend(&name_str, tag); + nsres = nsIDOMHTMLDocument_CreateElement(doc->nsdoc, &name_str, &elem); + nsAString_Finish(&name_str); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + nsres = nsIDOMElement_GetAttributes(elem_with_attr, &attrs); + if(NS_FAILED(nsres)) + goto fail; + + nsres = nsIDOMMozNamedAttrMap_GetLength(attrs, &length); + if(NS_FAILED(nsres)) { + nsIDOMMozNamedAttrMap_Release(attrs); + goto fail; + } + + nsAString_Init(&name_str, NULL); + nsAString_Init(&val_str, NULL); + for(i = 0; i < length; i++) { + nsres = nsIDOMMozNamedAttrMap_Item(attrs, i, &attr); + if(NS_FAILED(nsres)) + continue; + + nsres = nsIDOMAttr_GetNodeName(attr, &name_str); + nsres2 = nsIDOMAttr_GetNodeValue(attr, &val_str); + nsIDOMAttr_Release(attr); + if(NS_FAILED(nsres) || NS_FAILED(nsres2)) + continue; + + nsres = nsIDOMElement_SetAttribute(elem, &name_str, &val_str); + } + nsAString_Finish(&name_str); + nsAString_Finish(&val_str); + + nsIDOMMozNamedAttrMap_Release(attrs); + *ret = elem; + return S_OK; + +fail: + nsIDOMElement_Release(elem); + return E_FAIL; +}
Isn't Node.cloneNode what you need here?
+static HRESULT create_nselem_parse(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement **ret) +{ + static const WCHAR prefix[15] = L"<BODY><DUMMYTAG"; + static const WCHAR suffix[] = L"/></BODY>";
This whole function looks hacky. Why do you need extra <BODY> element? Could we just parse the whole tag string? Thanks, Jacek
On 01/11/2021 16:06, Jacek Caban wrote:
On 10/25/21 3:30 PM, Gabriel Ivăncescu wrote:
This is implemented by creating a dummy HTML element and parsing the inline tag there, under a dummy tag name, so that it's filled with the parsed attributes. After that, its attributes are copied into a proper element with the proper tag name.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 154 +++++++++++++++++++++++++++++- dlls/mshtml/tests/documentmode.js | 69 +++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-)
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index de69e2f..d5463f5 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -355,6 +355,154 @@ static inline HTMLElement *impl_from_IHTMLElement(IHTMLElement *iface) return CONTAINING_RECORD(iface, HTMLElement, IHTMLElement_iface); } +static HRESULT create_nselem_with_elem_attrs(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement *elem_with_attr, nsIDOMElement **ret) +{ + nsIDOMMozNamedAttrMap *attrs; + nsAString name_str, val_str; + nsresult nsres, nsres2; + nsIDOMElement *elem; + nsIDOMAttr *attr; + UINT32 i, length; + + nsAString_InitDepend(&name_str, tag); + nsres = nsIDOMHTMLDocument_CreateElement(doc->nsdoc, &name_str, &elem); + nsAString_Finish(&name_str); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + nsres = nsIDOMElement_GetAttributes(elem_with_attr, &attrs); + if(NS_FAILED(nsres)) + goto fail; + + nsres = nsIDOMMozNamedAttrMap_GetLength(attrs, &length); + if(NS_FAILED(nsres)) { + nsIDOMMozNamedAttrMap_Release(attrs); + goto fail; + } + + nsAString_Init(&name_str, NULL); + nsAString_Init(&val_str, NULL); + for(i = 0; i < length; i++) { + nsres = nsIDOMMozNamedAttrMap_Item(attrs, i, &attr); + if(NS_FAILED(nsres)) + continue; + + nsres = nsIDOMAttr_GetNodeName(attr, &name_str); + nsres2 = nsIDOMAttr_GetNodeValue(attr, &val_str); + nsIDOMAttr_Release(attr); + if(NS_FAILED(nsres) || NS_FAILED(nsres2)) + continue; + + nsres = nsIDOMElement_SetAttribute(elem, &name_str, &val_str); + } + nsAString_Finish(&name_str); + nsAString_Finish(&val_str); + + nsIDOMMozNamedAttrMap_Release(attrs); + *ret = elem; + return S_OK; + +fail: + nsIDOMElement_Release(elem); + return E_FAIL; +}
Isn't Node.cloneNode what you need here?
Thanks for the suggestion, I wasn't aware it copied the attributes to a different element.
+static HRESULT create_nselem_parse(HTMLDocumentNode *doc, const WCHAR *tag, nsIDOMElement **ret) +{ + static const WCHAR prefix[15] = L"<BODY><DUMMYTAG"; + static const WCHAR suffix[] = L"/></BODY>";
This whole function looks hacky. Why do you need extra <BODY> element? Could we just parse the whole tag string?
Would it work properly with things like <html ...> or <body ...> elements? I have tests for them and I remember testing them precisely for this reason. I remember I tried it originally to parse the whole thing, but ended up with those corner cases not working. I'll look at it again and see if I messed something up, but I can't guarantee it will work. Thanks, Gabriel
Ok so, I actually can't seem to solve this using either suggestion. First, Node.cloneNode seems to copy the entire element, unless I'm missing something. But what I need is to clone just the attributes of the DUMMYTAG element into the proper element, for example: Input: <div test="a"> What happens: SetInnerHTML('<body><dummytag test="a"/></body>') Create a "div" element, and copy all attrs (test) from dummytag to it Return the "div" element, which now has the "test" attribute How can I do this with cloneNode since it must copy to different element type? Now the second suggestion: if I use SetInnerHTML directly to the parsed tag element, it will fail to work for some of the html elements. I notice that the html element then contains <head> and <body> already, even if it just parses a div. I figured I could search for <body> and then get its child. But that's not gonna work for some of those pre-defined elements. How can I handle a <head>, <body> or <html> element parse in such situation? Do you have any ideas? Thanks, Gabriel
For IE modes 8 and above, IHTMLElement::setAttribute uses the underlying nsIDOMElement::SetAttribute, which does nothing since we're already enumerating the ns attributes, and we're supposed to create the DISPIDs for them. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlelem.c | 5 ++++- dlls/mshtml/tests/documentmode.js | 4 ---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index d5463f5..f1b09d2 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -6536,7 +6536,10 @@ static HRESULT HTMLElement_populate_props(DispatchEx *dispex) } else V_BSTR(&value) = NULL; - IHTMLElement_setAttribute(&This->IHTMLElement_iface, name, value, 0); + hres = IDispatchEx_GetDispID(&dispex->IDispatchEx_iface, name, fdexNameEnsure | fdexNameCaseInsensitive, &id); + if(SUCCEEDED(hres)) + set_elem_attr_value_by_dispid(This, id, &value); + SysFreeString(name); VariantClear(&value); } diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 7e85af6..9914db9 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -515,19 +515,15 @@ sync_test("createElement_inline_props", function() { e = document.createElement("<div test='a\"' abcd=\""b"\">"); ok(e.tagName === "DIV", "<div test=\"a\" abcd=\"b\">.tagName returned " + e.tagName); - todo_wine_if(v == 8). ok(e.test === "a\"", "<div test='a\"' abcd=\""b"\">.test returned " + e.test); - todo_wine_if(v == 8). ok(e.abcd === "\"b\"", "<div test='a\"' abcd=\""b"\">.abcd returned " + e.abcd); e = document.createElement("<html test='<'>"); ok(e.tagName === "HTML", "<html test='<'>.tagName returned " + e.tagName); - todo_wine_if(v == 8). ok(e.test === "<", "<html test='<'>.test returned " + e.test); e = document.createElement("<body test='>'>"); ok(e.tagName === "BODY", "<body test='>'>.tagName returned " + e.tagName); - todo_wine_if(v == 8). ok(e.test === ">", "<body test='>'>.test returned " + e.test); }else { s = ""; -- 2.31.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=100707 Your paranoid android. === w8adm (32 bit report) === mshtml: events.c:1089: Test failed: unexpected call img_onerror events: Timeout === w8 (32 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS === w10pro64_ar (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS === w10pro64_he (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS === w10pro64_zh_CN (64 bit report) === mshtml: htmldoc.c:2541: Test failed: unexpected call UpdateUI htmldoc.c:2853: Test failed: unexpected call Exec_UPDATECOMMANDS
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban -
Marvin