feat(input): retry when hit target check fails, prepare for page pause (#2020)

Author: dgozman

src/chromium/crPage.ts

@@ -258,6 +258,10 @@ export class CRPage implements PageDelegate {
     return this._sessionForHandle(handle)._scrollRectIntoViewIfNeeded(handle, rect);
   }
 
+  async setActivityPaused(paused: boolean): Promise<void> {
+    await this._forAllFrameSessions(frame => frame._setActivityPaused(paused));
+  }
+
   async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     return this._sessionForHandle(handle)._getContentQuads(handle);
   }
@@ -795,6 +799,9 @@ class FrameSession {
     });
   }
 
+  async _setActivityPaused(paused: boolean): Promise<void> {
+  }
+
   async _getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     const result = await this._client.send('DOM.getContentQuads', {
       objectId: toRemoteObject(handle).objectId

src/dom.ts

@@ -163,9 +163,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise<void> {
-    this._page._log(inputLog, 'scrolling into view if needed...');
     await this._page._delegate.scrollRectIntoViewIfNeeded(this, rect);
-    this._page._log(inputLog, '...done');
   }
 
   async scrollIntoViewIfNeeded() {
@@ -229,44 +227,78 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return point;
   }
 
-  async _performPointerAction(action: (point: types.Point) => Promise<void>, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
+  async _retryPointerAction(action: (point: types.Point) => Promise<void>, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<void> {
     const deadline = this._page._timeoutSettings.computeDeadline(options);
-    const { force = false } = (options || {});
-    if (!force)
+    while (!helper.isPastDeadline(deadline)) {
+      const result = await this._performPointerAction(action, deadline, options);
+      if (result === 'done')
+        return;
+    }
+    throw new TimeoutError(`waiting for element to receive pointer events failed: timeout exceeded`);
+  }
+
+  async _performPointerAction(action: (point: types.Point) => Promise<void>, deadline: number, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise<'done' | 'retry'> {
+    const { force = false, position } = options;
+    if (!force && !(options as any).__testHookSkipStablePosition)
       await this._waitForDisplayedAtStablePosition(deadline);
-    const position = options ? options.position : undefined;
-    await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined);
-    const point = position ? await this._offsetPoint(position) : await this._clickablePoint();
-    point.x = (point.x * 100 | 0) / 100;
-    point.y = (point.y * 100 | 0) / 100;
-    await this._page.mouse.move(point.x, point.y);  // Force any hover effects before waiting for hit target.
-    if (options && (options as any).__testHookBeforeWaitForHitTarget)
-      await (options as any).__testHookBeforeWaitForHitTarget();
-    if (!force)
-      await this._waitForHitTargetAt(point, deadline);
 
-    await this._page._frameManager.waitForSignalsCreatedBy(async () => {
-      let restoreModifiers: input.Modifier[] | undefined;
-      if (options && options.modifiers)
-        restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
-      this._page._log(inputLog, 'performing input action...');
-      await action(point);
-      this._page._log(inputLog, '...done');
-      if (restoreModifiers)
-        await this._page.keyboard._ensureModifiers(restoreModifiers);
-    }, deadline, options, true);
+    let paused = false;
+    try {
+      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);
+      this._page._log(inputLog, '...done scrolling');
+      const point = roundPoint(position ? await this._offsetPoint(position) : await this._clickablePoint());
+
+      if (!force) {
+        if ((options as any).__testHookBeforeHitTarget)
+          await (options as any).__testHookBeforeHitTarget();
+        this._page._log(inputLog, `checking that element receives pointer events at (${point.x},${point.y})...`);
+        const matchesHitTarget = await this._checkHitTargetAt(point);
+        if (!matchesHitTarget) {
+          this._page._log(inputLog, '...element does not receive pointer events, retrying input action');
+          await this._page._delegate.setActivityPaused(false);
+          paused = false;
+          return 'retry';
+        }
+        this._page._log(inputLog, `...element does receive pointer events, continuing input action`);
+      }
+
+      await this._page._frameManager.waitForSignalsCreatedBy(async () => {
+        let restoreModifiers: input.Modifier[] | undefined;
+        if (options && options.modifiers)
+          restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
+        this._page._log(inputLog, 'performing input action...');
+        await action(point);
+        this._page._log(inputLog, '...input action done');
+        this._page._log(inputLog, 'waiting for navigations to finish...');
+        await this._page._delegate.setActivityPaused(false);
+        paused = false;
+        if (restoreModifiers)
+          await this._page.keyboard._ensureModifiers(restoreModifiers);
+      }, deadline, options, true);
+      this._page._log(inputLog, '...navigations have finished');
+
+      return 'done';
+    } finally {
+      if (paused)
+        await this._page._delegate.setActivityPaused(false);
+    }
   }
 
   hover(options?: PointerActionOptions & types.PointerActionWaitOptions): Promise<void> {
-    return this._performPointerAction(point => this._page.mouse.move(point.x, point.y), options);
+    return this._retryPointerAction(point => this._page.mouse.move(point.x, point.y), options);
   }
 
   click(options?: ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<void> {
-    return this._performPointerAction(point => this._page.mouse.click(point.x, point.y, options), options);
+    return this._retryPointerAction(point => this._page.mouse.click(point.x, point.y, options), options);
   }
 
   dblclick(options?: MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<void> {
-    return this._performPointerAction(point => this._page.mouse.dblclick(point.x, point.y, options), options);
+    return this._retryPointerAction(point => this._page.mouse.dblclick(point.x, point.y, options), options);
   }
 
   async selectOption(values: string | ElementHandle | types.SelectOption | string[] | ElementHandle[] | types.SelectOption[], options?: types.NavigatingActionWaitOptions): Promise<string[]> {
@@ -429,11 +461,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     const timeoutMessage = 'element to be displayed and not moving';
     const injectedResult = await helper.waitWithDeadline(stablePromise, timeoutMessage, deadline);
     handleInjectedResult(injectedResult, timeoutMessage);
-    this._page._log(inputLog, '...done');
+    this._page._log(inputLog, '...element is displayed and does not move');
   }
 
-  async _waitForHitTargetAt(point: types.Point, deadline: number): Promise<void> {
-    this._page._log(inputLog, `waiting for element to receive pointer events at (${point.x},${point.y}) ...`);
+  async _checkHitTargetAt(point: types.Point): Promise<boolean> {
     const frame = await this.ownerFrame();
     if (frame && frame.parentFrame()) {
       const element = await frame.frameElement();
@@ -443,13 +474,10 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       // Translate from viewport coordinates to frame coordinates.
       point = { x: point.x - box.x, y: point.y - box.y };
     }
-    const hitTargetPromise = this._evaluateInUtility(({ injected, node }, { timeout, point }) => {
-      return injected.waitForHitTargetAt(node, timeout, point);
-    }, { timeout: helper.timeUntilDeadline(deadline), point });
-    const timeoutMessage = 'element to receive pointer events';
-    const injectedResult = await helper.waitWithDeadline(hitTargetPromise, timeoutMessage, deadline);
-    handleInjectedResult(injectedResult, timeoutMessage);
-    this._page._log(inputLog, '...done');
+    const injectedResult = await this._evaluateInUtility(({ injected, node }, { point }) => {
+      return injected.checkHitTargetAt(node, point);
+    }, { point });
+    return handleInjectedResult(injectedResult, '');
   }
 }
 
@@ -470,3 +498,10 @@ function handleInjectedResult<T = undefined>(injectedResult: InjectedResult<T>,
     throw new Error(injectedResult.error);
   return injectedResult.value as T;
 }
+
+function roundPoint(point: types.Point): types.Point {
+  return {
+    x: (point.x * 100 | 0) / 100,
+    y: (point.y * 100 | 0) / 100,
+  };
+}

src/firefox/ffPage.ts

@@ -428,6 +428,9 @@ export class FFPage implements PageDelegate {
     });
   }
 
+  async setActivityPaused(paused: boolean): Promise<void> {
+  }
+
   async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     const result = await this._session.send('Page.getContentQuads', {
       frameId: handle._context.frame._id,

src/injected/injected.ts

@@ -324,30 +324,16 @@ export class Injected {
     return { status: result === 'notconnected' ? 'notconnected' : (result ? 'success' : 'timeout') };
   }
 
-  async waitForHitTargetAt(node: Node, timeout: number, point: types.Point): Promise<InjectedResult> {
-    const targetElement = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
-    let element = targetElement;
+  checkHitTargetAt(node: Node, point: types.Point): InjectedResult<boolean> {
+    let element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
     while (element && window.getComputedStyle(element).pointerEvents === 'none')
       element = element.parentElement;
-    if (!element)
-      return { status: 'notconnected' };
-    const result = await this.poll('raf', timeout, (): 'notconnected' | 'moved' | boolean => {
-      if (!element!.isConnected)
-        return 'notconnected';
-      const clientRect = targetElement!.getBoundingClientRect();
-      if (clientRect.left > point.x || clientRect.left + clientRect.width < point.x ||
-          clientRect.top > point.y || clientRect.top + clientRect.height < point.y)
-        return 'moved';
-      let hitElement = this._deepElementFromPoint(document, point.x, point.y);
-      while (hitElement && hitElement !== element)
-        hitElement = this._parentElementOrShadowHost(hitElement);
-      return hitElement === element;
-    });
-    if (result === 'notconnected')
+    if (!element || !element.isConnected)
       return { status: 'notconnected' };
-    if (result === 'moved')
-      return { status: 'error', error: 'Element has moved during the action' };
-    return { status: result ? 'success' : 'timeout' };
+    let hitElement = this._deepElementFromPoint(document, point.x, point.y);
+    while (hitElement && hitElement !== element)
+      hitElement = this._parentElementOrShadowHost(hitElement);
+    return { status: 'success', value: hitElement === element };
   }
 
   dispatchEvent(node: Node, type: string, eventInit: Object) {

src/page.ts

@@ -69,6 +69,7 @@ export interface PageDelegate {
   getBoundingBox(handle: dom.ElementHandle): Promise<types.Rect | null>;
   getFrameElement(frame: frames.Frame): Promise<dom.ElementHandle>;
   scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<void>;
+  setActivityPaused(paused: boolean): Promise<void>;
 
   getAccessibilityTree(needle?: dom.ElementHandle): Promise<{tree: accessibility.AXNode, needle: accessibility.AXNode | null}>;
   pdf?: (options?: types.PDFOptions) => Promise<Buffer>;

src/webkit/wkPage.ts

@@ -731,6 +731,9 @@ export class WKPage implements PageDelegate {
     });
   }
 
+  async setActivityPaused(paused: boolean): Promise<void> {
+  }
+
   async getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
     const result = await this._session.send('DOM.getContentQuads', {
       objectId: toRemoteObject(handle).objectId!

test/assets/input/animating-button.html

@@ -14,21 +14,29 @@
   button.addEventListener('click', () => window.clicked = true);
   document.body.appendChild(button);
 }
+
 function stopButton(remove) {
   const button = document.querySelector('button');
   button.style.marginLeft = button.getBoundingClientRect().left + 'px';
   button.style.animation = '';
   if (remove)
     button.remove();
 }
-function startJumping() {
+
+let x = 0;
+function jump() {
+  x += 300;
   const button = document.querySelector('button');
-  let x = 0;
+  button.style.marginLeft = x + 'px';
+}
+
+function startJumping() {
+  x = 0;
   const moveIt = () => {
-    x += 300;
-    button.style.marginLeft = x + 'px';
+    jump();
     requestAnimationFrame(moveIt);
   };
+  setInterval(jump, 0);
   moveIt();
 }
 </script>

test/click.spec.js

@@ -221,7 +221,6 @@ describe('Page.click', function() {
       'mouseover',
       'mouseenter',
       'mousemove',
-      'mousemove',
       'mousedown',
       'mouseup',
       'click',
@@ -571,37 +570,50 @@ describe('Page.click', function() {
     expect(clicked).toBe(true);
     expect(await page.evaluate(() => window.clicked)).toBe(true);
   });
-  it('should fail when element moves during hit testing', async({page, server}) => {
+  it('should retry when element jumps during hit testing', async({page, server}) => {
     await page.goto(server.PREFIX + '/input/animating-button.html');
     await page.evaluate(() => addButton());
     let clicked = false;
     const handle = await page.$('button');
-    const __testHookBeforeWaitForHitTarget = () => page.evaluate(() => startJumping());
-    const promise = handle.click({ timeout: 0, __testHookBeforeWaitForHitTarget }).then(() => clicked = true).catch(e => e);
+    const __testHookBeforeHitTarget = () => page.evaluate(() => { if (window.x === 0) jump(); });
+    const promise = handle.click({ timeout: 0, __testHookBeforeHitTarget }).then(() => clicked = true);
     expect(clicked).toBe(false);
     expect(await page.evaluate(() => window.clicked)).toBe(undefined);
     await page.evaluate(() => stopButton());
+    await promise;
+    expect(clicked).toBe(true);
+    expect(await page.evaluate(() => window.clicked)).toBe(true);
+  });
+  it('should fail when element jumps during hit testing', async({page, server}) => {
+    await page.goto(server.PREFIX + '/input/animating-button.html');
+    await page.evaluate(() => addButton());
+    await page.evaluate(() => stopButton());
+    let clicked = false;
+    const handle = await page.$('button');
+    const __testHookBeforeHitTarget = () => page.evaluate(() => jump());
+    const promise = handle.click({ timeout: 1000, __testHookBeforeHitTarget, __testHookSkipStablePosition: true }).then(() => clicked = true).catch(e => e);
     const error = await promise;
     expect(clicked).toBe(false);
-    expect(error.message).toBe('Element has moved during the action');
     expect(await page.evaluate(() => window.clicked)).toBe(undefined);
-  });
-  it('should fail when element is blocked on hover', async({page, server}) => {
-    await page.setContent(`<style>
-      container { display: block; position: relative; width: 200px; height: 50px; }
-      div, button { position: absolute; left: 0; top: 0; bottom: 0; right: 0; }
-      div { pointer-events: none; }
-      container:hover div { pointer-events: auto; background: red; }
-    </style>
-    <container>
-      <button onclick="window.clicked=true">Click me</button>
-      <div></div>
-    </container>`);
-    const error = await page.click('button', { timeout: 3000 }).catch(e => e);
     expect(error.message).toBe('waiting for element to receive pointer events failed: timeout exceeded');
+  });
+  it.fail(CHROMIUM || WEBKIT || FFOX)('should work when element jumps uncontrollably', async({page, server}) => {
+    // This test requires pausing the page.
+    await page.goto(server.PREFIX + '/input/animating-button.html');
+    await page.evaluate(() => addButton());
+    await page.evaluate(() => stopButton());
+    const handle = await page.$('button');
+    await page.evaluate(() => startJumping());
+    let clicked = false;
+    const promise = handle.click({ timeout: 1000, __testHookSkipStablePosition: true }).then(() => clicked = true);
+    expect(clicked).toBe(false);
     expect(await page.evaluate(() => window.clicked)).toBe(undefined);
+    await promise;
+    expect(clicked).toBe(true);
+    expect(await page.evaluate(() => window.clicked)).toBe(true);
   });
-  it('should wait while element is blocked on hover', async({page, server}) => {
+  it.fail(CHROMIUM || WEBKIT || FFOX)('should wait while element is blocked on hover', async({page, server}) => {
+    // This test requires pausing the page.
     await page.setContent(`<style>
       @keyframes move-out { from { marign-left: 0; } to { margin-left: 150px; } }
       container { display: block; position: relative; width: 200px; height: 50px; }