diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 526e90e4ad..05ddb154ee 100644 --- a/src/chromium/crPage.ts +++ b/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 { + 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 { - 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; }); } diff --git a/src/dom.ts b/src/dom.ts index 215a9e9615..4ce1e26930 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -171,15 +171,15 @@ export class ElementHandle extends js.JSHandle { injected.dispatchEvent(node, type, eventInit), { type, eventInit }); } - async _scrollRectIntoViewIfNeeded(rect?: types.Rect): Promise { - 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 { + private async _clickablePoint(): Promise { 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 extends js.JSHandle { 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 extends js.JSHandle { return result; } - private async _offsetPoint(offset: types.Point): Promise { + private async _offsetPoint(offset: types.Point): Promise { 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, options: PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}): Promise { @@ -257,11 +253,30 @@ export class ElementHandle extends js.JSHandle { 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) diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 28e889cf56..95b4b3fb3c 100644 --- a/src/firefox/ffPage.ts +++ b/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 { - 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; diff --git a/src/page.ts b/src/page.ts index 048856bd45..eb20e9107f 100644 --- a/src/page.ts +++ b/src/page.ts @@ -68,7 +68,7 @@ export interface PageDelegate { setInputFiles(handle: dom.ElementHandle, files: types.FilePayload[]): Promise; getBoundingBox(handle: dom.ElementHandle): Promise; getFrameElement(frame: frames.Frame): Promise; - scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise; + scrollRectIntoViewIfNeeded(handle: dom.ElementHandle, rect?: types.Rect): Promise<'success' | 'invisible'>; setActivityPaused(paused: boolean): Promise; rafCountForStablePosition(): number; diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 6b12bc41b3..91e9735860 100644 --- a/src/webkit/wkPage.ts +++ b/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 { - 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; }); } diff --git a/test/click.spec.js b/test/click.spec.js index bc282080e5..32b86ffcb1 100644 --- a/test/click.spec.js +++ b/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(` +
+ +
+ `); + 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(` +
+ +
+ `); + 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(''); let clicked = false; diff --git a/test/elementhandle.spec.js b/test/elementhandle.spec.js index eb1a2f00ea..4a2bab10dd 100644 --- a/test/elementhandle.spec.js +++ b/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
elements', async({page, server}) => { + it('should throw for
elements with force', async({page, server}) => { await page.setContent('hello
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'); }); });