From 95a7a757f041def08855ad247093975476f0c1a3 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 24 Oct 2024 16:57:16 +0200 Subject: [PATCH] fix(debug controller): highlight selectors in iframe --- .../src/server/injected/injectedScript.ts | 19 +++++++++--- .../src/server/injected/recorder/recorder.ts | 30 +++++++++++++------ .../playwright-core/src/server/recorder.ts | 6 ++-- tests/library/debug-controller.spec.ts | 19 ++++++++++++ 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 6446d16966..209a74e3bb 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -19,7 +19,7 @@ import { XPathEngine } from './xpathSelectorEngine'; import { ReactEngine } from './reactSelectorEngine'; import { VueEngine } from './vueSelectorEngine'; import { createRoleEngine } from './roleSelectorEngine'; -import { parseAttributeSelector } from '../../utils/isomorphic/selectorParser'; +import { parseAttributeSelector, splitSelectorByFrame } from '../../utils/isomorphic/selectorParser'; import type { NestedSelectorBody, ParsedSelector, ParsedSelectorPart } from '../../utils/isomorphic/selectorParser'; import { visitAllSelectorParts, parseSelector, stringifySelector } from '../../utils/isomorphic/selectorParser'; import { type TextMatcher, elementMatchesText, elementText, type ElementText, getElementLabels } from './selectorUtils'; @@ -166,12 +166,23 @@ export class InjectedScript { return this._testIdAttributeNameForStrictErrorAndConsoleCodegen; } - parseSelector(selector: string): ParsedSelector { - const result = parseSelector(selector); - visitAllSelectorParts(result, part => { + private _checkForUnknownEngine(parsed: ParsedSelector, selector: string) { + visitAllSelectorParts(parsed, part => { if (!this._engines.has(part.name)) throw this.createStacklessError(`Unknown engine "${part.name}" while parsing selector ${selector}`); }); + } + + parseSelector(selector: string): ParsedSelector { + const result = parseSelector(selector); + this._checkForUnknownEngine(result, selector); + return result; + } + + splitSelectorByFrame(selector: string): ParsedSelector[] { + const result = splitSelectorByFrame(selector); + for (const part of result) + this._checkForUnknownEngine(part, selector); return result; } diff --git a/packages/playwright-core/src/server/injected/recorder/recorder.ts b/packages/playwright-core/src/server/injected/recorder/recorder.ts index 1d50495c77..fa436736b9 100644 --- a/packages/playwright-core/src/server/injected/recorder/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder/recorder.ts @@ -21,6 +21,7 @@ import type { ElementInfo, Mode, OverlayState, UIState } from '@recorder/recorde import type { ElementText } from '../selectorUtils'; import type { Highlight, HighlightOptions } from '../highlight'; import clipPaths from './clipPaths'; +import { parseSelector, splitSelectorByFrame } from '@isomorphic/selectorParser'; export interface RecorderDelegate { performAction?(action: actions.PerformOnRecordAction): Promise; @@ -1471,18 +1472,29 @@ function removeEventListeners(listeners: (() => void)[]) { listeners.splice(0, listeners.length); } +function frameDepth() { + let depth = 0; + // eslint-disable-next-line no-restricted-globals + let w: Window = window; + while (w.parent !== w) { + w = w.parent; + depth++; + } + return depth; +} + function querySelector(injectedScript: InjectedScript, selector: string, ownerDocument: Document): { selector: string, elements: Element[] } { + const empty = { selector, elements: [] }; try { - const parsedSelector = injectedScript.parseSelector(selector); - return { - selector, - elements: injectedScript.querySelectorAll(parsedSelector, ownerDocument) - }; + // selector starts from the root frame, we need to figure out if this frame is the right one + // as a cheap heuristic, we check if it targets the right depth + const parts = injectedScript.splitSelectorByFrame(selector); + const selectorTargetsFrameDepth = parts.length - 1 === frameDepth(); + if (selectorTargetsFrameDepth) + return { selector, elements: injectedScript.querySelectorAll(parts[parts.length - 1], ownerDocument) }; + return empty; } catch (e) { - return { - selector, - elements: [], - }; + return empty; } } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 74ebefee86..d03bbd759a 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -246,8 +246,10 @@ export class Recorder implements InstrumentationListener, IRecorder { } private _refreshOverlay() { - for (const page of this._context.pages()) - page.mainFrame().evaluateExpression('window.__pw_refreshOverlay()').catch(() => {}); + for (const page of this._context.pages()) { + for (const frame of page.frames()) + frame.evaluateExpression('window.__pw_refreshOverlay()').catch(() => {}); + } } async onBeforeCall(sdkObject: SdkObject, metadata: CallMetadata) { diff --git a/tests/library/debug-controller.spec.ts b/tests/library/debug-controller.spec.ts index cdc579fed2..a2785504cd 100644 --- a/tests/library/debug-controller.spec.ts +++ b/tests/library/debug-controller.spec.ts @@ -253,3 +253,22 @@ test('should reset routes before reuse', async ({ server, connectedBrowserFactor await expect(page2).toHaveTitle('console.log test'); await browser2.close(); }); + +test('should highlight inside iframe', async ({ backend, connectedBrowser }, testInfo) => { + testInfo.annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/33146' }); + + const context = await connectedBrowser._newContextForReuse(); + const page = await context.newPage(); + await backend.navigate({ url: `data:text/html,