address feedback

This commit is contained in:
Simon Knott 2024-10-25 12:14:44 +02:00
parent 51326e91c7
commit bd57ebba24
No known key found for this signature in database
GPG key ID: 8CEDC00028084AEC
5 changed files with 41 additions and 37 deletions

View file

@ -948,6 +948,12 @@ export class Frame extends SdkObject {
return this._parentFrame; return this._parentFrame;
} }
_depth(): number {
if (!this._parentFrame)
return 0;
return this._parentFrame._depth() + 1;
}
childFrames(): Frame[] { childFrames(): Frame[] {
return Array.from(this._childFrames); return Array.from(this._childFrames);
} }

View file

@ -19,7 +19,7 @@ import { XPathEngine } from './xpathSelectorEngine';
import { ReactEngine } from './reactSelectorEngine'; import { ReactEngine } from './reactSelectorEngine';
import { VueEngine } from './vueSelectorEngine'; import { VueEngine } from './vueSelectorEngine';
import { createRoleEngine } from './roleSelectorEngine'; 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 type { NestedSelectorBody, ParsedSelector, ParsedSelectorPart } from '../../utils/isomorphic/selectorParser';
import { visitAllSelectorParts, parseSelector, stringifySelector } from '../../utils/isomorphic/selectorParser'; import { visitAllSelectorParts, parseSelector, stringifySelector } from '../../utils/isomorphic/selectorParser';
import { type TextMatcher, elementMatchesText, elementText, type ElementText, getElementLabels } from './selectorUtils'; import { type TextMatcher, elementMatchesText, elementText, type ElementText, getElementLabels } from './selectorUtils';
@ -166,23 +166,12 @@ export class InjectedScript {
return this._testIdAttributeNameForStrictErrorAndConsoleCodegen; return this._testIdAttributeNameForStrictErrorAndConsoleCodegen;
} }
private _checkForUnknownEngine(parsed: ParsedSelector, selector: string) { parseSelector(selector: string): ParsedSelector {
visitAllSelectorParts(parsed, part => { const result = parseSelector(selector);
visitAllSelectorParts(result, part => {
if (!this._engines.has(part.name)) if (!this._engines.has(part.name))
throw this.createStacklessError(`Unknown engine "${part.name}" while parsing selector ${selector}`); 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; return result;
} }

View file

@ -1471,29 +1471,18 @@ function removeEventListeners(listeners: (() => void)[]) {
listeners.splice(0, listeners.length); 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[] } { function querySelector(injectedScript: InjectedScript, selector: string, ownerDocument: Document): { selector: string, elements: Element[] } {
const empty = { selector, elements: [] };
try { try {
// selector starts from the root frame, we need to figure out if this frame is the right one const parsedSelector = injectedScript.parseSelector(selector);
// as a cheap heuristic, we check if it targets the right depth return {
const parts = injectedScript.splitSelectorByFrame(selector); selector,
const selectorTargetsFrameDepth = parts.length - 1 === frameDepth(); elements: injectedScript.querySelectorAll(parsedSelector, ownerDocument)
if (selectorTargetsFrameDepth) };
return { selector, elements: injectedScript.querySelectorAll(parts[parts.length - 1], ownerDocument) };
return empty;
} catch (e) { } catch (e) {
return empty; return {
selector,
elements: [],
};
} }
} }

View file

@ -30,6 +30,8 @@ import type { IRecorderAppFactory, IRecorderApp, IRecorder } from './recorder/re
import { metadataToCallLog } from './recorder/recorderUtils'; import { metadataToCallLog } from './recorder/recorderUtils';
import type * as actions from '@recorder/actions'; import type * as actions from '@recorder/actions';
import { buildFullSelector } from '../utils/isomorphic/recorderUtils'; import { buildFullSelector } from '../utils/isomorphic/recorderUtils';
import { splitSelectorByFrame, stringifySelector } from '../utils/isomorphic/selectorParser';
import type { Frame } from './frames';
const recorderSymbol = Symbol('recorderSymbol'); const recorderSymbol = Symbol('recorderSymbol');
@ -149,7 +151,7 @@ export class Recorder implements InstrumentationListener, IRecorder {
let actionPoint: Point | undefined; let actionPoint: Point | undefined;
const hasActiveScreenshotCommand = [...this._currentCallsMetadata.keys()].some(isScreenshotCommand); const hasActiveScreenshotCommand = [...this._currentCallsMetadata.keys()].some(isScreenshotCommand);
if (!hasActiveScreenshotCommand) { if (!hasActiveScreenshotCommand) {
actionSelector = this._highlightedSelector; actionSelector = this._scopeHighlightedSelectorToFrame(source.frame);
for (const [metadata, sdkObject] of this._currentCallsMetadata) { for (const [metadata, sdkObject] of this._currentCallsMetadata) {
if (source.page === sdkObject.attribution.page) { if (source.page === sdkObject.attribution.page) {
actionPoint = metadata.point || actionPoint; actionPoint = metadata.point || actionPoint;
@ -241,6 +243,17 @@ export class Recorder implements InstrumentationListener, IRecorder {
this._refreshOverlay(); 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) { setOutput(codegenId: string, outputFile: string | undefined) {
this._contextRecorder.setOutput(codegenId, outputFile); this._contextRecorder.setOutput(codegenId, outputFile);
} }

View file

@ -259,7 +259,7 @@ test('should highlight inside iframe', async ({ backend, connectedBrowser }, tes
const context = await connectedBrowser._newContextForReuse(); const context = await connectedBrowser._newContextForReuse();
const page = await context.newPage(); const page = await context.newPage();
await backend.navigate({ url: `data:text/html,<iframe srcdoc="<div>bar</div>"/>` }); await backend.navigate({ url: `data:text/html,<div>bar</div><iframe srcdoc="<div>bar</div>"/>` });
await page.frameLocator('iframe').getByText('bar').highlight(); await page.frameLocator('iframe').getByText('bar').highlight();
@ -271,4 +271,11 @@ test('should highlight inside iframe', async ({ backend, connectedBrowser }, tes
await backend.highlight({ selector: `frameLocator('iframe').getByText('bar')` }); await backend.highlight({ selector: `frameLocator('iframe').getByText('bar')` });
await expect(highlight).not.toHaveCount(0); await expect(highlight).not.toHaveCount(0);
await backend.highlight({ selector: `frameLocator('iframe').frameLocator('iframe').getByText('bar')` });
await expect(highlight).toHaveCount(0);
await backend.highlight({ selector: `getByText('bar')` });
await expect(highlight).toHaveCount(1);
await expect(page.locator('x-pw-highlight')).toHaveCount(1);
}); });