Hi Andreas,

Thanks for the patch.

On 4/15/19 9:26 PM, Andreas Maier wrote:
Enumerator is an MS extension.
This fixes installatiion of VyChat219.msi.

Signed-off-by: Andreas Maier <staubim@quantentunnel.de>
---
 dlls/jscript/Makefile.in   |   1 +
 dlls/jscript/enumerator.c  | 455 +++++++++++++++++++++++++++++++++++++++++++++
 dlls/jscript/global.c      |  18 +-
 dlls/jscript/jscript.h     |  10 +
 dlls/jscript/jsglobal.idl  |   3 -
 dlls/jscript/tests/rsrc.rc |   3 +
 dlls/jscript/tests/run.c   |   1 +
 7 files changed, 480 insertions(+), 11 deletions(-)
 create mode 100644 dlls/jscript/enumerator.c


Please split the patch. Eg. send a stub implementation first that has only a constructor that creates an instance with its methods returning E_NOTIMPL first, something that would allow a one-line test to pass:

new Enumerator([]);

That part could mostly go as is, I have some comments to object implementations bellow.


+#define DATATYPE_ARRAY           0
+#define DATATYPE_STRINGLIST      1
+#define DATATYPE_DRIVECOLLECTION 2
+
+typedef struct {
+    jsdisp_t dispex;
+
+    int index;
+    int length;
+    int datatype;
+    /* constructor with jsarray e.g. ["A","B"] */
+    jsdisp_t *array;
+    /* constructor with stringlist-interface
+       methods Items and Count are available.
+       e.g. StringList or IDriveCollection-Interface */
+    IDispatch *stringlist;
+} EnumeratorInstance;


It doesn't look right to have to implement it for each particular interface. We will likely need separate code path for internal jscript objects and general IDipsatch objects, but we shouldn't need to know anything about IDriveCollection here. This will need more testing. I've attached a tiny first step for such investigation. It uses Enumerator constructor on test-provided object. If you run it on Windows, you will see that it tries to call Invoke(DISPID_NEWENUM) and fails because we don't implement that. My guess would be that it needs DISPID_NEWENUM to succeed and then queries for IEnumVARIANT on the result, but it would need more testing. If my guess is right, we'd need just IEnumVARIANT pointer here.


diff --git a/dlls/jscript/jsglobal.idl b/dlls/jscript/jsglobal.idl
index b8604e99f2..7e0c1a30cf 100644
--- a/dlls/jscript/jsglobal.idl
+++ b/dlls/jscript/jsglobal.idl
@@ -74,9 +74,6 @@ library JSGlobal
         [id(DISPID_GLOBAL_VBARRAY)]
         VARIANT VBArray();

-        [id(DISPID_GLOBAL_ENUMERATOR)]
-        VARIANT Enumerator();
-
         [id(DISPID_GLOBAL_ESCAPE)]
         VARIANT escape(VARIANT String);


Why do you remove it?


diff --git a/dlls/jscript/tests/rsrc.rc b/dlls/jscript/tests/rsrc.rc
index d9ee6b23ae..b130df294a 100644
--- a/dlls/jscript/tests/rsrc.rc
+++ b/dlls/jscript/tests/rsrc.rc
@@ -25,6 +25,9 @@ cc.js 40 "cc.js"
 /* @makedep: lang.js */
 lang.js 40 "lang.js"

+/* @makedep: extension.js */
+extension.js 40 "extension.js"
+
 /* @makedep: regexp.js */
 regexp.js 40 "regexp.js"

diff --git a/dlls/jscript/tests/run.c b/dlls/jscript/tests/run.c
index aadb31700d..9e92a418f1 100644
--- a/dlls/jscript/tests/run.c
+++ b/dlls/jscript/tests/run.c
@@ -2684,6 +2684,7 @@ static BOOL run_tests(void)

     run_from_res("lang.js");
     run_from_res("api.js");
+    run_from_res("extension.js");


Please put tests into existing files. For plain JavaScript code, api.js would be appropriate. For test checking interaction with host objects, having short scripts inside run.c will be probably better.


Thanks,

Jacek