diff --git a/docs/src/actionability.md b/docs/src/actionability.md index f0f3ca721d..72058ad414 100644 --- a/docs/src/actionability.md +++ b/docs/src/actionability.md @@ -24,7 +24,7 @@ Here is the complete list of actionability checks performed for each action: | tap | Yes | Yes | Yes | Yes | Yes | - | | uncheck | Yes | Yes | Yes | Yes | Yes | - | | hover | Yes | Yes | Yes | Yes | - | - | -| scrollIntoViewIfNeeded | Yes | Yes | Yes | - | - | - | +| scrollIntoViewIfNeeded | Yes | - | Yes | - | - | - | | screenshot | Yes | Yes | Yes | - | - | - | | fill | Yes | Yes | - | - | Yes | Yes | | selectText | Yes | Yes | - | - | - | - | diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index 19d1021770..5b6fa39233 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -20,7 +20,7 @@ import type * as channels from '../protocol/channels'; import { isSessionClosedError } from './protocolError'; import type { ScreenshotOptions } from './screenshotter'; import type * as frames from './frames'; -import type { InjectedScript, InjectedScriptPoll, LogEntry, HitTargetInterceptionResult } from './injected/injectedScript'; +import type { InjectedScript, InjectedScriptPoll, LogEntry, HitTargetInterceptionResult, ElementState } from './injected/injectedScript'; import type { CallMetadata } from './instrumentation'; import * as js from './javascript'; import type { Page } from './page'; @@ -260,14 +260,22 @@ export class ElementHandle extends js.JSHandle { return await this._page._delegate.scrollRectIntoViewIfNeeded(this, rect); } - async _waitAndScrollIntoViewIfNeeded(progress: Progress): Promise { + async _waitAndScrollIntoViewIfNeeded(progress: Progress, waitForVisible: boolean): Promise { + const timeouts = [0, 50, 100, 250]; while (progress.isRunning()) { - assertDone(throwRetargetableDOMError(await this._waitForDisplayedAtStablePosition(progress, false /* force */, false /* waitForEnabled */))); - + assertDone(throwRetargetableDOMError(await this._waitForElementStates(progress, waitForVisible ? ['visible', 'stable'] : ['stable'], false /* force */))); progress.throwIfAborted(); // Avoid action that has side-effects. const result = throwRetargetableDOMError(await this._scrollRectIntoViewIfNeeded()); - if (result === 'error:notvisible') + if (result === 'error:notvisible') { + if (!waitForVisible) { + // Wait for a timeout to avoid retrying too often when not waiting for visible. + // If we wait for visible, this should be covered by _waitForElementStates instead. + const timeout = timeouts.shift() ?? 500; + progress.log(` element is not displayed, retrying in ${timeout}ms`); + await new Promise(f => setTimeout(f, timeout)); + } continue; + } assertDone(result); return; } @@ -276,7 +284,7 @@ export class ElementHandle extends js.JSHandle { async scrollIntoViewIfNeeded(metadata: CallMetadata, options: types.TimeoutOptions = {}) { const controller = new ProgressController(metadata, this); return controller.run( - progress => this._waitAndScrollIntoViewIfNeeded(progress), + progress => this._waitAndScrollIntoViewIfNeeded(progress, false /* waitForVisible */), this._page._timeoutSettings.timeout(options)); } @@ -395,7 +403,7 @@ export class ElementHandle extends js.JSHandle { const { force = false, position } = options; if ((options as any).__testHookBeforeStable) await (options as any).__testHookBeforeStable(); - const result = await this._waitForDisplayedAtStablePosition(progress, force, waitForEnabled); + const result = await this._waitForElementStates(progress, waitForEnabled ? ['visible', 'enabled', 'stable'] : ['visible', 'stable'], force); if (result !== 'done') return result; if ((options as any).__testHookAfterStable) @@ -845,21 +853,15 @@ export class ElementHandle extends js.JSHandle { return this; } - async _waitForDisplayedAtStablePosition(progress: Progress, force: boolean, waitForEnabled: boolean): Promise<'error:notconnected' | 'done'> { - if (waitForEnabled) - progress.log(` waiting for element to be visible, enabled and stable`); - else - progress.log(` waiting for element to be visible and stable`); - const result = await this.evaluatePoll(progress, ([injected, node, { waitForEnabled, force }]) => { - return injected.waitForElementStatesAndPerformAction(node, - waitForEnabled ? ['visible', 'stable', 'enabled'] : ['visible', 'stable'], force, () => 'done' as const); - }, { waitForEnabled, force }); + async _waitForElementStates(progress: Progress, states: ElementState[], force: boolean): Promise<'error:notconnected' | 'done'> { + const title = joinWithAnd(states); + progress.log(` waiting for element to be ${title}`); + const result = await this.evaluatePoll(progress, ([injected, node, { states, force }]) => { + return injected.waitForElementStatesAndPerformAction(node, states, force, () => 'done' as const); + }, { states, force }); if (result === 'error:notconnected') return result; - if (waitForEnabled) - progress.log(' element is visible, enabled and stable'); - else - progress.log(' element is visible and stable'); + progress.log(` element is ${title}`); return result; } @@ -1023,4 +1025,10 @@ export function waitForSelectorTask(selector: SelectorInfo, state: 'attached' | }, { parsed: selector.parsed, strict: selector.strict, state, omitReturnValue, root }); } +function joinWithAnd(strings: string[]): string { + if (strings.length < 1) + return strings.join(', '); + return strings.slice(0, strings.length - 1).join(', ') + ' and ' + strings[strings.length - 1]; +} + export const kUnableToAdoptErrorMessage = 'Unable to adopt element handle from a different document'; diff --git a/packages/playwright-core/src/server/screenshotter.ts b/packages/playwright-core/src/server/screenshotter.ts index 9e117dee16..58792dae8c 100644 --- a/packages/playwright-core/src/server/screenshotter.ts +++ b/packages/playwright-core/src/server/screenshotter.ts @@ -119,7 +119,7 @@ export class Screenshotter { await this._preparePageForScreenshot(progress, options.caret !== 'initial', options.animations === 'disabled', options.fonts === 'ready'); progress.throwIfAborted(); // Do not do extra work. - await handle._waitAndScrollIntoViewIfNeeded(progress); + await handle._waitAndScrollIntoViewIfNeeded(progress, true /* waitForVisible */); progress.throwIfAborted(); // Do not do extra work. const boundingBox = await handle.boundingBox(); diff --git a/tests/page/elementhandle-scroll-into-view.spec.ts b/tests/page/elementhandle-scroll-into-view.spec.ts index 9c53c170ba..20347e03c2 100644 --- a/tests/page/elementhandle-scroll-into-view.spec.ts +++ b/tests/page/elementhandle-scroll-into-view.spec.ts @@ -54,6 +54,7 @@ async function testWaiting(page, after) { await promise; expect(done).toBe(true); } + it('should wait for display:none to become visible', async ({ page, server }) => { await page.setContent('
Hello
'); await testWaiting(page, div => div.style.display = 'block'); @@ -64,14 +65,16 @@ it('should wait for display:contents to become visible', async ({ page, server } await testWaiting(page, div => div.style.display = 'block'); }); -it('should wait for visibility:hidden to become visible', async ({ page, server }) => { +it('should work for visibility:hidden element', async ({ page }) => { await page.setContent('
Hello
'); - await testWaiting(page, div => div.style.visibility = 'visible'); + const div = await page.$('div'); + await div.scrollIntoViewIfNeeded(); }); -it('should wait for zero-sized element to become visible', async ({ page, server }) => { +it('should work for zero-sized element', async ({ page }) => { await page.setContent('
Hello
'); - await testWaiting(page, div => div.style.height = '100px'); + const div = await page.$('div'); + await div.scrollIntoViewIfNeeded(); }); it('should wait for nested display:none to become visible', async ({ page, server }) => { @@ -99,5 +102,5 @@ it('should timeout waiting for visible', async ({ page, server }) => { await page.setContent('
Hello
'); const div = await page.$('div'); const error = await div.scrollIntoViewIfNeeded({ timeout: 3000 }).catch(e => e); - expect(error.message).toContain('element is not visible'); + expect(error.message).toContain('element is not displayed, retrying in 100ms'); }); diff --git a/tests/page/locator-misc-2.spec.ts b/tests/page/locator-misc-2.spec.ts index 5d5bb3cabe..c21b329405 100644 --- a/tests/page/locator-misc-2.spec.ts +++ b/tests/page/locator-misc-2.spec.ts @@ -42,6 +42,34 @@ it('should scroll into view', async ({ page, server, isAndroid }) => { } }); +it('should scroll zero-sized element into view', async ({ page, isAndroid }) => { + it.fixme(isAndroid); + + await page.setContent(` + +
+

SCROLL DOWN

+
+
+ + `); + expect(await page.locator('#lazyload').boundingBox()).toEqual({ x: 0, y: 2020, width: 1280, height: 0 }); + await page.locator('#lazyload').scrollIntoViewIfNeeded(); + await page.evaluate(() => new Promise(requestAnimationFrame)); + expect(await page.locator('#lazyload').textContent()).toBe('LAZY LOADED CONTENT'); + expect(await page.locator('#lazyload').boundingBox()).toEqual({ x: 0, y: 720, width: 1280, height: 20 }); +}); + it('should select textarea', async ({ page, server, browserName }) => { await page.goto(server.PREFIX + '/input/textarea.html'); const textarea = page.locator('textarea');