From 793586e42ca9664360425c192d34b4c65e11cad9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 23 Apr 2020 14:44:06 -0700 Subject: [PATCH] fix(click): throw instead of timing out when the element has moved (#1942) --- src/injected/injected.ts | 15 +++++++++--- test/click.spec.js | 49 ---------------------------------------- 2 files changed, 12 insertions(+), 52 deletions(-) diff --git a/src/injected/injected.ts b/src/injected/injected.ts index 37a1041c3a..c585487392 100644 --- a/src/injected/injected.ts +++ b/src/injected/injected.ts @@ -321,20 +321,29 @@ export class Injected { } async waitForHitTargetAt(node: Node, timeout: number, point: types.Point): Promise { - let element = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement; + const targetElement = node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement; + let element = targetElement; while (element && window.getComputedStyle(element).pointerEvents === 'none') element = element.parentElement; if (!element) return { status: 'notconnected' }; - const result = await this.poll('raf', timeout, (): 'notconnected' | boolean => { + 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; }); - return { status: result === 'notconnected' ? 'notconnected' : (result ? 'success' : 'timeout') }; + if (result === 'notconnected') + return { status: 'notconnected' }; + if (result === 'moved') + return { status: 'error', error: 'Element has moved during the action' }; + return { status: result ? 'success' : 'timeout' }; } private _parentElementOrShadowHost(element: Element): Element | undefined { diff --git a/test/click.spec.js b/test/click.spec.js index f41a2c4ab6..8f97903da7 100644 --- a/test/click.spec.js +++ b/test/click.spec.js @@ -506,55 +506,6 @@ describe('Page.click', function() { await page.click('button'); expect(await page.evaluate('window.clicked')).toBe(true); }); - it('should fail to click a button animated via CSS animations and setInterval', async({page}) => { - // This test has a setInterval that consistently animates a button. - const buttonSize = 10; - const containerWidth = 500; - const transition = 100; - await page.setContent(` - - -
- -
- - - - `); - await page.evaluate(transition => { - window.setInterval(animateLeft, transition); - animateLeft(); - }, transition); - - // Ideally, we we detect the button to be continuously animating, and timeout waiting for it to stop. - // That does not happen though: - // - Chromium headless does not issue rafs between first and second animateLeft() calls. - // - Chromium and WebKit keep element bounds the same when for 2 frames when changing left to a new value. - // This test currently documents our flaky behavior, because it's unclear whether we could - // guarantee timeout. - const error1 = await page.click('button', { timeout: 250 }).catch(e => e); - if (error1) - expect(error1.message).toContain('timeout exceeded'); - const error2 = await page.click('button', { timeout: 250 }).catch(e => e); - if (error2) - expect(error2.message).toContain('timeout exceeded'); - }); it('should report nice error when element is detached and force-clicked', async({page, server}) => { await page.goto(server.PREFIX + '/input/animating-button.html'); await page.evaluate(() => addButton());