feat(click): retry when the element it outside of the viewport (#2330)

The element might get animated into the viewport.

Author: dgozman

src/chromium/crPage.ts

@@ -255,7 +255,7 @@ export class CRPage implements PageDelegate {
     return this._sessionForHandle(handle)._getBoundingBox(handle);
   }
 
-  async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
+  async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> {
     return this._sessionForHandle(handle)._scrollRectIntoViewIfNeeded(handle, rect);
   }
 
@@ -803,15 +803,15 @@ class FrameSession {
     return {x, y, width, height};
   }
 
-  async _scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
-    await this._client.send('DOM.scrollIntoViewIfNeeded', {
+  async _scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> {
+    return await this._client.send('DOM.scrollIntoViewIfNeeded', {
       objectId: handle._remoteObject.objectId,
       rect,
-    }).catch(e => {
+    }).then(() => 'success' as const).catch(e => {
+      if (e instanceof Error && e.message.includes('Node does not have a layout object'))
+        return 'invisible';
       if (e instanceof Error && e.message.includes('Node is detached from document'))
         throw new NotConnectedError();
-      if (e instanceof Error && e.message.includes('Node does not have a layout object'))
-        e.message = 'Node is either not visible or not an HTMLElement';
       throw e;
     });
   }

src/dom.ts

@@ -171,15 +171,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       injected.dispatchEvent(node, type, eventInit), { type, eventInit });
   }
 
-  async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise<void> {
-    await this._page._delegate.scrollRectIntoViewIfNeeded(this, rect);
+  async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise<'success' | 'invisible'> {
+    return await this._page._delegate.scrollRectIntoViewIfNeeded(this, rect);
   }
 
   async scrollIntoViewIfNeeded() {
     await this._scrollRectIntoViewIfNeeded();
   }
 
-  private async _clickablePoint(): Promise<types.Point> {
+  private async _clickablePoint(): Promise<types.Point | 'invisible' | 'outsideviewport'> {
     const intersectQuadWithViewport = (quad: types.Quad): types.Quad => {
       return quad.map(point => ({
         x: Math.min(Math.max(point.x, 0), metrics.width),
@@ -204,11 +204,11 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       this._page._delegate.layoutViewport(),
     ] as const);
     if (!quads || !quads.length)
-      throw new Error('Node is either not visible or not an HTMLElement');
+      return 'invisible';
 
     const filtered = quads.map(quad => intersectQuadWithViewport(quad)).filter(quad => computeQuadArea(quad) > 1);
     if (!filtered.length)
-      throw new Error('Node is either not visible or not an HTMLElement');
+      return 'outsideviewport';
     // Return the middle point of the first quad.
     const result = { x: 0, y: 0 };
     for (const point of filtered[0]) {
@@ -218,22 +218,18 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return result;
   }
 
-  private async _offsetPoint(offset: types.Point): Promise<types.Point> {
+  private async _offsetPoint(offset: types.Point): Promise<types.Point | 'invisible'> {
     const [box, border] = await Promise.all([
       this.boundingBox(),
       this._evaluateInUtility(({ injected, node }) => injected.getElementBorderWidth(node), {}).catch(logError(this._context._logger)),
     ]);
-    const point = { x: offset.x, y: offset.y };
-    if (box) {
-      point.x += box.x;
-      point.y += box.y;
-    }
-    if (border) {
-      // Make point relative to the padding box to align with offsetX/offsetY.
-      point.x += border.left;
-      point.y += border.top;
-    }
-    return point;
+    if (!box || !border)
+      return 'invisible';
+    // Make point relative to the padding box to align with offsetX/offsetY.
+    return {
+      x: box.x + border.left + offset.x,
+      y: box.y + border.top + offset.y,
+    };
   }
 
   async _retryPointerAction(actionName: string, action: (point: types.Point) => Promise<void>, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
@@ -257,11 +253,30 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       await this._page._delegate.setActivityPaused(true);
       paused = true;
 
-      // Scroll into view and calculate the point again while paused just in case something has moved.
       this._page._log(inputLog, 'scrolling into view if needed...');
-      await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined);
+      const scrolled = await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined);
+      if (scrolled === 'invisible') {
+        if (force)
+          throw new Error('Element is not visible');
+        this._page._log(inputLog, '...element is not visible, retrying input action');
+        return 'retry';
+      }
       this._page._log(inputLog, '...done scrolling');
-      const point = roundPoint(position ? await this._offsetPoint(position) : await this._clickablePoint());
+
+      const maybePoint = position ? await this._offsetPoint(position) : await this._clickablePoint();
+      if (maybePoint === 'invisible') {
+        if (force)
+          throw new Error('Element is not visible');
+        this._page._log(inputLog, 'element is not visibile, retrying input action');
+        return 'retry';
+      }
+      if (maybePoint === 'outsideviewport') {
+        if (force)
+          throw new Error('Element is outside of the viewport');
+        this._page._log(inputLog, 'element is outside of the viewport, retrying input action');
+        return 'retry';
+      }
+      const point = roundPoint(maybePoint);
 
       if (!force) {
         if ((options as any).__testHookBeforeHitTarget)

src/firefox/ffPage.ts

@@ -411,12 +411,12 @@ export class FFPage implements PageDelegate {
     return { x: minX, y: minY, width: maxX - minX, height: maxY - minY };
   }
 
-  async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
-    await this._session.send('Page.scrollIntoViewIfNeeded', {
+  async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> {
+    return await this._session.send('Page.scrollIntoViewIfNeeded', {
       frameId: handle._context.frame._id,
       objectId: handle._remoteObject.objectId!,
       rect,
-    }).catch(e => {
+    }).then(() => 'success' as const).catch(e => {
       if (e instanceof Error && e.message.includes('Node is detached from document'))
         throw new NotConnectedError();
       throw e;

src/page.ts

@@ -68,7 +68,7 @@ export interface PageDelegate {
   setInputFiles(handle: dom.ElementHandle<HTMLInputElement>, files: types.FilePayload[]): Promise<void>;
   getBoundingBox(handle: dom.ElementHandle): Promise<types.Rect | null>;
   getFrameElement(frame: frames.Frame): Promise<dom.ElementHandle>;
-  scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void>;
+  scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'>;
   setActivityPaused(paused: boolean): Promise<void>;
   rafCountForStablePosition(): number;
 

src/webkit/wkPage.ts

@@ -735,15 +735,15 @@ export class WKPage implements PageDelegate {
     return { x: minX, y: minY, width: maxX - minX, height: maxY - minY };
   }
 
-  async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void> {
-    await this._session.send('DOM.scrollIntoViewIfNeeded', {
+  async scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'> {
+    return await this._session.send('DOM.scrollIntoViewIfNeeded', {
       objectId: toRemoteObject(handle).objectId!,
       rect,
-    }).catch(e => {
+    }).then(() => 'success' as const).catch(e => {
+      if (e instanceof Error && e.message.includes('Node does not have a layout object'))
+        return 'invisible';
       if (e instanceof Error && e.message.includes('Node is detached from document'))
         throw new NotConnectedError();
-      if (e instanceof Error && e.message.includes('Node does not have a layout object'))
-        e.message = 'Node is either not visible or not an HTMLElement';
       throw e;
     });
   }

test/click.spec.js

@@ -138,12 +138,12 @@ describe('Page.click', function() {
     await page.click('button');
     expect(await page.evaluate(() => result)).toBe('Clicked');
   });
-  it('should not wait with false waitFor', async({page, server}) => {
+  it('should not wait with force', async({page, server}) => {
     let error = null;
     await page.goto(server.PREFIX + '/input/button.html');
     await page.$eval('button', b => b.style.display = 'none');
     await page.click('button', { force: true }).catch(e => error = e);
-    expect(error.message).toBe('Node is either not visible or not an HTMLElement');
+    expect(error.message).toBe('Element is not visible');
     expect(await page.evaluate(() => result)).toBe('Was not clicked');
   });
   it('should waitFor display:none to be gone', async({page, server}) => {
@@ -591,6 +591,61 @@ describe('Page.click', function() {
     expect(clicked).toBe(true);
     expect(await page.evaluate(() => window.clicked)).toBe(true);
   });
+  it('should retry when element is animating from outside the viewport', async({page, server}) => {
+    await page.setContent(`<style>
+      @keyframes move {
+        from { left: -300px; }
+        to { left: 0; }
+      }
+      button {
+        position: absolute;
+        left: -300px;
+        top: 0;
+        bottom: 0;
+        width: 200px;
+      }
+      button.animated {
+        animation: 1s linear 1s move forwards;
+      }
+      </style>
+      <div style="position: relative; width: 300px; height: 300px;">
+        <button onclick="window.clicked=true"></button>
+      </div>
+    `);
+    const handle = await page.$('button');
+    const promise = handle.click();
+    await handle.evaluate(button => button.className = 'animated');
+    await promise;
+    expect(await page.evaluate(() => window.clicked)).toBe(true);
+  });
+  it('should fail when element is animating from outside the viewport with force', async({page, server}) => {
+    await page.setContent(`<style>
+      @keyframes move {
+        from { left: -300px; }
+        to { left: 0; }
+      }
+      button {
+        position: absolute;
+        left: -300px;
+        top: 0;
+        bottom: 0;
+        width: 200px;
+      }
+      button.animated {
+        animation: 1s linear 1s move forwards;
+      }
+      </style>
+      <div style="position: relative; width: 300px; height: 300px;">
+        <button onclick="window.clicked=true"></button>
+      </div>
+    `);
+    const handle = await page.$('button');
+    const promise = handle.click({ force: true }).catch(e => e);
+    await handle.evaluate(button => button.className = 'animated');
+    const error = await promise;
+    expect(await page.evaluate(() => window.clicked)).toBe(undefined);
+    expect(error.message).toBe('Element is outside of the viewport');
+  });
   it('should fail when element jumps during hit testing', async({page, server}) => {
     await page.setContent('<button>Click me</button>');
     let clicked = false;

test/elementhandle.spec.js

@@ -254,25 +254,25 @@ describe('ElementHandle.click', function() {
     await button.click().catch(err => error = err);
     expect(error.message).toContain('Element is not attached to the DOM');
   });
-  it('should throw for hidden nodes', async({page, server}) => {
+  it('should throw for hidden nodes with force', async({page, server}) => {
     await page.goto(server.PREFIX + '/input/button.html');
     const button = await page.$('button');
     await page.evaluate(button => button.style.display = 'none', button);
     const error = await button.click({ force: true }).catch(err => err);
-    expect(error.message).toBe('Node is either not visible or not an HTMLElement');
+    expect(error.message).toBe('Element is not visible');
   });
-  it('should throw for recursively hidden nodes', async({page, server}) => {
+  it('should throw for recursively hidden nodes with force', async({page, server}) => {
     await page.goto(server.PREFIX + '/input/button.html');
     const button = await page.$('button');
     await page.evaluate(button => button.parentElement.style.display = 'none', button);
     const error = await button.click({ force: true }).catch(err => err);
-    expect(error.message).toBe('Node is either not visible or not an HTMLElement');
+    expect(error.message).toBe('Element is not visible');
   });
-  it('should throw for <br> elements', async({page, server}) => {
+  it('should throw for <br> elements with force', async({page, server}) => {
     await page.setContent('hello<br>goodbye');
     const br = await page.$('br');
     const error = await br.click({ force: true }).catch(err => err);
-    expect(error.message).toBe('Node is either not visible or not an HTMLElement');
+    expect(error.message).toBe('Element is outside of the viewport');
   });
 });