Merge pull request #336 from ulixee/waitForVisible

Bug fixes for SA

Author: calebjclark

commons/Timer.ts

@@ -38,7 +38,7 @@ export default class Timer {
   }
 
   public isExpired(): boolean {
-    return this.elapsedMillis() > this.timeoutMillis;
+    return this.elapsedMillis() >= this.timeoutMillis;
   }
 
   public isResolved(): boolean {

core/injected-scripts/jsPath.ts

@@ -164,42 +164,41 @@ class JsPath {
   ): Promise<IExecJsPathResult<INodeVisibility>> {
     const objectAtPath = new ObjectAtPath(jsPath, containerOffset);
     try {
-      return await new Promise<IExecJsPathResult<INodeVisibility>>(async resolve => {
-        const end = new Date();
-        end.setTime(end.getTime() + timeoutMillis);
-
-        while (new Date() < end) {
-          try {
-            if (!objectAtPath.objectAtPath) objectAtPath.lookup();
+      const end = new Date();
+      end.setTime(end.getTime() + (timeoutMillis || 0));
+
+      while (new Date() < end) {
+        try {
+          if (!objectAtPath.objectAtPath) objectAtPath.lookup();
+
+          let isElementValid = !!objectAtPath.objectAtPath;
+          let visibility: INodeVisibility = {
+            nodeExists: isElementValid,
+          };
+          if (isElementValid && waitForVisible) {
+            visibility = objectAtPath.getComputedVisibility();
+            isElementValid = visibility.isVisible;
+          }
 
-            let isElementValid = !!objectAtPath.objectAtPath;
-            let visibility: INodeVisibility = {
-              nodeExists: isElementValid,
+          if (isElementValid) {
+            return {
+              nodePointer: objectAtPath.extractNodePointer(),
+              value: visibility,
             };
-            if (isElementValid && waitForVisible) {
-              visibility = objectAtPath.getComputedVisibility();
-              isElementValid = visibility.isVisible;
-            }
-
-            if (isElementValid) {
-              return resolve({
-                nodePointer: objectAtPath.extractNodePointer(),
-                value: visibility,
-              });
-            }
-          } catch (err) {
-            // can happen if lookup path is bad
           }
-          // eslint-disable-next-line promise/param-names
-          await new Promise(resolve1 => setTimeout(resolve1, 20));
-          await new Promise(requestAnimationFrame);
+        } catch (err) {
+          if (String(err).includes('not a valid selector')) throw err;
+          // can also happen if lookup path doesn't exist yet... in which case we want to keep trying
         }
+        // eslint-disable-next-line promise/param-names
+        await new Promise(resolve1 => setTimeout(resolve1, 20));
+        await new Promise(requestAnimationFrame);
+      }
 
-        resolve(<IExecJsPathResult>{
-          nodePointer: objectAtPath.extractNodePointer(),
-          value: objectAtPath.getComputedVisibility(),
-        });
-      });
+      return {
+        nodePointer: objectAtPath.extractNodePointer(),
+        value: objectAtPath.getComputedVisibility(),
+      };
     } catch (error) {
       return objectAtPath.toReturnError(error);
     }
@@ -248,8 +247,8 @@ class ObjectAtPath {
     const element = this.closestElement;
     if (!element) return null;
     const { x, y, width, height } = element.getBoundingClientRect();
-    const centerX = Math.floor(100 * x + width / 2) / 100;
-    const centerY = Math.floor(100 * y + height / 2) / 100;
+    const centerX = round(x + width / 2);
+    const centerY = round(y + height / 2);
 
     this._obstructedByElement = document.elementFromPoint(centerX, centerY);
     return this._obstructedByElement;
@@ -349,38 +348,48 @@ class ObjectAtPath {
   }
 
   public lookup() {
-    this.objectAtPath = window;
-    if (this.jsPath[0] === 'window') this.jsPath.shift();
-    this.lookupStepIndex = 0;
-    for (const step of this.jsPath) {
-      this.lookupStep = step;
-      if (Array.isArray(step)) {
-        const [methodName, ...args] = step;
-        // extract node ids as args
-        const finalArgs = args.map(x => {
-          if (typeof x !== 'string') return x;
-          if (!x.startsWith('$$jsPath=')) return x;
-          const innerPath = JSON.parse(x.split('$$jsPath=').pop());
-          const sub = new ObjectAtPath(innerPath, this.containerOffset).lookup();
-          return sub.objectAtPath;
-        });
-        // handlers for getComputedStyle/Visibility/getNodeId/getBoundingRect
-        if (methodName.startsWith('__') && methodName.endsWith('__')) {
-          this.hasCustomMethodLookup = true;
-          this.objectAtPath = this[`${methodName.replace(/__/g, '')}`](...finalArgs);
+    try {
+      // track object as we navigate so we can extract properties along the way
+      this.objectAtPath = window;
+      this.lookupStepIndex = 0;
+      if (this.jsPath[0] === 'window') {
+        this.jsPath.shift();
+        this.lookupStepIndex = 1;
+      }
+      for (const step of this.jsPath) {
+        this.lookupStep = step;
+        if (Array.isArray(step)) {
+          const [methodName, ...args] = step;
+          // extract node ids as args
+          const finalArgs = args.map(x => {
+            if (typeof x !== 'string') return x;
+            if (!x.startsWith('$$jsPath=')) return x;
+            const innerPath = JSON.parse(x.split('$$jsPath=').pop());
+            const sub = new ObjectAtPath(innerPath, this.containerOffset).lookup();
+            return sub.objectAtPath;
+          });
+          // handlers for getComputedStyle/Visibility/getNodeId/getBoundingRect
+          if (methodName.startsWith('__') && methodName.endsWith('__')) {
+            this.hasCustomMethodLookup = true;
+            this.objectAtPath = this[`${methodName.replace(/__/g, '')}`](...finalArgs);
+          } else {
+            const methodProperty = propertyName(methodName);
+            this.objectAtPath = this.objectAtPath[methodProperty](...finalArgs);
+          }
+        } else if (typeof step === 'number') {
+          this.objectAtPath = NodeTracker.getWatchedNodeWithId(step);
+        } else if (typeof step === 'string') {
+          const prop = propertyName(step);
+          this.objectAtPath = this.objectAtPath[prop];
         } else {
-          const methodProperty = propertyName(methodName);
-          this.objectAtPath = this.objectAtPath[methodProperty](...finalArgs);
+          throw new Error('unknown JsPathStep');
         }
-      } else if (typeof step === 'number') {
-        this.objectAtPath = NodeTracker.getWatchedNodeWithId(step);
-      } else if (typeof step === 'string') {
-        const prop = propertyName(step);
-        this.objectAtPath = this.objectAtPath[prop];
-      } else {
-        throw new Error('unknown JsPathStep');
+        this.lookupStepIndex += 1;
       }
-      this.lookupStepIndex += 1;
+    } catch (err) {
+      // don't store the invalid path if we failed at a step
+      this.objectAtPath = null;
+      throw err;
     }
 
     return this;
@@ -555,3 +564,7 @@ function isIterableOrArray(object) {
   if (!object || typeof object === 'string' || object instanceof String) return false;
   return !!object[Symbol.iterator] || Array.isArray(object);
 }
+
+function round(num: number): number {
+  return Math.floor(100 * num) / 100;
+}

core/lib/FrameEnvironment.ts

@@ -393,6 +393,7 @@ b) Use the UserProfile feature to set cookies for 1 or more domains before they'
           if (!waitForVisible) isValid = isNodeVisible.value?.nodeExists;
           if (isValid) return true;
         } catch (err) {
+          if (String(err).includes('not a valid selector')) throw err;
           // don't log during loop
         }
 

core/test/navigation.test.ts

@@ -348,6 +348,43 @@ setTimeout(function() {
 
   it.todo('handles going to about:blank');
 
+  it('can wait for another tab', async () => {
+    let userAgentString1: string;
+    let userAgentString2: string;
+    koaServer.get('/tabTest', ctx => {
+      userAgentString1 = ctx.get('user-agent');
+      ctx.body = `<body>
+<a target="_blank" href="/tabTestDest">Nothing really here</a>
+</body>`;
+    });
+    koaServer.get('/tabTestDest', ctx => {
+      userAgentString2 = ctx.get('user-agent');
+      ctx.body = `<body><h1 id="newTabHeader">You are here</h1></body>`;
+    });
+    const { tab } = await createSession();
+    await tab.goto(`${koaServer.baseUrl}/tabTest`);
+    await tab.interact([
+      {
+        command: InteractionCommand.click,
+        mousePosition: ['window', 'document', ['querySelector', 'a']],
+      },
+    ]);
+
+    const session = tab.session;
+
+    const newTab = await tab.waitForNewTab();
+    expect(session.tabsById.size).toBe(2);
+    await newTab.waitForLoad('PaintingStable');
+    const header = await newTab.execJsPath([
+      'document',
+      ['querySelector', '#newTabHeader'],
+      'textContent',
+    ]);
+    expect(header.value).toBe('You are here');
+    expect(userAgentString1).toBe(userAgentString2);
+    await newTab.close();
+  });
+
   it('should not trigger location change for first navigation of new tabs', async () => {
     const { tab } = await createSession();