From f3fd3ebc3755741edd0953bd59f9e29e5d697459 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 5 Nov 2021 15:36:01 -0800 Subject: [PATCH] chore(frame-selector): add more tests, use frame logic in element handle (#10097) --- packages/playwright-core/src/server/dom.ts | 51 ++++----- packages/playwright-core/src/server/frames.ts | 101 ++++++++++++------ tests/page/page-wait-for-selector-1.spec.ts | 3 +- tests/page/page-wait-for-selector-2.spec.ts | 20 ++++ tests/page/selectors-frame.spec.ts | 72 +++++++++++++ 5 files changed, 184 insertions(+), 63 deletions(-) diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index f5b10464ad..0f44f4da52 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -177,8 +177,12 @@ export class ElementHandle extends js.JSHandle { return null; } + async isIframeElement(): Promise { + return this.evaluateInUtility(([injected, node]) => node && (node.nodeName === 'IFRAME' || node.nodeName === 'FRAME'), {}); + } + async contentFrame(): Promise { - const isFrameElement = throwRetargetableDOMError(await this.evaluateInUtility(([injected, node]) => node && (node.nodeName === 'IFRAME' || node.nodeName === 'FRAME'), {})); + const isFrameElement = throwRetargetableDOMError(await this.isIframeElement()); if (!isFrameElement) return null; return this._page._delegate.getContentFrame(this); @@ -692,21 +696,27 @@ export class ElementHandle extends js.JSHandle { } async querySelector(selector: string, options: types.StrictOptions): Promise { - const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, options, this); + const pair = await this._frame.resolveFrameForSelectorNoWait(selector, options, this); + if (!pair) + return null; + const { frame, info } = pair; // 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[]> { - const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, {}, this); + const pair = await this._frame.resolveFrameForSelectorNoWait(selector, {}, this); + if (!pair) + return []; + const { frame, info } = pair; // 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 { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, { strict }, this); + const pair = 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); + const handle = pair ? await this._page.selectors.query(pair.frame, pair.info, this._frame === pair.frame ? this : undefined) : null; if (!handle) throw new Error(`Error: failed to find element matching selector "${selector}"`); const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); @@ -715,7 +725,10 @@ export class ElementHandle extends js.JSHandle { } async evalOnSelectorAllAndWaitForSignals(selector: string, expression: string, isFunction: boolean | undefined, arg: any): Promise { - const { frame, info } = await this._frame.resolveFrameForSelectorNoWait(selector, {}, this); + const pair = await this._frame.resolveFrameForSelectorNoWait(selector, {}, this); + if (!pair) + throw new Error(`Error: failed to find frame for selector "${selector}"`); + const { frame, info } = pair; // 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); @@ -767,31 +780,7 @@ export class ElementHandle extends js.JSHandle { } async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions = {}): Promise | null> { - const { state = 'visible' } = options; - if (!['attached', 'detached', 'visible', 'hidden'].includes(state)) - throw new Error(`state: expected one of (attached|detached|visible|hidden)`); - const controller = new ProgressController(metadata, this); - return controller.run(async progress => { - progress.log(`waiting for selector "${selector}"${state === 'attached' ? '' : ' to be ' + state}`); - 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)); + return this._frame.waitForSelector(metadata, selector, options, this); } async _adoptTo(context: FrameExecutionContext): Promise> { diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 80d752b9a1..b54eb3b6e5 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -712,11 +712,13 @@ export class Frame extends SdkObject { async querySelector(selector: string, options: types.StrictOptions): Promise | null> { debugLogger.log('api', ` finding element using the selector "${selector}"`); - const { frame, info } = await this.resolveFrameForSelectorNoWait(selector, options); - return this._page.selectors.query(frame, info); + const result = await this.resolveFrameForSelectorNoWait(selector, options); + if (!result) + return null; + return this._page.selectors.query(result.frame, result.info); } - async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions & { omitReturnValue?: boolean } = {}): Promise | null> { + async waitForSelector(metadata: CallMetadata, selector: string, options: types.WaitForElementOptions & { omitReturnValue?: boolean }, scope?: dom.ElementHandle): Promise | null> { const controller = new ProgressController(metadata, this); if ((options as any).visibility) throw new Error('options.visibility is not supported, did you mean options.state?'); @@ -728,8 +730,11 @@ export class Frame extends SdkObject { return controller.run(async progress => { progress.log(`waiting for selector "${selector}"${state === 'attached' ? '' : ' to be ' + state}`); 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); + // Be careful, |this| can be different from |frame|. + const actualScope = this === frame ? scope : undefined; + const task = dom.waitForSelectorTask(info, state, options.omitReturnValue, actualScope); + const result = actualScope ? await frame._runWaitForSelectorTaskOnce(progress, stringifySelector(info.parsed), info.world, task) + : await frame._scheduleRerunnableHandleTask(progress, info.world, task); if (!result.asElement()) { result.dispose(); return null; @@ -742,7 +747,7 @@ export class Frame extends SdkObject { } catch (e) { return continuePolling; } - }); + }, scope); }, this._page._timeoutSettings.timeout(options)); } @@ -754,8 +759,8 @@ export class Frame extends SdkObject { } 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); + const pair = await this.resolveFrameForSelectorNoWait(selector, { strict }); + const handle = pair ? await this._page.selectors.query(pair.frame, pair.info) : null; if (!handle) throw new Error(`Error: failed to find element matching selector "${selector}"`); const result = await handle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); @@ -764,16 +769,20 @@ export class Frame extends SdkObject { } 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 pair = await this.resolveFrameForSelectorNoWait(selector, {}); + if (!pair) + throw new Error(`Error: failed to find frame for selector "${selector}"`); + const arrayHandle = await this._page.selectors._queryArray(pair.frame, pair.info); const result = await arrayHandle.evaluateExpressionAndWaitForSignals(expression, isFunction, true, arg); arrayHandle.dispose(); return result; } async querySelectorAll(selector: string): Promise[]> { - const { frame, info } = await this.resolveFrameForSelectorNoWait(selector, {}); - return this._page.selectors._queryAll(frame, info, undefined, true /* adoptToMain */); + const pair = await this.resolveFrameForSelectorNoWait(selector, {}); + if (!pair) + return []; + return this._page.selectors._queryAll(pair.frame, pair.info, undefined, true /* adoptToMain */); } async content(): Promise { @@ -961,7 +970,13 @@ export class Frame extends SdkObject { scope?: dom.ElementHandle): Promise { const continuePolling = Symbol('continuePolling'); while (progress.isRunning()) { - const { frame, info } = await this.resolveFrameForSelector(progress, selector, options, scope); + const pair = await this._resolveFrameForSelector(progress, selector, options, scope); + if (!pair) { + // Missing content frame. + await new Promise(f => setTimeout(f, 100)); + continue; + } + const { frame, info } = pair; try { const result = await action(frame, info, continuePolling); if (result === continuePolling) @@ -987,6 +1002,7 @@ export class Frame extends SdkObject { strict: boolean | undefined, action: (handle: dom.ElementHandle) => Promise): Promise { return this.retryWithProgress(progress, selector, { strict }, async (frame, info, continuePolling) => { + // Be careful, |this| can be different from |frame|. progress.log(`waiting for selector "${selector}"`); const task = dom.waitForSelectorTask(info, 'attached'); const handle = await frame._scheduleRerunnableHandleTask(progress, info.world, task); @@ -1106,8 +1122,10 @@ export class Frame extends SdkObject { const controller = new ProgressController(metadata, this); return controller.run(async progress => { progress.log(` checking visibility of "${selector}"`); - const { frame, info } = await this.resolveFrameForSelector(progress, selector, options); - const element = await this._page.selectors.query(frame, info); + const pair = await this.resolveFrameForSelectorNoWait(selector, options); + if (!pair) + return false; + const element = await this._page.selectors.query(pair.frame, pair.info); return element ? await element.isVisible() : false; }, this._page._timeoutSettings.timeout({})); } @@ -1309,6 +1327,7 @@ export class Frame extends SdkObject { return controller.run(async progress => { return this.retryWithProgress(progress, selector, options, async (frame, info) => { + // Be careful, |this| can be different from |frame|. progress.log(`waiting for selector "${selector}"`); return await frame._scheduleRerunnableTaskInFrame(progress, info, callbackText, taskData, options); }); @@ -1442,7 +1461,7 @@ export class Frame extends SdkObject { }, { source, arg }); } - async resolveFrameForSelector(progress: Progress, selector: string, options: types.StrictOptions & types.TimeoutOptions, scope?: dom.ElementHandle): Promise<{ frame: Frame, info: SelectorInfo }> { + private async _resolveFrameForSelector(progress: Progress, selector: string, options: types.StrictOptions & types.TimeoutOptions, scope?: dom.ElementHandle): Promise<{ frame: Frame, info: SelectorInfo } | null> { const elementPath: dom.ElementHandle[] = []; progress.cleanupWhenAborted(() => { // Do not await here to avoid being blocked, either by stalled @@ -1457,8 +1476,31 @@ export class Frame extends SdkObject { 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', false, i === 0 ? scope : undefined); - const handle = await frame._scheduleRerunnableHandleTask(progress, info.world, task); + const handle = i === 0 && scope ? await frame._runWaitForSelectorTaskOnce(progress, stringifySelector(info.parsed), info.world, task) + : await frame._scheduleRerunnableHandleTask(progress, info.world, task); const element = handle.asElement() as dom.ElementHandle; + const isIframe = await element.isIframeElement(); + if (isIframe === 'error:notconnected') + return null; // retry + if (!isIframe) + throw new Error(`Selector "${stringifySelector(info.parsed)}" resolved to ${element.preview()},