From bd57ebba24dc7314f80a8b2b727d3940e02e931c Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 25 Oct 2024 12:14:44 +0200 Subject: [PATCH] address feedback --- packages/playwright-core/src/server/frames.ts | 6 ++++ .../src/server/injected/injectedScript.ts | 19 +++--------- .../src/server/injected/recorder/recorder.ts | 29 ++++++------------- .../playwright-core/src/server/recorder.ts | 15 +++++++++- tests/library/debug-controller.spec.ts | 9 +++++- 5 files changed, 41 insertions(+), 37 deletions(-) diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 597ff35951..2bf9c6dd57 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -948,6 +948,12 @@ export class Frame extends SdkObject { return this._parentFrame; } + _depth(): number { + if (!this._parentFrame) + return 0; + return this._parentFrame._depth() + 1; + } + childFrames(): Frame[] { return Array.from(this._childFrames); } diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 209a74e3bb..6446d16966 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, splitSelectorByFrame } from '../../utils/isomorphic/selectorParser'; +import { parseAttributeSelector } 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,23 +166,12 @@ export class InjectedScript { return this._testIdAttributeNameForStrictErrorAndConsoleCodegen; } - private _checkForUnknownEngine(parsed: ParsedSelector, selector: string) { - visitAllSelectorParts(parsed, part => { + parseSelector(selector: string): ParsedSelector { + const result = parseSelector(selector); + visitAllSelectorParts(result, 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 d566952636..1d50495c77 100644 --- a/packages/playwright-core/src/server/injected/recorder/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder/recorder.ts @@ -1471,29 +1471,18 @@ 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 { - // 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; + const parsedSelector = injectedScript.parseSelector(selector); + return { + selector, + elements: injectedScript.querySelectorAll(parsedSelector, ownerDocument) + }; } catch (e) { - return empty; + return { + selector, + elements: [], + }; } } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index d03bbd759a..2e8dc6f160 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -30,6 +30,8 @@ import type { IRecorderAppFactory, IRecorderApp, IRecorder } from './recorder/re import { metadataToCallLog } from './recorder/recorderUtils'; import type * as actions from '@recorder/actions'; import { buildFullSelector } from '../utils/isomorphic/recorderUtils'; +import { splitSelectorByFrame, stringifySelector } from '../utils/isomorphic/selectorParser'; +import type { Frame } from './frames'; const recorderSymbol = Symbol('recorderSymbol'); @@ -149,7 +151,7 @@ export class Recorder implements InstrumentationListener, IRecorder { let actionPoint: Point | undefined; const hasActiveScreenshotCommand = [...this._currentCallsMetadata.keys()].some(isScreenshotCommand); if (!hasActiveScreenshotCommand) { - actionSelector = this._highlightedSelector; + actionSelector = this._scopeHighlightedSelectorToFrame(source.frame); for (const [metadata, sdkObject] of this._currentCallsMetadata) { if (source.page === sdkObject.attribution.page) { actionPoint = metadata.point || actionPoint; @@ -241,6 +243,17 @@ export class Recorder implements InstrumentationListener, IRecorder { this._refreshOverlay(); } + private _scopeHighlightedSelectorToFrame(frame: Frame): string { + if (this._highlightedSelector === '') + return ''; + const parts = splitSelectorByFrame(this._highlightedSelector); + const selectorDepth = parts.length - 1; + const frameDepth = frame._depth(); + if (frameDepth < selectorDepth) + return ''; + return stringifySelector(parts[parts.length - 1]); + } + setOutput(codegenId: string, outputFile: string | undefined) { this._contextRecorder.setOutput(codegenId, outputFile); } diff --git a/tests/library/debug-controller.spec.ts b/tests/library/debug-controller.spec.ts index a2785504cd..b1a93a0138 100644 --- a/tests/library/debug-controller.spec.ts +++ b/tests/library/debug-controller.spec.ts @@ -259,7 +259,7 @@ test('should highlight inside iframe', async ({ backend, connectedBrowser }, tes const context = await connectedBrowser._newContextForReuse(); const page = await context.newPage(); - await backend.navigate({ url: `data:text/html,