From f19864890fa6d4ecdadb2bba7def2f5bcde6bec5 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 5 Nov 2021 10:06:04 -0800 Subject: [PATCH] feat(iframe): make iframe selectors work w/ element handles (#10063) --- .../src/dispatchers/frameDispatcher.ts | 8 +- .../src/server/common/selectorParser.ts | 2 + packages/playwright-core/src/server/dom.ts | 56 ++++-- .../src/server/firefox/ffPage.ts | 3 +- packages/playwright-core/src/server/frames.ts | 190 ++++++++++-------- .../playwright-core/src/server/selectors.ts | 8 +- .../src/server/webkit/wkPage.ts | 3 +- tests/page/selectors-frame.spec.ts | 114 ++++++++++- 8 files changed, 266 insertions(+), 118 deletions(-) diff --git a/packages/playwright-core/src/dispatchers/frameDispatcher.ts b/packages/playwright-core/src/dispatchers/frameDispatcher.ts index 9c1a73c9a1..914cc7e9c1 100644 --- a/packages/playwright-core/src/dispatchers/frameDispatcher.ts +++ b/packages/playwright-core/src/dispatchers/frameDispatcher.ts @@ -83,19 +83,19 @@ export class FrameDispatcher extends Dispatcher { - return { value: serializeResult(await this._frame.evalOnSelectorAndWaitForSignals(metadata, params.selector, !!params.strict, params.expression, params.isFunction, parseArgument(params.arg))) }; + return { value: serializeResult(await this._frame.evalOnSelectorAndWaitForSignals(params.selector, !!params.strict, params.expression, params.isFunction, parseArgument(params.arg))) }; } async evalOnSelectorAll(params: channels.FrameEvalOnSelectorAllParams, metadata: CallMetadata): Promise { - return { value: serializeResult(await this._frame.evalOnSelectorAllAndWaitForSignals(metadata, params.selector, params.expression, params.isFunction, parseArgument(params.arg))) }; + return { value: serializeResult(await this._frame.evalOnSelectorAllAndWaitForSignals(params.selector, params.expression, params.isFunction, parseArgument(params.arg))) }; } async querySelector(params: channels.FrameQuerySelectorParams, metadata: CallMetadata): Promise { - return { element: ElementHandleDispatcher.fromNullable(this._scope, await this._frame.querySelector(metadata, params.selector, params)) }; + return { element: ElementHandleDispatcher.fromNullable(this._scope, await this._frame.querySelector(params.selector, params)) }; } async querySelectorAll(params: channels.FrameQuerySelectorAllParams, metadata: CallMetadata): Promise { - const elements = await this._frame.querySelectorAll(metadata, params.selector); + const elements = await this._frame.querySelectorAll(params.selector); return { elements: elements.map(e => ElementHandleDispatcher.from(this._scope, e)) }; } diff --git a/packages/playwright-core/src/server/common/selectorParser.ts b/packages/playwright-core/src/server/common/selectorParser.ts index 7d14293203..73ad53c301 100644 --- a/packages/playwright-core/src/server/common/selectorParser.ts +++ b/packages/playwright-core/src/server/common/selectorParser.ts @@ -65,6 +65,8 @@ export function splitSelectorByFrame(selectorText: string): ParsedSelector[] { for (let i = 0; i < selector.parts.length; ++i) { const part = selector.parts[i]; if (part.name === 'content-frame') { + if (!chunk.parts.length) + throw new Error('Selector cannot start with "content-frame", select the iframe first'); result.push(chunk); chunk = { parts: [] }; chunkStartIndex = i + 1; diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index 51fd81841d..f5b10464ad 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -109,10 +109,12 @@ export class ElementHandle extends js.JSHandle { declare readonly _context: FrameExecutionContext; readonly _page: Page; declare readonly _objectId: string; + private _frame: frames.Frame; constructor(context: FrameExecutionContext, objectId: string) { super(context, 'node', undefined, objectId); this._page = context.frame._page; + this._frame = context.frame; this._initializePreview().catch(e => {}); } @@ -127,7 +129,7 @@ export class ElementHandle extends js.JSHandle { async evaluateInUtility(pageFunction: js.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise { try { - const utility = await this._context.frame._utilityContext(); + const utility = await this._frame._utilityContext(); return await utility.evaluate(pageFunction, [await utility.injectedScript(), this, arg]); } catch (e) { if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e)) @@ -138,7 +140,7 @@ export class ElementHandle extends js.JSHandle { async evaluateHandleInUtility(pageFunction: js.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise | 'error:notconnected'> { try { - const utility = await this._context.frame._utilityContext(); + const utility = await this._frame._utilityContext(); return await utility.evaluateHandle(pageFunction, [await utility.injectedScript(), this, arg]); } catch (e) { if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e)) @@ -149,7 +151,7 @@ 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._context.frame._utilityContext(); + 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(); @@ -227,7 +229,7 @@ export class ElementHandle extends js.JSHandle { } async dispatchEvent(type: string, eventInit: Object = {}) { - const main = await this._context.frame._mainContext(); + const main = await this._frame._mainContext(); await this._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { return main.evaluate(([injected, node, { type, eventInit }]) => injected.dispatchEvent(node, type, eventInit), [await main.injectedScript(), this, { type, eventInit }] as const); }); @@ -690,15 +692,21 @@ export class ElementHandle extends js.JSHandle { } async querySelector(selector: string, options: types.StrictOptions): Promise { - return this._page.selectors.query(this._context.frame, selector, options, this); + const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, options, this); + // If we end up in the same frame => use the scope again, line above was noop. + return this._page.selectors.query(frame, info, this._frame === frame ? this : undefined); } async querySelectorAll(selector: string): Promise[]> { - return this._page.selectors._queryAll(this._context.frame, selector, this, true /* adoptToMain */); + const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, {}, this); + // If we end up in the same frame => use the scope again, line above was noop. + return this._page.selectors._queryAll(frame, info, this._frame === frame ? this : undefined, true /* adoptToMain */); } async evalOnSelectorAndWaitForSignals(selector: string, strict: boolean, expression: string, isFunction: boolean | undefined, arg: any): Promise { - const handle = await this._page.selectors.query(this._context.frame, selector, { strict }, this); + const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, { strict }, this); + // If we end up in the same frame => use the scope again, line above was noop. + const handle = await this._page.selectors.query(frame, info, this._frame === frame ? this : undefined); if (!handle) throw new Error(`Error: failed to find element matching selector "${selector}"`); const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); @@ -707,7 +715,9 @@ export class ElementHandle extends js.JSHandle { } async evalOnSelectorAllAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise { - const arrayHandle = await this._page.selectors._queryArray(this._context.frame, selector, this); + const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, {}, this); + // If we end up in the same frame => use the scope again, line above was noop. + const arrayHandle = await this._page.selectors._queryArray(frame, info, this._frame === frame ? this : undefined); const result = await arrayHandle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); arrayHandle.dispose(); return result; @@ -760,21 +770,27 @@ export class ElementHandle extends js.JSHandle { const { state = 'visible' } = options; if (!['attached', 'detached', 'visible', 'hidden'].includes(state)) throw new Error(`state: expected one of (attached|detached|visible|hidden)`); - const info = this._page.parseSelector(selector, options); - const task = waitForSelectorTask(info, state, false, this); const controller = new ProgressController(metadata, this); return controller.run(async progress => { progress.log(`waiting for selector "${selector}"${state === 'attached' ? '' : ' to be ' + state}`); - const context = await this._context.frame._context(info.world); - const injected = await context.injectedScript(); - const pollHandler = new InjectedScriptPollHandler(progress, await task(injected)); - const result = await pollHandler.finishHandle(); - if (!result.asElement()) { - result.dispose(); - return null; - } - const handle = result.asElement() as ElementHandle; - return handle._adoptTo(await this._context.frame._mainContext()); + return this._frame.retryWithProgress(progress, selector, options, async (frame, info, continuePolling) => { + // If we end up in the same frame => use the scope again, line above was noop. + const task = waitForSelectorTask(info, state, false, frame === this._frame ? this : undefined); + const context = await frame._context(info.world); + const injected = await context.injectedScript(); + const pollHandler = new InjectedScriptPollHandler(progress, await task(injected)); + const result = await pollHandler.finishHandle(); + if (!result.asElement()) { + result.dispose(); + return null; + } + const handle = result.asElement() as ElementHandle; + try { + return await handle._adoptTo(await frame._mainContext()); + } catch (e) { + return continuePolling; + } + }, this); }, this._page._timeoutSettings.timeout(options)); } diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts index b4c84a5dec..e1f94d91ed 100644 --- a/packages/playwright-core/src/server/firefox/ffPage.ts +++ b/packages/playwright-core/src/server/firefox/ffPage.ts @@ -555,7 +555,8 @@ export class FFPage implements PageDelegate { const parent = frame.parentFrame(); if (!parent) throw new Error('Frame has been detached.'); - const handles = await this._page.selectors._queryAll(parent, 'frame,iframe', undefined); + const info = this._page.parseSelector('frame,iframe'); + const handles = await this._page.selectors._queryAll(parent, info); const items = await Promise.all(handles.map(async handle => { const frame = await handle.contentFrame().catch(e => null); return { handle, frame }; diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 208aa65023..80d752b9a1 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -458,6 +458,10 @@ export class Frame extends SdkObject { this._subtreeLifecycleEvents.add('commit'); } + isDetached(): boolean { + return this._detached; + } + _onLifecycleEvent(event: types.LifecycleEvent) { if (this._firedLifecycleEvents.has(event)) return; @@ -706,15 +710,10 @@ export class Frame extends SdkObject { return value; } - async querySelector(metadata: CallMetadata, selector: string, options: types.StrictOptions): Promise | null> { + async querySelector(selector: string, options: types.StrictOptions): Promise | null> { debugLogger.log('api', ` finding element using the selector "${selector}"`); - const controller = new ProgressController(metadata, this); - return controller.run(progress => this._innerQuerySelector(progress, selector, options)); - } - - private async _innerQuerySelector(progress: Progress, selector: string, options: types.StrictOptions): Promise | null> { - const { frame, info } = await this._resolveFrame(progress, selector, options); - return this._page.selectors.query(frame, info, options); + const { frame, info } = await this.resolveFrameForSelectorNoWait(selector, options); + return this._page.selectors.query(frame, info); } async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions & { omitReturnValue?: boolean } = {}): Promise | null> { @@ -728,8 +727,7 @@ export class Frame extends SdkObject { throw new Error(`state: expected one of (attached|detached|visible|hidden)`); return controller.run(async progress => { progress.log(`waiting for selector "${selector}"${state === 'attached' ? '' : ' to be ' + state}`); - while (progress.isRunning()) { - const { frame, info } = await this._resolveFrame(progress, selector, options); + return this.retryWithProgress(progress, selector, options, async (frame, info, continuePolling) => { const task = dom.waitForSelectorTask(info, state, options.omitReturnValue); const result = await frame._scheduleRerunnableHandleTask(progress, info.world, task); if (!result.asElement()) { @@ -738,18 +736,13 @@ export class Frame extends SdkObject { } if ((options as any).__testHookBeforeAdoptNode) await (options as any).__testHookBeforeAdoptNode(); + const handle = result.asElement() as dom.ElementHandle; try { - const handle = result.asElement() as dom.ElementHandle; - const adopted = await handle._adoptTo(await this._mainContext()); - return adopted; + return await handle._adoptTo(await frame._mainContext()); } catch (e) { - // Navigated while trying to adopt the node. - if (js.isJavaScriptErrorInEvaluate(e)) - throw e; - result.dispose(); + return continuePolling; } - } - return null; + }); }, this._page._timeoutSettings.timeout(options)); } @@ -760,35 +753,27 @@ export class Frame extends SdkObject { await this._page._doSlowMo(); } - async evalOnSelectorAndWaitForSignals(metadata: CallMetadata, selector: string, strict: boolean, expression: string, isFunction: boolean | undefined, arg: any): Promise { - const controller = new ProgressController(metadata, this); - return controller.run(async progress => { - const handle = await this._innerQuerySelector(progress, selector, { strict }); - if (!handle) - throw new Error(`Error: failed to find element matching selector "${selector}"`); - const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); - handle.dispose(); - return result; - }); + async evalOnSelectorAndWaitForSignals(selector: string, strict: boolean, expression: string, isFunction: boolean | undefined, arg: any): Promise { + const { frame, info } = await this.resolveFrameForSelectorNoWait(selector, { strict }); + const handle = await this._page.selectors.query(frame, info); + if (!handle) + throw new Error(`Error: failed to find element matching selector "${selector}"`); + const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); + handle.dispose(); + return result; } - async evalOnSelectorAllAndWaitForSignals(metadata: CallMetadata, selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise { - const controller = new ProgressController(metadata, this); - return controller.run(async progress => { - const { frame, info } = await this._resolveFrame(progress, selector, {}); - const arrayHandle = await this._page.selectors._queryArray(frame, info); - const result = await arrayHandle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); - arrayHandle.dispose(); - return result; - }); + async evalOnSelectorAllAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise { + const { frame, info } = await this.resolveFrameForSelectorNoWait(selector, {}); + const arrayHandle = await this._page.selectors._queryArray(frame, info); + const result = await arrayHandle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); + arrayHandle.dispose(); + return result; } - async querySelectorAll(metadata: CallMetadata, selector: string): Promise[]> { - const controller = new ProgressController(metadata, this); - return controller.run(async progress => { - const { frame, info } = await this._resolveFrame(progress, selector, {}); - return this._page.selectors._queryAll(frame, info, undefined, true /* adoptToMain */); - }); + async querySelectorAll(selector: string): Promise[]> { + const { frame, info } = await this.resolveFrameForSelectorNoWait(selector, {}); + return this._page.selectors._queryAll(frame, info, undefined, true /* adoptToMain */); } async content(): Promise { @@ -968,31 +953,55 @@ export class Frame extends SdkObject { return result!; } + async retryWithProgress( + progress: Progress, + selector: string, + options: types.StrictOptions & types.TimeoutOptions, + action: (frame: Frame, info: SelectorInfo, continuePolling: symbol) => Promise, + scope?: dom.ElementHandle): Promise { + const continuePolling = Symbol('continuePolling'); + while (progress.isRunning()) { + const { frame, info } = await this.resolveFrameForSelector(progress, selector, options, scope); + try { + const result = await action(frame, info, continuePolling); + if (result === continuePolling) + continue; + return result as R; + } catch (e) { + // Always fail on JavaScript errors or when the main connection is closed. + if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e)) + throw e; + // If error has happened in the detached inner frame, ignore it, keep polling. + if (frame !== this && frame.isDetached()) + continue; + throw e; + } + } + progress.throwIfAborted(); + return undefined as any; + } + private async _retryWithProgressIfNotConnected( progress: Progress, selector: string, strict: boolean | undefined, action: (handle: dom.ElementHandle) => Promise): Promise { - while (progress.isRunning()) { + return this.retryWithProgress(progress, selector, { strict }, async (frame, info, continuePolling) => { progress.log(`waiting for selector "${selector}"`); - const { frame, info } = await this._resolveFrame(progress, selector, { strict }); const task = dom.waitForSelectorTask(info, 'attached'); const handle = await frame._scheduleRerunnableHandleTask(progress, info.world, task); const element = handle.asElement() as dom.ElementHandle; - progress.cleanupWhenAborted(() => { - // Do not await here to avoid being blocked, either by stalled - // page (e.g. alert) or unresolved navigation in Chromium. - element.dispose(); - }); - const result = await action(element); - element.dispose(); - if (result === 'error:notconnected') { - progress.log('element was detached from the DOM, retrying'); - continue; + try { + const result = await action(element); + if (result === 'error:notconnected') { + progress.log('element was detached from the DOM, retrying'); + return continuePolling; + } + return result; + } finally { + element?.dispose(); } - return result; - } - return undefined as any; + }); } async click(metadata: CallMetadata, selector: string, options: types.MouseClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions) { @@ -1097,7 +1106,8 @@ export class Frame extends SdkObject { const controller = new ProgressController(metadata, this); return controller.run(async progress => { progress.log(` checking visibility of "${selector}"`); - const element = await this._innerQuerySelector(progress, selector, options); + const { frame, info } = await this.resolveFrameForSelector(progress, selector, options); + const element = await this._page.selectors.query(frame, info); return element ? await element.isVisible() : false; }, this._page._timeoutSettings.timeout({})); } @@ -1214,10 +1224,10 @@ export class Frame extends SdkObject { // Reached the expected state! return result; }, { ...options, isArray }, { strict: true, querySelectorAll: isArray, mainWorld, omitAttached: true, logScale: true, ...options }).catch(e => { - if (js.isJavaScriptErrorInEvaluate(e)) - throw e; // Q: Why not throw upon isSessionClosedError(e) as in other places? // A: We want user to receive a friendly message containing the last intermediate result. + if (js.isJavaScriptErrorInEvaluate(e)) + throw e; return { received: controller.lastIntermediateResult(), matches: options.isNot, log: metadata.log }; }); } @@ -1298,18 +1308,10 @@ export class Frame extends SdkObject { const callbackText = body.toString(); return controller.run(async progress => { - while (progress.isRunning()) { + return this.retryWithProgress(progress, selector, options, async (frame, info) => { progress.log(`waiting for selector "${selector}"`); - const { frame, info } = await this._resolveFrame(progress, selector, options); - try { - return await frame._scheduleRerunnableTaskInFrame(progress, info, callbackText, taskData, options); - } catch (e) { - if (js.isJavaScriptErrorInEvaluate(e) || isSessionClosedError(e)) - throw e; - continue; - } - } - return undefined as any; + return await frame._scheduleRerunnableTaskInFrame(progress, info, callbackText, taskData, options); + }); }, this._page._timeoutSettings.timeout(options)); } @@ -1319,10 +1321,9 @@ export class Frame extends SdkObject { callbackText: string, taskData: T, options: types.TimeoutOptions & types.StrictOptions & { mainWorld?: boolean, querySelectorAll?: boolean, logScale?: boolean, omitAttached?: boolean }): Promise { - if (!progress.isRunning()) - progress.throwIfAborted(); + progress.throwIfAborted(); const data = this._contextData.get(options.mainWorld ? 'main' : info!.world)!; - + // This potentially runs in a sub-frame. { const rerunnableTask = new RerunnableTask(data, progress, injectedScript => { return injectedScript.evaluateHandle((injected, { info, taskData, callbackText, querySelectorAll, logScale, omitAttached, snapshotName }) => { @@ -1441,7 +1442,7 @@ export class Frame extends SdkObject { }, { source, arg }); } - private async _resolveFrame(progress: Progress, selector: string, options: types.StrictOptions & types.TimeoutOptions): Promise<{ frame: Frame, info: SelectorInfo }> { + async resolveFrameForSelector(progress: Progress, selector: string, options: types.StrictOptions & types.TimeoutOptions, scope?: dom.ElementHandle): Promise<{ frame: Frame, info: SelectorInfo }> { const elementPath: dom.ElementHandle[] = []; progress.cleanupWhenAborted(() => { // Do not await here to avoid being blocked, either by stalled @@ -1450,20 +1451,35 @@ export class Frame extends SdkObject { element.dispose(); }); - let frame: Frame = this; + let frame: Frame | null = this; const frameChunks = splitSelectorByFrame(selector); for (let i = 0; i < frameChunks.length - 1 && progress.isRunning(); ++i) { const info = this._page.parseSelector(frameChunks[i], options); - const task = dom.waitForSelectorTask(info, 'attached'); + const task = dom.waitForSelectorTask(info, 'attached', false, i === 0 ? scope : undefined); const handle = await frame._scheduleRerunnableHandleTask(progress, info.world, task); const element = handle.asElement() as dom.ElementHandle; - if (i < frameChunks.length - 1) { - frame = (await element.contentFrame())!; - element.dispose(); - if (!frame) - throw new Error(`Selector "${stringifySelector(info.parsed)}" resolved to ${element.preview()},