-- v4: mshtml: Don't create dynamic prop before checking if elem prop even exists. mshtml: Allow accessing some document elements as props via id. mshtml: Expose props via element name only for specific element types.
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmldoc.c | 31 ++++++++++++++++----------- dlls/mshtml/tests/documentmode.js | 35 ++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 13 deletions(-)
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index d86ed07e033..aa71e53d144 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -4820,23 +4820,30 @@ static inline HTMLDocument *impl_from_IDispatchEx(IDispatchEx *iface)
static HRESULT has_elem_name(nsIDOMHTMLDocument *nsdoc, const WCHAR *name) { - nsIDOMNodeList *node_list; - nsAString name_str; + static const WCHAR fmt[] = L":-moz-any(applet,embed,form,iframe,img,object)[name="%s"]"; + WCHAR buf[128], *selector = buf; + nsAString selector_str; + nsIDOMElement *nselem; nsresult nsres; - UINT32 len; + size_t len;
- nsAString_InitDepend(&name_str, name); - nsres = nsIDOMHTMLDocument_GetElementsByName(nsdoc, &name_str, &node_list); - nsAString_Finish(&name_str); - if(NS_FAILED(nsres)) - return map_nsresult(nsres); + len = wcslen(name) + ARRAY_SIZE(fmt) - 2 /* %s */; + if(len > ARRAY_SIZE(buf) && !(selector = heap_alloc(len * sizeof(WCHAR)))) + return E_OUTOFMEMORY; + swprintf(selector, len, fmt, name);
- nsres = nsIDOMNodeList_GetLength(node_list, &len); - nsIDOMNodeList_Release(node_list); + nsAString_InitDepend(&selector_str, selector); + nsres = nsIDOMHTMLDocument_QuerySelector(nsdoc, &selector_str, &nselem); + nsAString_Finish(&selector_str); + if(selector != buf) + heap_free(selector); if(NS_FAILED(nsres)) return map_nsresult(nsres);
- return len ? S_OK : DISP_E_UNKNOWNNAME; + if(!nselem) + return DISP_E_UNKNOWNNAME; + nsIDOMElement_Release(nselem); + return S_OK; }
static HRESULT dispid_from_elem_name(HTMLDocumentNode *This, const WCHAR *name, DISPID *dispid) @@ -5918,7 +5925,7 @@ static HRESULT HTMLDocumentNode_next_dispid(DispatchEx *dispex, DISPID id, DISPI }
/* Populate possibly missing DISPIDs */ - nsAString_InitDepend(&nsstr, L"[name]"); + nsAString_InitDepend(&nsstr, L":-moz-any(applet,embed,form,iframe,img,object)[name]"); nsres = nsIDOMHTMLDocument_QuerySelectorAll(This->nsdoc, &nsstr, &node_list); nsAString_Finish(&nsstr); if(NS_FAILED(nsres)) diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index ad258b3df2e..5aa9759fe81 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -669,7 +669,7 @@ sync_test("for..in", function() {
sync_test("elem_by_id", function() { document.body.innerHTML = '<form id="testid" name="testname"></form>'; - var found; + var found, i;
var id_elem = document.getElementById("testid"); ok(id_elem.tagName === "FORM", "id_elem.tagName = " + id_elem.tagName); @@ -704,6 +704,39 @@ sync_test("elem_by_id", function() { found = true; } ok(found, "testname was not enumerated in document"); + + // these tags expose name as props, and id only if they have a name + var tags = [ "embed", "form", "iframe", "img" ]; + for(i in tags) { + var tag = tags[i]; + document.body.innerHTML = '<' + tag + ' id="testid" name="testname"></' + tag + '><' + tag + ' id="foobar"></' + tag + '>'; + ok("testname" in document, tag + " did not expose testname"); + todo_wine. + ok("testid" in document, tag + " did not expose testid"); + ok(!("foobar" in document), tag + " exposed foobar"); + } + + // these tags always expose their id as well as name (we don't test applet because it makes Windows pop up a dialog box) + tags = [ "object" ]; + for(i in tags) { + var tag = tags[i]; + document.body.innerHTML = '<' + tag + ' id="testid" name="testname"></' + tag + '><' + tag + ' id="foobar"></' + tag + '>'; + ok("testname" in document, tag + " did not expose testname"); + todo_wine. + ok("testid" in document, tag + " did not expose testid"); + todo_wine. + ok("foobar" in document, tag + " did not expose foobar"); + } + + // all other tags don't expose props for either id or name, test a few of them here + tags = [ "a", "b", "body", "center", "div", "frame", "h2", "head", "html", "input", "meta", "p", "span", "style", "table", "winetest" ]; + for(i in tags) { + var tag = tags[i]; + document.body.innerHTML = '<' + tag + ' id="testid" name="testname"></' + tag + '><' + tag + ' id="foobar"></' + tag + '>'; + ok(!("testname" in document), tag + " exposed testname"); + ok(!("testid" in document), tag + " exposed testid"); + ok(!("foobar" in document), tag + " exposed foobar"); + } });
sync_test("doc_mode", function() {
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmldoc.c | 58 +++++++++++++++++++++++-------- dlls/mshtml/tests/documentmode.js | 3 -- 2 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index aa71e53d144..5805138f4b2 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -4846,6 +4846,41 @@ static HRESULT has_elem_name(nsIDOMHTMLDocument *nsdoc, const WCHAR *name) return S_OK; }
+static HRESULT get_elem_by_name_or_id(nsIDOMHTMLDocument *nsdoc, const WCHAR *name, nsIDOMElement **ret) +{ + static const WCHAR fmt[] = L":-moz-any(embed,form,iframe,img):-moz-any([name="%s"],[id="%s"][name])," + L":-moz-any(applet,object):-moz-any([name="%s"],[id="%s"])"; + WCHAR buf[384], *selector = buf; + nsAString selector_str; + nsIDOMElement *nselem; + nsresult nsres; + size_t len; + + len = wcslen(name) * 4 + ARRAY_SIZE(fmt) - 8 /* %s */; + if(len > ARRAY_SIZE(buf) && !(selector = heap_alloc(len * sizeof(WCHAR)))) + return E_OUTOFMEMORY; + swprintf(selector, len, fmt, name, name, name, name); + + nsAString_InitDepend(&selector_str, selector); + nsres = nsIDOMHTMLDocument_QuerySelector(nsdoc, &selector_str, &nselem); + nsAString_Finish(&selector_str); + if(selector != buf) + heap_free(selector); + if(NS_FAILED(nsres)) + return map_nsresult(nsres); + + if(ret) { + *ret = nselem; + return S_OK; + } + + if(nselem) { + nsIDOMElement_Release(nselem); + return S_OK; + } + return DISP_E_UNKNOWNNAME; +} + static HRESULT dispid_from_elem_name(HTMLDocumentNode *This, const WCHAR *name, DISPID *dispid) { unsigned i; @@ -4951,7 +4986,7 @@ static HRESULT WINAPI DocDispatchEx_GetDispID(IDispatchEx *iface, BSTR bstrName, return hres;
if(This->doc_node->nsdoc) { - hres = has_elem_name(This->doc_node->nsdoc, bstrName); + hres = get_elem_by_name_or_id(This->doc_node->nsdoc, bstrName, NULL); if(SUCCEEDED(hres)) hres = dispid_from_elem_name(This->doc_node, bstrName, pid); } @@ -5859,12 +5894,9 @@ static HRESULT HTMLDocumentNode_invoke(DispatchEx *dispex, DISPID id, LCID lcid, VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller) { HTMLDocumentNode *This = impl_from_DispatchEx(dispex); - nsIDOMNodeList *node_list; - nsAString name_str; - nsIDOMNode *nsnode; + nsIDOMElement *nselem; HTMLDOMNode *node; unsigned i; - nsresult nsres; HRESULT hres;
if(flags != DISPATCH_PROPERTYGET && flags != (DISPATCH_METHOD|DISPATCH_PROPERTYGET)) { @@ -5877,18 +5909,14 @@ static HRESULT HTMLDocumentNode_invoke(DispatchEx *dispex, DISPID id, LCID lcid, if(!This->nsdoc || i >= This->elem_vars_cnt) return DISP_E_MEMBERNOTFOUND;
- nsAString_InitDepend(&name_str, This->elem_vars[i]); - nsres = nsIDOMHTMLDocument_GetElementsByName(This->nsdoc, &name_str, &node_list); - nsAString_Finish(&name_str); - if(NS_FAILED(nsres)) - return E_FAIL; - - nsres = nsIDOMNodeList_Item(node_list, 0, &nsnode); - nsIDOMNodeList_Release(node_list); - if(NS_FAILED(nsres) || !nsnode) + hres = get_elem_by_name_or_id(This->nsdoc, This->elem_vars[i], &nselem); + if(FAILED(hres)) + return hres; + if(!nselem) return DISP_E_MEMBERNOTFOUND;
- hres = get_node(nsnode, TRUE, &node); + hres = get_node((nsIDOMNode*)nselem, TRUE, &node); + nsIDOMElement_Release(nselem); if(FAILED(hres)) return hres;
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 5aa9759fe81..fa264526302 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -711,7 +711,6 @@ sync_test("elem_by_id", function() { var tag = tags[i]; document.body.innerHTML = '<' + tag + ' id="testid" name="testname"></' + tag + '><' + tag + ' id="foobar"></' + tag + '>'; ok("testname" in document, tag + " did not expose testname"); - todo_wine. ok("testid" in document, tag + " did not expose testid"); ok(!("foobar" in document), tag + " exposed foobar"); } @@ -722,9 +721,7 @@ sync_test("elem_by_id", function() { var tag = tags[i]; document.body.innerHTML = '<' + tag + ' id="testid" name="testname"></' + tag + '><' + tag + ' id="foobar"></' + tag + '>'; ok("testname" in document, tag + " did not expose testname"); - todo_wine. ok("testid" in document, tag + " did not expose testid"); - todo_wine. ok("foobar" in document, tag + " did not expose foobar"); }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmldoc.c | 11 +++---- dlls/mshtml/mshtml_private.h | 1 + dlls/mshtml/tests/documentmode.js | 49 +++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 5805138f4b2..2a99316d129 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -4981,7 +4981,7 @@ static HRESULT WINAPI DocDispatchEx_GetDispID(IDispatchEx *iface, BSTR bstrName, HTMLDocument *This = impl_from_IDispatchEx(iface); HRESULT hres;
- hres = IDispatchEx_GetDispID(This->dispex, bstrName, grfdex, pid); + hres = IDispatchEx_GetDispID(This->dispex, bstrName, grfdex & ~fdexNameEnsure, pid); if(hres != DISP_E_UNKNOWNNAME) return hres;
@@ -4990,6 +4990,9 @@ static HRESULT WINAPI DocDispatchEx_GetDispID(IDispatchEx *iface, BSTR bstrName, if(SUCCEEDED(hres)) hres = dispid_from_elem_name(This->doc_node, bstrName, pid); } + + if(hres == DISP_E_UNKNOWNNAME && (grfdex & fdexNameEnsure)) + hres = IDispatchEx_GetDispID(This->dispex, bstrName, grfdex, pid); return hres; }
@@ -5899,10 +5902,8 @@ static HRESULT HTMLDocumentNode_invoke(DispatchEx *dispex, DISPID id, LCID lcid, unsigned i; HRESULT hres;
- if(flags != DISPATCH_PROPERTYGET && flags != (DISPATCH_METHOD|DISPATCH_PROPERTYGET)) { - FIXME("unsupported flags %x\n", flags); - return E_NOTIMPL; - } + if(flags != DISPATCH_PROPERTYGET && flags != (DISPATCH_METHOD|DISPATCH_PROPERTYGET)) + return MSHTML_E_INVALID_PROPERTY;
i = id - MSHTML_DISPID_CUSTOM_MIN;
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 4ce90d3f17e..7ac4cea2e2b 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -72,6 +72,7 @@
#define NSAPI WINAPI
+#define MSHTML_E_INVALID_PROPERTY 0x800a01b6 #define MSHTML_E_INVALID_ACTION 0x800a01bd #define MSHTML_E_NODOC 0x800a025c
diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index fa264526302..a9ff38d5781 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -669,13 +669,13 @@ sync_test("for..in", function() {
sync_test("elem_by_id", function() { document.body.innerHTML = '<form id="testid" name="testname"></form>'; - var found, i; + var v = document.documentMode, found, i;
var id_elem = document.getElementById("testid"); ok(id_elem.tagName === "FORM", "id_elem.tagName = " + id_elem.tagName);
var name_elem = document.getElementById("testname"); - if(document.documentMode < 8) + if(v < 8) ok(id_elem === name_elem, "id_elem != id_elem"); else ok(name_elem === null, "name_elem != null"); @@ -705,6 +705,51 @@ sync_test("elem_by_id", function() { } ok(found, "testname was not enumerated in document");
+ try { + document.testname(); + ok(false, "document.testname() did not throw exception"); + }catch(e) { + ok(e.number === 0xa01b6 - 0x80000000, "document.testname() threw = " + e.number); + } + + try { + document.testname = "foo"; + ok(v >= 9, "Setting document.testname did not throw exception"); + + id_elem = document.testid; + ok(id_elem.tagName === "FORM", "document.testid after set = " + id_elem); + name_elem = document.testname; + ok(name_elem === "foo", "document.testname after set = " + name_elem); + }catch(e) { + todo_wine_if(v >= 9). + ok(v < 9 && e.number === 0xa01b6 - 0x80000000, "Setting document.testname threw = " + e.number); + } + + try { + document.testid = "bar"; + ok(v >= 9, "Setting document.testid did not throw exception"); + + id_elem = document.testid; + ok(id_elem === "bar", "document.testid after both set = " + id_elem); + name_elem = document.testname; + ok(name_elem === "foo", "document.testname after both set = " + name_elem); + + found = false, name_elem = false; + for(id_elem in document) { + if(id_elem === "testid") + found = true; + if(id_elem === "testname") + name_elem = true; + } + ok(found, "testid was not enumerated in document after both set"); + ok(name_elem, "testname was not enumerated in document after both set"); + delete document.testid; + delete document.testname; + }catch(e) { + todo_wine_if(v >= 9). + ok(v < 9 && e.number === 0xa01b6 - 0x80000000, "Setting document.testid threw = " + e.number); + } + // these tags expose name as props, and id only if they have a name var tags = [ "embed", "form", "iframe", "img" ]; for(i in tags) {
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=124966
Your paranoid android.
=== debian11 (build log) ===
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24721. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24721. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24721.
I've also quoted the value in the selector to be safe.
This merge request was approved by Jacek Caban.
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)
-
Marvin