diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index ded96eec5f..0fb196c4a2 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -19,7 +19,7 @@ import * as injectedScriptSource from '../generated/injectedScriptSource'; import { isSessionClosedError } from './protocolError'; import type { ScreenshotOptions } from './screenshotter'; import type * as frames from './frames'; -import type { InjectedScript, InjectedScriptPoll, LogEntry, HitTargetInterceptionResult, ElementState } from './injected/injectedScript'; +import type { InjectedScript, HitTargetInterceptionResult, ElementState } from './injected/injectedScript'; import type { CallMetadata } from './instrumentation'; import * as js from './javascript'; import type { Page } from './page'; @@ -36,6 +36,7 @@ export type InputFilesItems = { }; type ActionName = 'click' | 'hover' | 'dblclick' | 'tap' | 'move and up' | 'move and down'; +type PerformActionResult = 'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | 'error:optionsnotfound' | { missingState: ElementState } | { hitTargetDescription: string } | 'done'; export class NonRecoverableDOMError extends Error { } @@ -156,19 +157,6 @@ export class ElementHandle extends js.JSHandle { } } - async evaluatePoll(progress: Progress, pageFunction: js.Func1<[js.JSHandle, ElementHandle, Arg], InjectedScriptPoll>, arg: Arg): Promise { - try { - const utility = await this._frame._utilityContext(); - const poll = await utility.evaluateHandle(pageFunction, [await utility.injectedScript(), this, arg]); - const pollHandler = new InjectedScriptPollHandler(progress, poll); - return await pollHandler.finish(); - } catch (e) { - if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e)) - throw e; - return 'error:notconnected'; - } - } - async ownerFrame(): Promise { const frameId = await this._page._delegate.getOwnerFrame(this); if (!frameId) @@ -224,24 +212,16 @@ export class ElementHandle extends js.JSHandle { } async _waitAndScrollIntoViewIfNeeded(progress: Progress, waitForVisible: boolean): Promise { - const timeouts = [0, 50, 100, 250]; - while (progress.isRunning()) { - 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 (!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; - } + const result = await this._retryAction(progress, 'scroll into view', async () => { + progress.log(` waiting for element to be stable`); + const waitResult = await this.evaluateInUtility(async ([injected, node, { waitForVisible }]) => { + return await injected.checkElementStates(node, waitForVisible ? ['visible', 'stable'] : ['stable']); + }, { waitForVisible }); + if (waitResult) + return waitResult; + return await this._scrollRectIntoViewIfNeeded(); + }, {}); + assertDone(throwRetargetableDOMError(result)); } async scrollIntoViewIfNeeded(metadata: CallMetadata, options: types.TimeoutOptions = {}) { @@ -308,23 +288,11 @@ export class ElementHandle extends js.JSHandle { }; } - async _retryPointerAction(progress: Progress, actionName: ActionName, waitForEnabled: boolean, action: (point: types.Point) => Promise, - options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> { + async _retryAction(progress: Progress, actionName: string, action: (retry: number) => Promise, options: { trial?: boolean, force?: boolean }): Promise<'error:notconnected' | 'done'> { let retry = 0; // We progressively wait longer between retries, up to 500ms. const waitTime = [0, 20, 100, 100, 500]; - // By default, we scroll with protocol method to reveal the action point. - // However, that might not work to scroll from under position:sticky elements - // that overlay the target element. To fight this, we cycle through different - // scroll alignments. This works in most scenarios. - const scrollOptions: (ScrollIntoViewOptions | undefined)[] = [ - undefined, - { block: 'end', inline: 'end' }, - { block: 'center', inline: 'center' }, - { block: 'start', inline: 'start' }, - ]; - while (progress.isRunning()) { if (retry) { progress.log(`retrying ${actionName} action${options.trial ? ' (trial run)' : ''}, attempt #${retry}`); @@ -338,8 +306,7 @@ export class ElementHandle extends js.JSHandle { } else { progress.log(`attempting ${actionName} action${options.trial ? ' (trial run)' : ''}`); } - const forceScrollOptions = scrollOptions[retry % scrollOptions.length]; - const result = await this._performPointerAction(progress, actionName, waitForEnabled, action, forceScrollOptions, options); + const result = await action(retry); ++retry; if (result === 'error:notvisible') { if (options.force) @@ -353,16 +320,42 @@ export class ElementHandle extends js.JSHandle { progress.log(' element is outside of the viewport'); continue; } + if (result === 'error:optionsnotfound') { + progress.log(' did not find some options'); + continue; + } if (typeof result === 'object' && 'hitTargetDescription' in result) { progress.log(` ${result.hitTargetDescription} intercepts pointer events`); continue; } + if (typeof result === 'object' && 'missingState' in result) { + progress.log(` element is not ${result.missingState}`); + continue; + } return result; } return 'done'; } - async _performPointerAction(progress: Progress, actionName: ActionName, waitForEnabled: boolean, action: (point: types.Point) => Promise, forceScrollOptions: ScrollIntoViewOptions | undefined, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notvisible' | 'error:notconnected' | 'error:notinviewport' | { hitTargetDescription: string } | 'done'> { + async _retryPointerAction(progress: Progress, actionName: ActionName, waitForEnabled: boolean, action: (point: types.Point) => Promise, + options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> { + return await this._retryAction(progress, actionName, async retry => { + // By default, we scroll with protocol method to reveal the action point. + // However, that might not work to scroll from under position:sticky elements + // that overlay the target element. To fight this, we cycle through different + // scroll alignments. This works in most scenarios. + const scrollOptions: (ScrollIntoViewOptions | undefined)[] = [ + undefined, + { block: 'end', inline: 'end' }, + { block: 'center', inline: 'center' }, + { block: 'start', inline: 'start' }, + ]; + const forceScrollOptions = scrollOptions[retry % scrollOptions.length]; + return await this._performPointerAction(progress, actionName, waitForEnabled, action, forceScrollOptions, options); + }, options); + } + + async _performPointerAction(progress: Progress, actionName: ActionName, waitForEnabled: boolean, action: (point: types.Point) => Promise, forceScrollOptions: ScrollIntoViewOptions | undefined, options: types.PointerActionOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions): Promise { const { force = false, position } = options; const doScrollIntoView = async () => { @@ -386,9 +379,18 @@ export class ElementHandle extends js.JSHandle { if ((options as any).__testHookBeforeStable) await (options as any).__testHookBeforeStable(); - const result = await this._waitForElementStates(progress, waitForEnabled ? ['visible', 'enabled', 'stable'] : ['visible', 'stable'], force); - if (result !== 'done') - return result; + + if (!force) { + const elementStates: ElementState[] = waitForEnabled ? ['visible', 'enabled', 'stable'] : ['visible', 'stable']; + progress.log(` waiting for element to be ${waitForEnabled ? 'visible, enabled and stable' : 'visible and stable'}`); + const result = await this.evaluateInUtility(async ([injected, node, { elementStates }]) => { + return await injected.checkElementStates(node, elementStates); + }, { elementStates }); + if (result) + return result; + progress.log(` element is ${waitForEnabled ? 'visible, enabled and stable' : 'visible and stable'}`); + } + if ((options as any).__testHookAfterStable) await (options as any).__testHookAfterStable(); @@ -407,7 +409,9 @@ export class ElementHandle extends js.JSHandle { await progress.beforeInputAction(this); let hitTargetInterceptionHandle: js.JSHandle | undefined; - if (!options.force) { + if (force) { + progress.log(` forcing action`); + } else { if ((options as any).__testHookBeforeHitTarget) await (options as any).__testHookBeforeHitTarget(); @@ -526,16 +530,28 @@ export class ElementHandle extends js.JSHandle { } async _selectOption(progress: Progress, elements: ElementHandle[], values: types.SelectOption[], options: types.NavigatingActionWaitOptions & types.ForceOptions): Promise { - const optionsToSelect = [...elements, ...values]; - await progress.beforeInputAction(this); - return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { - progress.throwIfAborted(); // Avoid action that has side-effects. - progress.log(' selecting specified option(s)'); - const result = await this.evaluatePoll(progress, ([injected, node, { optionsToSelect, force }]) => { - return injected.waitForElementStatesAndPerformAction(node, ['visible', 'enabled'], force, injected.selectOptions.bind(injected, optionsToSelect)); + let resultingOptions: string[] = []; + await this._retryAction(progress, 'select option', async () => { + await progress.beforeInputAction(this); + if (!options.force) + progress.log(` waiting for element to be visible and enabled`); + const optionsToSelect = [...elements, ...values]; + const result = await this.evaluateInUtility(async ([injected, node, { optionsToSelect, force }]) => { + if (!force) { + const checkResult = await injected.checkElementStates(node, ['visible', 'enabled']); + if (checkResult) + return checkResult; + } + return injected.selectOptions(node, optionsToSelect); }, { optionsToSelect, force: options.force }); + if (Array.isArray(result)) { + progress.log(' selected specified option(s)'); + resultingOptions = result; + return 'done'; + } return result; - }); + }, options); + return resultingOptions; } async fill(metadata: CallMetadata, value: string, options: types.NavigatingActionWaitOptions & types.ForceOptions = {}): Promise { @@ -547,37 +563,49 @@ export class ElementHandle extends js.JSHandle { } async _fill(progress: Progress, value: string, options: types.NavigatingActionWaitOptions & types.ForceOptions): Promise<'error:notconnected' | 'done'> { - progress.log(`elementHandle.fill("${value}")`); - await progress.beforeInputAction(this); - return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { - progress.log(' waiting for element to be visible, enabled and editable'); - const filled = await this.evaluatePoll(progress, ([injected, node, { value, force }]) => { - return injected.waitForElementStatesAndPerformAction(node, ['visible', 'enabled', 'editable'], force, injected.fill.bind(injected, value)); - }, { value, force: options.force }); - progress.throwIfAborted(); // Avoid action that has side-effects. - if (filled === 'error:notconnected') - return filled; - progress.log(' element is visible, enabled and editable'); - if (filled === 'needsinput') { + progress.log(` fill("${value}")`); + return await this._retryAction(progress, 'fill', async () => { + await progress.beforeInputAction(this); + return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => { + if (!options.force) + progress.log(' waiting for element to be visible, enabled and editable'); + const result = await this.evaluateInUtility(async ([injected, node, { value, force }]) => { + if (!force) { + const checkResult = await injected.checkElementStates(node, ['visible', 'enabled', 'editable']); + if (checkResult) + return checkResult; + } + return injected.fill(node, value); + }, { value, force: options.force }); progress.throwIfAborted(); // Avoid action that has side-effects. - if (value) - await this._page.keyboard.insertText(value); - else - await this._page.keyboard.press('Delete'); - } else { - assertDone(filled); - } - return 'done'; - }, 'input'); + if (result === 'needsinput') { + if (value) + await this._page.keyboard.insertText(value); + else + await this._page.keyboard.press('Delete'); + return 'done'; + } else { + return result; + } + }, 'input'); + }, options); } async selectText(metadata: CallMetadata, options: types.TimeoutOptions & types.ForceOptions = {}): Promise { const controller = new ProgressController(metadata, this); return controller.run(async progress => { - progress.throwIfAborted(); // Avoid action that has side-effects. - const result = await this.evaluatePoll(progress, ([injected, node, force]) => { - return injected.waitForElementStatesAndPerformAction(node, ['visible'], force, injected.selectText.bind(injected)); - }, options.force); + const result = await this._retryAction(progress, 'selectText', async () => { + if (!options.force) + progress.log(' waiting for element to be visible'); + return await this.evaluateInUtility(async ([injected, node, { force }]) => { + if (!force) { + const checkResult = await injected.checkElementStates(node, ['visible']); + if (checkResult) + return checkResult; + } + return injected.selectText(node); + }, { force: options.force }); + }, options); assertDone(throwRetargetableDOMError(result)); }, this._page._timeoutSettings.timeout(options)); } @@ -765,10 +793,12 @@ export class ElementHandle extends js.JSHandle { async waitForElementState(metadata: CallMetadata, state: 'visible' | 'hidden' | 'stable' | 'enabled' | 'disabled' | 'editable', options: types.TimeoutOptions = {}): Promise { const controller = new ProgressController(metadata, this); return controller.run(async progress => { - progress.log(` waiting for element to be ${state}`); - const result = await this.evaluatePoll(progress, ([injected, node, state]) => { - return injected.waitForElementStatesAndPerformAction(node, [state], false, () => 'done' as const); - }, state); + const actionName = `wait for ${state}`; + const result = await this._retryAction(progress, actionName, async () => { + return await this.evaluateInUtility(async ([injected, node, state]) => { + return (await injected.checkElementStates(node, [state])) || 'done'; + }, state); + }, {}); assertDone(throwRetargetableDOMError(result)); }, this._page._timeoutSettings.timeout(options)); } @@ -786,18 +816,6 @@ export class ElementHandle extends js.JSHandle { return this; } - 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; - progress.log(` element is ${title}`); - return result; - } - async _checkFrameIsHitTarget(point: types.Point): Promise<{ framePoint: types.Point | undefined } | 'error:notconnected' | { hitTargetDescription: string }> { let frame = this._frame; const data: { frame: frames.Frame, frameElement: ElementHandle | null, pointInFrame: types.Point }[] = []; @@ -835,72 +853,6 @@ export class ElementHandle extends js.JSHandle { } } -// Handles an InjectedScriptPoll running in injected script: -// - streams logs into progress; -// - cancels the poll when progress cancels. -export class InjectedScriptPollHandler { - private _progress: Progress; - private _poll: js.JSHandle> | null; - - constructor(progress: Progress, poll: js.JSHandle>) { - this._progress = progress; - this._poll = poll; - // Ensure we cancel the poll before progress aborts and returns: - // - no unnecessary work in the page; - // - no possible side effects after progress promise rejects. - this._progress.cleanupWhenAborted(() => this.cancel()); - this._streamLogs(); - } - - private async _streamLogs() { - while (this._poll && this._progress.isRunning()) { - const log = await this._poll.evaluate(poll => poll.takeNextLogs()).catch(e => [] as LogEntry[]); - if (!this._poll || !this._progress.isRunning()) - return; - for (const entry of log) - this._progress.logEntry(entry); - } - } - - async finishHandle(): Promise> { - try { - const result = await this._poll!.evaluateHandle(poll => poll.run()); - await this._finishInternal(); - return result; - } finally { - await this.cancel(); - } - } - - async finish(): Promise { - try { - const result = await this._poll!.evaluate(poll => poll.run()); - await this._finishInternal(); - return result; - } finally { - await this.cancel(); - } - } - - private async _finishInternal() { - if (!this._poll) - return; - // Retrieve all the logs before continuing. - const log = await this._poll.evaluate(poll => poll.takeLastLogs()).catch(e => [] as LogEntry[]); - for (const entry of log) - this._progress.logEntry(entry); - } - - async cancel() { - if (!this._poll) - return; - const copy = this._poll; - this._poll = null; - await copy.evaluate(p => p.cancel()).catch(e => {}); - copy.dispose(); - } -} - export function throwRetargetableDOMError(result: T | 'error:notconnected'): T { if (result === 'error:notconnected') throw new Error('Element is not attached to the DOM'); @@ -938,12 +890,4 @@ function compensateHalfIntegerRoundingError(point: types.Point) { point.y -= 0.02; } -export type SchedulableTask = (injectedScript: js.JSHandle) => Promise>>; - -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/frames.ts b/packages/playwright-core/src/server/frames.ts index 6631da6d69..8b14e6baa5 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -34,7 +34,7 @@ import { ManualPromise } from '../utils/manualPromise'; import { debugLogger } from '../common/debugLogger'; import type { CallMetadata } from './instrumentation'; import { serverSideCallMetadata, SdkObject } from './instrumentation'; -import type { InjectedScript, ElementStateWithoutStable, FrameExpectParams, InjectedScriptPoll, InjectedScriptProgress } from './injected/injectedScript'; +import type { InjectedScript, ElementStateWithoutStable, FrameExpectParams } from './injected/injectedScript'; import { isSessionClosedError } from './protocolError'; import { type ParsedSelector, isInvalidSelectorError } from '../utils/isomorphic/selectorParser'; import type { ScreenshotOptions } from './screenshotter'; @@ -80,8 +80,6 @@ export type NavigationEvent = { isPublic?: boolean; }; -export type SchedulableTask = (injectedScript: js.JSHandle) => Promise>>; -export type DomTaskBody = (progress: InjectedScriptProgress, element: E, data: T, elements: Element[]) => R | symbol; type ElementCallback = (injected: InjectedScript, element: Element, data: T) => R; export class NavigationAbortedError extends Error { diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 67852c541f..3cdce8fc41 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -35,31 +35,8 @@ import { asLocator } from '../../utils/isomorphic/locatorGenerators'; import type { Language } from '../../utils/isomorphic/locatorGenerators'; import { normalizeWhiteSpace, trimStringWithEllipsis } from '../../utils/isomorphic/stringUtils'; -type Predicate = (progress: InjectedScriptProgress) => T | symbol; - -export type InjectedScriptProgress = { - injectedScript: InjectedScript; - continuePolling: symbol; - aborted: boolean; - log: (message: string) => void; - logRepeating: (message: string) => void; -}; - -export type LogEntry = { - message?: string; -}; - export type FrameExpectParams = Omit & { expectedValue?: any }; -export type InjectedScriptPoll = { - run: () => Promise, - // Takes more logs, waiting until at least one message is available. - takeNextLogs: () => Promise, - // Takes all current logs without waiting. - takeLastLogs: () => LogEntry[], - cancel: () => void, -}; - export type ElementStateWithoutStable = 'visible' | 'hidden' | 'enabled' | 'disabled' | 'editable' | 'checked' | 'unchecked'; export type ElementState = ElementStateWithoutStable | 'stable'; @@ -449,92 +426,6 @@ export class InjectedScript { }); } - pollRaf(predicate: Predicate): InjectedScriptPoll { - return this.poll(predicate, next => requestAnimationFrame(next)); - } - - private poll(predicate: Predicate, scheduleNext: (next: () => void) => void): InjectedScriptPoll { - return this._runAbortableTask(progress => { - let fulfill: (result: T) => void; - let reject: (error: Error) => void; - const result = new Promise((f, r) => { fulfill = f; reject = r; }); - - const next = () => { - if (progress.aborted) - return; - try { - const success = predicate(progress); - if (success !== progress.continuePolling) - fulfill(success as T); - else - scheduleNext(next); - } catch (e) { - progress.log(' ' + e.message); - reject(e); - } - }; - - next(); - return result; - }); - } - - private _runAbortableTask(task: (progess: InjectedScriptProgress) => Promise): InjectedScriptPoll { - let unsentLog: LogEntry[] = []; - let takeNextLogsCallback: ((logs: LogEntry[]) => void) | undefined; - let taskFinished = false; - const logReady = () => { - if (!takeNextLogsCallback) - return; - takeNextLogsCallback(unsentLog); - unsentLog = []; - takeNextLogsCallback = undefined; - }; - - const takeNextLogs = () => new Promise(fulfill => { - takeNextLogsCallback = fulfill; - if (unsentLog.length || taskFinished) - logReady(); - }); - - let lastMessage = ''; - const progress: InjectedScriptProgress = { - injectedScript: this, - aborted: false, - continuePolling: Symbol('continuePolling'), - log: (message: string) => { - lastMessage = message; - unsentLog.push({ message }); - logReady(); - }, - logRepeating: (message: string) => { - if (message !== lastMessage) - progress.log(message); - }, - }; - - const run = () => { - const result = task(progress); - - // After the task has finished, there should be no more logs. - // Release any pending `takeNextLogs` call, and do not block any future ones. - // This prevents non-finished protocol evaluation calls and memory leaks. - result.finally(() => { - taskFinished = true; - logReady(); - }); - - return result; - }; - - return { - takeNextLogs, - run, - cancel: () => { progress.aborted = true; }, - takeLastLogs: () => unsentLog, - }; - } - getElementBorderWidth(node: Node): { left: number; top: number; } { if (node.nodeType !== Node.ELEMENT_NODE || !node.ownerDocument || !node.ownerDocument.defaultView) return { left: 0, top: 0 }; @@ -581,65 +472,73 @@ export class InjectedScript { return element; } - waitForElementStatesAndPerformAction(node: Node, states: ElementState[], force: boolean | undefined, - callback: (node: Node, progress: InjectedScriptProgress) => T | symbol): InjectedScriptPoll { + async checkElementStates(node: Node, states: ElementState[]): Promise<'error:notconnected' | { missingState: ElementState } | undefined> { + if (states.includes('stable')) { + const stableResult = await this._checkElementIsStable(node); + if (stableResult === false) + return { missingState: 'stable' }; + if (stableResult === 'error:notconnected') + return stableResult; + } + for (const state of states) { + if (state !== 'stable') { + const result = this.elementState(node, state); + if (result === false) + return { missingState: state }; + if (result === 'error:notconnected') + return result; + } + } + } + + private async _checkElementIsStable(node: Node): Promise<'error:notconnected' | boolean> { + const continuePolling = Symbol('continuePolling'); let lastRect: { x: number, y: number, width: number, height: number } | undefined; - let counter = 0; - let samePositionCounter = 0; + let stableRafCounter = 0; let lastTime = 0; - return this.pollRaf(progress => { - if (force) { - progress.log(` forcing action`); - return callback(node, progress); + const check = () => { + const element = this.retarget(node, 'no-follow-label'); + if (!element) + return 'error:notconnected'; + + // Drop frames that are shorter than 16ms - WebKit Win bug. + const time = performance.now(); + if (this._stableRafCount > 1 && time - lastTime < 15) + return continuePolling; + lastTime = time; + + const clientRect = element.getBoundingClientRect(); + const rect = { x: clientRect.top, y: clientRect.left, width: clientRect.width, height: clientRect.height }; + if (lastRect) { + const samePosition = rect.x === lastRect.x && rect.y === lastRect.y && rect.width === lastRect.width && rect.height === lastRect.height; + if (!samePosition) + return false; + if (++stableRafCounter >= this._stableRafCount) + return true; } + lastRect = rect; + return continuePolling; + }; - for (const state of states) { - if (state !== 'stable') { - const result = this.elementState(node, state); - if (typeof result !== 'boolean') - return result; - if (!result) { - progress.logRepeating(` element is not ${state} - waiting...`); - return progress.continuePolling; - } - continue; - } + let fulfill: (result: 'error:notconnected' | boolean) => void; + let reject: (error: Error) => void; + const result = new Promise<'error:notconnected' | boolean>((f, r) => { fulfill = f; reject = r; }); - const element = this.retarget(node, 'no-follow-label'); - if (!element) - return 'error:notconnected'; - - // First raf happens in the same animation frame as evaluation, so it does not produce - // any client rect difference compared to synchronous call. We skip the synchronous call - // and only force layout during actual rafs as a small optimisation. - if (++counter === 1) - return progress.continuePolling; - - // Drop frames that are shorter than 16ms - WebKit Win bug. - const time = performance.now(); - if (this._stableRafCount > 1 && time - lastTime < 15) - return progress.continuePolling; - lastTime = time; - - const clientRect = element.getBoundingClientRect(); - const rect = { x: clientRect.top, y: clientRect.left, width: clientRect.width, height: clientRect.height }; - const samePosition = lastRect && rect.x === lastRect.x && rect.y === lastRect.y && rect.width === lastRect.width && rect.height === lastRect.height; - if (samePosition) - ++samePositionCounter; + const raf = () => { + try { + const success = check(); + if (success !== continuePolling) + fulfill(success); else - samePositionCounter = 0; - const isStable = samePositionCounter >= this._stableRafCount; - const isStableForLogs = isStable || !lastRect; - lastRect = rect; - if (!isStableForLogs) - progress.logRepeating(` element is not stable - waiting...`); - if (!isStable) - return progress.continuePolling; + requestAnimationFrame(raf); + } catch (e) { + reject(e); } + }; + requestAnimationFrame(raf); - return callback(node, progress); - }); + return result; } elementState(node: Node, state: ElementStateWithoutStable): boolean | 'error:notconnected' { @@ -675,9 +574,7 @@ export class InjectedScript { throw this.createStacklessError(`Unexpected element state "${state}"`); } - selectOptions(optionsToSelect: (Node | { valueOrLabel?: string, value?: string, label?: string, index?: number })[], - node: Node, progress: InjectedScriptProgress): string[] | 'error:notconnected' | symbol { - + selectOptions(node: Node, optionsToSelect: (Node | { valueOrLabel?: string, value?: string, label?: string, index?: number })[]): string[] | 'error:notconnected' | 'error:optionsnotfound' { const element = this.retarget(node, 'follow-label'); if (!element) return 'error:notconnected'; @@ -713,19 +610,16 @@ export class InjectedScript { break; } } - if (remainingOptionsToSelect.length) { - progress.logRepeating(' did not find some options - waiting... '); - return progress.continuePolling; - } + if (remainingOptionsToSelect.length) + return 'error:optionsnotfound'; select.value = undefined as any; selectedOptions.forEach(option => option.selected = true); - progress.log(' selected specified option(s)'); select.dispatchEvent(new Event('input', { bubbles: true, composed: true })); select.dispatchEvent(new Event('change', { bubbles: true })); return selectedOptions.map(option => option.value); } - fill(value: string, node: Node, progress: InjectedScriptProgress): 'error:notconnected' | 'needsinput' | 'done' { + fill(node: Node, value: string): 'error:notconnected' | 'needsinput' | 'done' { const element = this.retarget(node, 'follow-label'); if (!element) return 'error:notconnected'; @@ -734,10 +628,8 @@ export class InjectedScript { const type = input.type.toLowerCase(); const kInputTypesToSetValue = new Set(['color', 'date', 'time', 'datetime-local', 'month', 'range', 'week']); const kInputTypesToTypeInto = new Set(['', 'email', 'number', 'password', 'search', 'tel', 'text', 'url']); - if (!kInputTypesToTypeInto.has(type) && !kInputTypesToSetValue.has(type)) { - progress.log(` input of type "${type}" cannot be filled`); + if (!kInputTypesToTypeInto.has(type) && !kInputTypesToSetValue.has(type)) throw this.createStacklessError(`Input of type "${type}" cannot be filled`); - } if (type === 'number') { value = value.trim(); if (isNaN(Number(value))) diff --git a/packages/playwright-core/src/server/progress.ts b/packages/playwright-core/src/server/progress.ts index 094230a7c0..6f99322935 100644 --- a/packages/playwright-core/src/server/progress.ts +++ b/packages/playwright-core/src/server/progress.ts @@ -20,11 +20,9 @@ import type { LogName } from '../common/debugLogger'; import type { CallMetadata, Instrumentation, SdkObject } from './instrumentation'; import type { ElementHandle } from './dom'; import { ManualPromise } from '../utils/manualPromise'; -import type { LogEntry } from './injected/injectedScript'; export interface Progress { log(message: string): void; - logEntry(entry: LogEntry): void; timeUntilDeadline(): number; isRunning(): boolean; cleanupWhenAborted(cleanup: () => any): void; @@ -74,16 +72,10 @@ export class ProgressController { const progress: Progress = { log: message => { - progress.logEntry({ message }); - }, - logEntry: entry => { - if ('message' in entry) { - const message = entry.message!; - if (this._state === 'running') - this.metadata.log.push(message); - // Note: we might be sending logs after progress has finished, for example browser logs. - this.instrumentation.onCallLog(this.sdkObject, this.metadata, this._logName, message); - } + if (this._state === 'running') + this.metadata.log.push(message); + // Note: we might be sending logs after progress has finished, for example browser logs. + this.instrumentation.onCallLog(this.sdkObject, this.metadata, this._logName, message); }, timeUntilDeadline: () => this._deadline ? this._deadline - monotonicTime() : 2147483647, // 2^31-1 safe setTimeout in Node. isRunning: () => this._state === 'running', diff --git a/tests/page/elementhandle-scroll-into-view.spec.ts b/tests/page/elementhandle-scroll-into-view.spec.ts index 70705b05da..5fe53d0722 100644 --- a/tests/page/elementhandle-scroll-into-view.spec.ts +++ b/tests/page/elementhandle-scroll-into-view.spec.ts @@ -120,5 +120,6 @@ 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 displayed, retrying in 100ms'); + expect(error.message).toContain('element is not visible'); + expect(error.message).toContain('retrying scroll into view action'); }); diff --git a/tests/page/page-click-timeout-1.spec.ts b/tests/page/page-click-timeout-1.spec.ts index 6856cead27..ad6b1fec77 100644 --- a/tests/page/page-click-timeout-1.spec.ts +++ b/tests/page/page-click-timeout-1.spec.ts @@ -32,5 +32,6 @@ it('should timeout waiting for button to be enabled', async ({ page }) => { const error = await page.click('text=Click target', { timeout: 3000 }).catch(e => e); expect(await page.evaluate('window.__CLICKED')).toBe(undefined); expect(error.message).toContain('page.click: Timeout 3000ms exceeded.'); - expect(error.message).toContain('element is not enabled - waiting'); + expect(error.message).toContain('element is not enabled'); + expect(error.message).toContain('retrying click action'); }); diff --git a/tests/page/page-click-timeout-2.spec.ts b/tests/page/page-click-timeout-2.spec.ts index 7a2f30eaa9..b3e4f12974 100644 --- a/tests/page/page-click-timeout-2.spec.ts +++ b/tests/page/page-click-timeout-2.spec.ts @@ -23,7 +23,8 @@ it('should timeout waiting for display:none to be gone', async ({ page, server } const error = await page.click('button', { timeout: 5000 }).catch(e => e); expect(error.message).toContain('page.click: Timeout 5000ms exceeded.'); expect(error.message).toContain('waiting for element to be visible, enabled and stable'); - expect(error.message).toContain('element is not visible - waiting'); + expect(error.message).toContain('element is not visible'); + expect(error.message).toContain('retrying click action'); }); it('should timeout waiting for visibility:hidden to be gone', async ({ page, server }) => { @@ -32,5 +33,6 @@ it('should timeout waiting for visibility:hidden to be gone', async ({ page, ser const error = await page.click('button', { timeout: 5000 }).catch(e => e); expect(error.message).toContain('page.click: Timeout 5000ms exceeded.'); expect(error.message).toContain('waiting for element to be visible, enabled and stable'); - expect(error.message).toContain('element is not visible - waiting'); + expect(error.message).toContain('element is not visible'); + expect(error.message).toContain('retrying click action'); }); diff --git a/tests/page/page-click-timeout-4.spec.ts b/tests/page/page-click-timeout-4.spec.ts index dcd490ab45..0b167a6344 100644 --- a/tests/page/page-click-timeout-4.spec.ts +++ b/tests/page/page-click-timeout-4.spec.ts @@ -27,7 +27,8 @@ it('should timeout waiting for stable position', async ({ page, server }) => { const error = await button.click({ timeout: 3000 }).catch(e => e); expect(error.message).toContain('elementHandle.click: Timeout 3000ms exceeded.'); expect(error.message).toContain('waiting for element to be visible, enabled and stable'); - expect(error.message).toContain('element is not stable - waiting'); + expect(error.message).toContain('element is not stable'); + expect(error.message).toContain('retrying click action'); }); it('should click for the second time after first timeout', async ({ page, server, mode }) => { diff --git a/tests/page/page-click.spec.ts b/tests/page/page-click.spec.ts index 2c8a32b40d..9059303ba5 100644 --- a/tests/page/page-click.spec.ts +++ b/tests/page/page-click.spec.ts @@ -860,9 +860,12 @@ it('should not hang when frame is detached', async ({ page, server, mode }) => { let resolveDetachPromise; const detachPromise = new Promise(resolve => resolveDetachPromise = resolve); + let firstTime = true; const __testHookBeforeStable = () => { // Detach the frame after "waiting for stable" has started. - + if (!firstTime) + return; + firstTime = false; setTimeout(async () => { await detachFrame(page, 'frame1'); resolveDetachPromise(); diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 1a7774c840..3677231c4e 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -370,7 +370,7 @@ test('should fail to screenshot an element that keeps moving', async ({ runInlin }); expect(result.exitCode).toBe(1); expect(result.output).toContain(`Timeout 2000ms exceeded`); - expect(result.output).toContain(`element is not stable - waiting`); + expect(result.output).toContain(`element is not stable`); expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-actual.png'))).toBe(false); expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-expected.png'))).toBe(false); expect(fs.existsSync(testInfo.outputPath('test-results', 'a-is-a-test', 'is-a-test-1-diff.png'))).toBe(false);