From 3bc9e07daf103a4eff9cfc7e2b2397d4c46e0a06 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 3 Nov 2022 15:17:08 -0700 Subject: [PATCH] chore: parse locators strictly (#18553) --- .../src/server/debugController.ts | 10 +++--- .../src/server/injected/recorder.ts | 2 +- .../src/server/isomorphic/locatorParser.ts | 14 ++++++-- .../playwright-core/src/server/recorder.ts | 11 ++++-- packages/recorder/src/recorder.tsx | 2 +- tests/library/locator-generator.spec.ts | 36 ++++++++++++++++--- 6 files changed, 56 insertions(+), 19 deletions(-) diff --git a/packages/playwright-core/src/server/debugController.ts b/packages/playwright-core/src/server/debugController.ts index d48548ef77..da2f0ed834 100644 --- a/packages/playwright-core/src/server/debugController.ts +++ b/packages/playwright-core/src/server/debugController.ts @@ -25,7 +25,6 @@ import { Recorder } from './recorder'; import { EmptyRecorderApp } from './recorder/recorderApp'; import { asLocator } from './isomorphic/locatorGenerators'; import type { Language } from './isomorphic/locatorGenerators'; -import { locatorOrSelectorAsSelector } from './isomorphic/locatorParser'; const internalMetadata = serverSideCallMetadata(); @@ -96,7 +95,7 @@ export class DebugController extends SdkObject { if (params.mode === 'none') { for (const recorder of await this._allRecorders()) { - recorder.setHighlightedSelector(''); + recorder.hideHighlightedSelecor(); recorder.setMode('none'); } this.setAutoCloseEnabled(true); @@ -114,7 +113,7 @@ export class DebugController extends SdkObject { } // Toggle the mode. for (const recorder of await this._allRecorders()) { - recorder.setHighlightedSelector(''); + recorder.hideHighlightedSelecor(); if (params.mode === 'recording') recorder.setOutput(this._codegenId, params.file); recorder.setMode(params.mode); @@ -139,15 +138,14 @@ export class DebugController extends SdkObject { } async highlight(selector: string) { - selector = locatorOrSelectorAsSelector(selector); for (const recorder of await this._allRecorders()) - recorder.setHighlightedSelector(selector); + recorder.setHighlightedSelector(this._sdkLanguage, selector); } async hideHighlight() { // Hide all active recorder highlights. for (const recorder of await this._allRecorders()) - recorder.setHighlightedSelector(''); + recorder.hideHighlightedSelecor(); // Hide all locator.highlight highlights. await this._playwright.hideHighlight(); } diff --git a/packages/playwright-core/src/server/injected/recorder.ts b/packages/playwright-core/src/server/injected/recorder.ts index 7df25aadf3..75de6fe827 100644 --- a/packages/playwright-core/src/server/injected/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder.ts @@ -228,7 +228,7 @@ class Recorder { private _onMouseLeave(event: MouseEvent) { // Leaving iframe. - if (this._deepEventTarget(event).nodeType === Node.DOCUMENT_NODE) { + if (window.top !== window && this._deepEventTarget(event).nodeType === Node.DOCUMENT_NODE) { this._hoveredElement = null; this._updateModelForHoveredElement(); } diff --git a/packages/playwright-core/src/server/isomorphic/locatorParser.ts b/packages/playwright-core/src/server/isomorphic/locatorParser.ts index 98995d0491..284979264d 100644 --- a/packages/playwright-core/src/server/isomorphic/locatorParser.ts +++ b/packages/playwright-core/src/server/isomorphic/locatorParser.ts @@ -15,9 +15,11 @@ */ import { escapeForAttributeSelector, escapeForTextSelector } from '../../utils/isomorphic/stringUtils'; +import { asLocator } from './locatorGenerators'; +import type { Language } from './locatorGenerators'; import { parseSelector } from './selectorParser'; -export function parseLocator(locator: string): string { +function parseLocator(locator: string): string { locator = locator .replace(/AriaRole\s*\.\s*([\w]+)/g, (_, group) => group.toLowerCase()) .replace(/(get_by_role|getByRole)\s*\(\s*(?:["'`])([^'"`]+)['"`]/g, (_, group1, group2) => `${group1}(${group2.toLowerCase()}`); @@ -121,15 +123,21 @@ export function parseLocator(locator: string): string { }).join(' >> '); } -export function locatorOrSelectorAsSelector(locator: string): string { +export function locatorOrSelectorAsSelector(language: Language, locator: string): string { try { parseSelector(locator); return locator; } catch (e) { } try { - return parseLocator(locator); + const selector = parseLocator(locator); + if (digestForComparison(asLocator(language, selector)) === digestForComparison(locator)) + return selector; } catch (e) { } return locator; } + +function digestForComparison(locator: string) { + return locator.replace(/\s/g, '').replace(/["`]/g, '\''); +} diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index e8320eeaa4..5881eb8abb 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -107,7 +107,7 @@ export class Recorder implements InstrumentationListener { return; } if (data.event === 'selectorUpdated') { - this.setHighlightedSelector(data.params.selector); + this.setHighlightedSelector(data.params.language, data.params.selector); return; } if (data.event === 'step') { @@ -210,8 +210,13 @@ export class Recorder implements InstrumentationListener { this._refreshOverlay(); } - setHighlightedSelector(selector: string) { - this._highlightedSelector = locatorOrSelectorAsSelector(selector); + setHighlightedSelector(language: Language, selector: string) { + this._highlightedSelector = locatorOrSelectorAsSelector(language, selector); + this._refreshOverlay(); + } + + hideHighlightedSelecor() { + this._highlightedSelector = ''; this._refreshOverlay(); } diff --git a/packages/recorder/src/recorder.tsx b/packages/recorder/src/recorder.tsx index dd5e425727..a43440fdc6 100644 --- a/packages/recorder/src/recorder.tsx +++ b/packages/recorder/src/recorder.tsx @@ -139,7 +139,7 @@ export const Recorder: React.FC = ({ }}>Explore { setLocator(text); - window.dispatch({ event: 'selectorUpdated', params: { selector: text } }); + window.dispatch({ event: 'selectorUpdated', params: { selector: text, language: source.language } }); }}> { copy(locator); diff --git a/tests/library/locator-generator.spec.ts b/tests/library/locator-generator.spec.ts index 454bed9417..d9f4126047 100644 --- a/tests/library/locator-generator.spec.ts +++ b/tests/library/locator-generator.spec.ts @@ -16,9 +16,11 @@ import { contextTest as it, expect } from '../config/browserTest'; import { asLocator } from '../../packages/playwright-core/lib/server/isomorphic/locatorGenerators'; -import { parseLocator } from '../../packages/playwright-core/lib/server/isomorphic/locatorParser'; +import { locatorOrSelectorAsSelector as parseLocator } from '../../packages/playwright-core/lib/server/isomorphic/locatorParser'; import type { Page, Frame, Locator } from 'playwright-core'; +it.skip(({ mode }) => mode !== 'default'); + function generate(locator: Locator) { return generateForSelector((locator as any)._selector); } @@ -27,7 +29,7 @@ function generateForSelector(selector: string) { const result: any = {}; for (const lang of ['javascript', 'python', 'java', 'csharp']) { const locatorString = asLocator(lang, selector, false); - expect.soft(parseLocator(locatorString), lang + ' mismatch').toBe(selector); + expect.soft(parseLocator(lang, locatorString), lang + ' mismatch').toBe(selector); result[lang] = locatorString; } return result; @@ -38,7 +40,7 @@ async function generateForNode(pageOrFrame: Page | Frame, target: string): Promi const result: any = {}; for (const lang of ['javascript', 'python', 'java', 'csharp']) { const locatorString = asLocator(lang, selector, false); - expect.soft(parseLocator(locatorString)).toBe(selector); + expect.soft(parseLocator(lang, locatorString)).toBe(selector); result[lang] = locatorString; } return result; @@ -257,8 +259,6 @@ it('reverse engineer hasText', async ({ page }) => { }); it.describe(() => { - it.skip(({ mode }) => mode !== 'default'); - it.beforeEach(async ({ context }) => { await (context as any)._enableRecorder({ language: 'javascript' }); }); @@ -299,3 +299,29 @@ it.describe(() => { }); }); }); + +it('parse locators strictly', () => { + const selector = 'div >> internal:has-text=\"Goodbye world\"i >> span'; + + // Exact + expect.soft(parseLocator('csharp', `Locator("div").Filter(new() { HasTextString: "Goodbye world" }).Locator("span")`)).toBe(selector); + expect.soft(parseLocator('java', `locator("div").filter(new Locator.LocatorOptions().setHasText("Goodbye world")).locator("span")`)).toBe(selector); + expect.soft(parseLocator('javascript', `locator('div').filter({ hasText: 'Goodbye world' }).locator('span')`)).toBe(selector); + expect.soft(parseLocator('python', `locator("div").filter(has_text="Goodbye world").locator("span")`)).toBe(selector); + + // Quotes + expect.soft(parseLocator('javascript', `locator("div").filter({ hasText: "Goodbye world" }).locator("span")`)).toBe(selector); + expect.soft(parseLocator('python', `locator('div').filter(has_text='Goodbye world').locator('span')`)).toBe(selector); + + // Whitespace + expect.soft(parseLocator('csharp', `Locator("div") . Filter (new ( ) { HasTextString: "Goodbye world" }).Locator( "span" )`)).toBe(selector); + expect.soft(parseLocator('java', ` locator("div" ). filter( new Locator. LocatorOptions ( ) .setHasText( "Goodbye world" ) ).locator( "span")`)).toBe(selector); + expect.soft(parseLocator('javascript', `locator\n('div')\n\n.filter({ hasText : 'Goodbye world'\n }\n).locator('span')\n`)).toBe(selector); + expect.soft(parseLocator('python', `\tlocator(\t"div").filter(\thas_text="Goodbye world"\t).locator\t("span")`)).toBe(selector); + + // Extra symbols + expect.soft(parseLocator('csharp', `Locator("div").Filter(new() { HasTextString: "Goodbye world" }).Locator("span"))`)).not.toBe(selector); + expect.soft(parseLocator('java', `locator("div").filter(new Locator.LocatorOptions().setHasText("Goodbye world"))..locator("span")`)).not.toBe(selector); + expect.soft(parseLocator('javascript', `locator('div').filter({ hasText: 'Goodbye world' }}).locator('span')`)).not.toBe(selector); + expect.soft(parseLocator('python', `locator("div").filter(has_text=="Goodbye world").locator("span")`)).not.toBe(selector); +});