[PATCH 0/2] MR1031: mshtml: Document element props improvements.

[PATCH 1/2] mshtml: Allow accessing document elements as props via id if they have name.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/htmldoc.c             | 53 +++++++++++++++++++++----------
 dlls/mshtml/tests/documentmode.js | 13 ++++++--
 2 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c
index d86ed07e033..138bb2df013 100644
--- a/dlls/mshtml/htmldoc.c
+++ b/dlls/mshtml/htmldoc.c
@@ -4839,6 +4839,28 @@ static HRESULT has_elem_name(nsIDOMHTMLDocument *nsdoc, const WCHAR *name)
     return len ? S_OK : DISP_E_UNKNOWNNAME;
 }

+static HRESULT has_elem_id(nsIDOMHTMLDocument *nsdoc, const WCHAR *id)
+{
+    nsIDOMElement *nselem;
+    cpp_bool has_name;
+    nsAString nsstr;
+    nsresult nsres;
+
+    nsAString_InitDepend(&nsstr, id);
+    nsres = nsIDOMHTMLDocument_GetElementById(nsdoc, &nsstr, &nselem);
+    nsAString_Finish(&nsstr);
+    if(NS_FAILED(nsres))
+        return map_nsresult(nsres);
+    if(!nselem)
+        return DISP_E_UNKNOWNNAME;
+
+    nsAString_InitDepend(&nsstr, L"name");
+    nsIDOMElement_HasAttribute(nselem, &nsstr, &has_name);
+    nsIDOMElement_Release(nselem);
+    nsAString_Finish(&nsstr);
+    return has_name ? S_OK : DISP_E_UNKNOWNNAME;
+}
+
 static HRESULT dispid_from_elem_name(HTMLDocumentNode *This, const WCHAR *name, DISPID *dispid)
 {
     unsigned i;
@@ -4945,6 +4967,8 @@ static HRESULT WINAPI DocDispatchEx_GetDispID(IDispatchEx *iface, BSTR bstrName,

if(This->doc_node->nsdoc) {
         hres = has_elem_name(This->doc_node->nsdoc, bstrName);
+        if(hres == DISP_E_UNKNOWNNAME)
+            hres = has_elem_id(This->doc_node->nsdoc, bstrName);
         if(SUCCEEDED(hres))
             hres = dispid_from_elem_name(This->doc_node, bstrName, pid);
     }
@@ -5851,13 +5875,11 @@ static HRESULT HTMLDocumentNode_get_name(DispatchEx *dispex, DISPID id, BSTR *na
 static HRESULT HTMLDocumentNode_invoke(DispatchEx *dispex, DISPID id, LCID lcid, WORD flags, DISPPARAMS *params,
         VARIANT *res, EXCEPINFO *ei, IServiceProvider *caller)
 {
+    static WCHAR nameW[] = L"name";
     HTMLDocumentNode *This = impl_from_DispatchEx(dispex);
-    nsIDOMNodeList *node_list;
-    nsAString name_str;
-    nsIDOMNode *nsnode;
-    HTMLDOMNode *node;
+    HTMLElement *elem;
+    VARIANT_BOOL b;
     unsigned i;
-    nsresult nsres;
     HRESULT hres;

if(flags != DISPATCH_PROPERTYGET && flags != (DISPATCH_METHOD|DISPATCH_PROPERTYGET)) {
@@ -5870,23 +5892,20 @@ 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_doc_elem_by_id(This, This->elem_vars[i], &elem);
+    if(FAILED(hres))
+        return hres;
+    if(!elem)
         return DISP_E_MEMBERNOTFOUND;

-    hres = get_node(nsnode, TRUE, &node);
-    if(FAILED(hres))
+    hres = IHTMLElement6_hasAttribute(&elem->IHTMLElement6_iface, nameW, &b);
+    if(FAILED(hres) || !b) {
+        IHTMLElement6_Release(&elem->IHTMLElement6_iface);
         return hres;
+    }

V_VT(res) = VT_DISPATCH;
-    V_DISPATCH(res) = (IDispatch*)&node->IHTMLDOMNode_iface;
+    V_DISPATCH(res) = (IDispatch*)&elem->node.IHTMLDOMNode_iface;
     return S_OK;
 }

diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js
index ad258b3df2e..32f69789e55 100644
--- a/dlls/mshtml/tests/documentmode.js
+++ b/dlls/mshtml/tests/documentmode.js
@@ -668,7 +668,7 @@ sync_test("for..in", function() {
 });

sync_test("elem_by_id", function() {
-    document.body.innerHTML = '<form id="testid" name="testname"></form>';
+    document.body.innerHTML = '<form id="testid" name="testname"></form><div id="foobar"></div>';
     var found;

var id_elem = document.getElementById("testid");
@@ -682,16 +682,23 @@ sync_test("elem_by_id", function() {

id_elem = window.testid;
     ok(id_elem.tagName === "FORM", "window.testid = " + id_elem);
+    name_elem = window.testname;
+    ok(name_elem.tagName === "FORM", "window.testname = " + name_elem);
+    id_elem = window.foobar;
+    ok(id_elem.tagName === "DIV", "window.foobar = " + id_elem);

+    id_elem = document.testid;
+    ok(id_elem.tagName === "FORM", "document.testid = " + id_elem);
     name_elem = document.testname;
     ok(name_elem.tagName === "FORM", "document.testname = " + name_elem);
+    ok(!("foobar" in document), "foobar in document");

for(id_elem in window)
-        ok(id_elem !== "testid" && id_elem != "testname", id_elem + " was enumerated in window");
+        ok(id_elem !== "testid" && id_elem != "testname" && id_elem != "foobar", id_elem + " was enumerated in window");
     window.testid = 137;
     found = false;
     for(id_elem in window) {
-        ok(id_elem != "testname", id_elem + " was enumerated in window after set to 137");
+        ok(id_elem != "testname" && id_elem != "foobar", id_elem + " was enumerated in window after set to 137");
         if(id_elem === "testid")
             found = true;
     }

[PATCH 2/2] mshtml: Don't create dynamic prop before checking if elem prop even exists.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/htmldoc.c             | 11 ++++----
 dlls/mshtml/mshtml_private.h      |  1 +
 dlls/mshtml/tests/documentmode.js | 46 +++++++++++++++++++++++++++++--
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c
index 138bb2df013..4fb634115ef 100644
--- a/dlls/mshtml/htmldoc.c
+++ b/dlls/mshtml/htmldoc.c
@@ -4961,7 +4961,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;

@@ -4972,6 +4972,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;
 }

@@ -5882,10 +5885,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 32f69789e55..2a060cebcea 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><div id="foobar"></div>';
-    var found;
+    var v = document.documentMode, found;

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");
@@ -711,6 +711,48 @@ sync_test("elem_by_id", function() {
             found = true;
     }
     ok(found, "testname was not enumerated in document");
+
+    try {
+        document.testname();
+    }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");
+    }catch(e) {
+        todo_wine_if(v >= 9).
+        ok(v < 9 && e.number === 0xa01b6 - 0x80000000, "Setting document.testid threw = " + e.number);
+    }
 });

sync_test("doc_mode", function() {

[PATCH 2/2] mshtml: Don't create dynamic prop before checking if elem prop even exists.

[Marvin]

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=124892

Your paranoid android.

=== debian11 (32 bit report) ===

urlmon:
protocol.c:3682: Test failed: wait timed out
protocol: Timeout

=== debian11 (build log) ===

Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24666.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24666.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24666.