From 5488c03d7f508737d65a4a932f06aec93969b6c4 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 16 Nov 2023 16:31:34 -0800 Subject: [PATCH] chore: make `asLocator()` always safe (#28207) --- packages/playwright-core/src/client/locator.ts | 2 +- .../src/utils/isomorphic/locatorGenerators.ts | 18 +++++++----------- .../src/utils/isomorphic/locatorParser.ts | 2 +- packages/playwright/src/index.ts | 2 +- packages/recorder/src/callLog.tsx | 5 ++--- packages/trace-viewer/src/ui/actionList.tsx | 2 +- packages/trace-viewer/src/ui/callTab.tsx | 2 +- packages/trace-viewer/src/ui/snapshotTab.tsx | 2 +- 8 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/playwright-core/src/client/locator.ts b/packages/playwright-core/src/client/locator.ts index fb811ea861..d6b47037bb 100644 --- a/packages/playwright-core/src/client/locator.ts +++ b/packages/playwright-core/src/client/locator.ts @@ -355,7 +355,7 @@ export class Locator implements api.Locator { } toString() { - return asLocator('javascript', this._selector, undefined, true); + return asLocator('javascript', this._selector); } } diff --git a/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts b/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts index 3715537f1d..e51fa0b623 100644 --- a/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts +++ b/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts @@ -35,20 +35,16 @@ export interface LocatorFactory { chainLocators(locators: string[]): string; } -export function asLocator(lang: Language, selector: string, isFrameLocator: boolean = false, playSafe: boolean = false): string { - return asLocators(lang, selector, isFrameLocator, playSafe)[0]; +export function asLocator(lang: Language, selector: string, isFrameLocator: boolean = false): string { + return asLocators(lang, selector, isFrameLocator)[0]; } -export function asLocators(lang: Language, selector: string, isFrameLocator: boolean = false, playSafe: boolean = false, maxOutputSize = 20, preferredQuote?: Quote): string[] { - if (playSafe) { - try { - return innerAsLocators(new generators[lang](preferredQuote), parseSelector(selector), isFrameLocator, maxOutputSize); - } catch (e) { - // Tolerate invalid input. - return [selector]; - } - } else { +export function asLocators(lang: Language, selector: string, isFrameLocator: boolean = false, maxOutputSize = 20, preferredQuote?: Quote): string[] { + try { return innerAsLocators(new generators[lang](preferredQuote), parseSelector(selector), isFrameLocator, maxOutputSize); + } catch (e) { + // Tolerate invalid input. + return [selector]; } } diff --git a/packages/playwright-core/src/utils/isomorphic/locatorParser.ts b/packages/playwright-core/src/utils/isomorphic/locatorParser.ts index 3b291a4996..886574e580 100644 --- a/packages/playwright-core/src/utils/isomorphic/locatorParser.ts +++ b/packages/playwright-core/src/utils/isomorphic/locatorParser.ts @@ -219,7 +219,7 @@ export function locatorOrSelectorAsSelector(language: Language, locator: string, } try { const { selector, preferredQuote } = parseLocator(locator, testIdAttributeName); - const locators = asLocators(language, selector, undefined, undefined, undefined, preferredQuote); + const locators = asLocators(language, selector, undefined, undefined, preferredQuote); const digest = digestForComparison(locator); if (locators.some(candidate => digestForComparison(candidate) === digest)) return selector; diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 20670b3462..1bc888ef45 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -729,7 +729,7 @@ function renderApiCall(apiName: string, params: any) { continue; let value; if (name === 'selector' && isString(params[name]) && params[name].startsWith('internal:')) { - const getter = asLocator('javascript', params[name], false, true); + const getter = asLocator('javascript', params[name]); apiName = apiName.replace(/^locator\./, 'locator.' + getter + '.'); apiName = apiName.replace(/^page\./, 'page.' + getter + '.'); apiName = apiName.replace(/^frame\./, 'frame.' + getter + '.'); diff --git a/packages/recorder/src/callLog.tsx b/packages/recorder/src/callLog.tsx index 022ffa75df..c47b1ac323 100644 --- a/packages/recorder/src/callLog.tsx +++ b/packages/recorder/src/callLog.tsx @@ -40,8 +40,7 @@ export const CallLogView: React.FC = ({ {log.map(callLog => { const expandOverride = expandOverrides.get(callLog.id); const isExpanded = typeof expandOverride === 'boolean' ? expandOverride : callLog.status !== 'done'; - const locator = callLog.params.selector ? asLocator(language, callLog.params.selector, false) : null; - const locatorCall = `page.${locator}`; + const locator = callLog.params.selector ? asLocator(language, callLog.params.selector) : null; let titlePrefix = callLog.title; let titleSuffix = ''; if (callLog.title.startsWith('expect.to') || callLog.title.startsWith('expect.not.to')) { @@ -63,7 +62,7 @@ export const CallLogView: React.FC = ({ }}> { titlePrefix } { callLog.params.url ? {callLog.params.url} : undefined } - { locator ? {locatorCall} : undefined } + { locator ? {`page.${locator}`} : undefined } { titleSuffix } { typeof callLog.duration === 'number' ? — {msToString(callLog.duration)} : undefined} diff --git a/packages/trace-viewer/src/ui/actionList.tsx b/packages/trace-viewer/src/ui/actionList.tsx index a0f239b712..886df28a4c 100644 --- a/packages/trace-viewer/src/ui/actionList.tsx +++ b/packages/trace-viewer/src/ui/actionList.tsx @@ -88,7 +88,7 @@ export const renderAction = ( }) => { const { sdkLanguage, revealConsole, isLive, showDuration, showBadges } = options; const { errors, warnings } = modelUtil.stats(action); - const locator = action.params.selector ? asLocator(sdkLanguage || 'javascript', action.params.selector, false /* isFrameLocator */, true /* playSafe */) : undefined; + const locator = action.params.selector ? asLocator(sdkLanguage || 'javascript', action.params.selector) : undefined; let time: string = ''; if (action.endTime) diff --git a/packages/trace-viewer/src/ui/callTab.tsx b/packages/trace-viewer/src/ui/callTab.tsx index bb4cd2a0b4..463ba5056c 100644 --- a/packages/trace-viewer/src/ui/callTab.tsx +++ b/packages/trace-viewer/src/ui/callTab.tsx @@ -86,7 +86,7 @@ function propertyToString(event: ActionTraceEvent, name: string, value: any, sdk if ((name === 'value' && isEval) || (name === 'received' && event.method === 'expect')) value = parseSerializedValue(value, new Array(10).fill({ handle: '' })); if (name === 'selector') - return { text: asLocator(sdkLanguage || 'javascript', event.params.selector, false /* isFrameLocator */, true /* playSafe */), type: 'locator', name: 'locator' }; + return { text: asLocator(sdkLanguage || 'javascript', event.params.selector), type: 'locator', name: 'locator' }; const type = typeof value; if (type !== 'object' || value === null) return { text: String(value), type, name }; diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index a292fd6785..9d0a03948b 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -245,7 +245,7 @@ export const InspectModeController: React.FunctionComponent<{ overlay: { offsetX: 0 }, }, { async setSelector(selector: string) { - setHighlightedLocator(asLocator(sdkLanguage, frameSelector + selector, false /* isFrameLocator */, true /* playSafe */)); + setHighlightedLocator(asLocator(sdkLanguage, frameSelector + selector)); }, highlightUpdated() { for (const r of recorders) {