chore: parse locators strictly (#18553)

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();
   }

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();
     }

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, '\'');
+}

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) {
+  setHighlightedSelector(language: Language, selector: string) {
-    this._highlightedSelector = locatorOrSelectorAsSelector(selector);
+    this._highlightedSelector = locatorOrSelectorAsSelector(language, selector);
+    this._refreshOverlay();
+  }
+
+  hideHighlightedSelecor() {
+    this._highlightedSelector = '';
     this._refreshOverlay();
   }
 

packages/recorder/src/recorder.tsx

@@ -139,7 +139,7 @@ export const Recorder: React.FC<RecorderProps> = ({
           }}>Explore</ToolbarButton>
           <CodeMirrorWrapper text={locator} language={source.language} readOnly={false} focusOnChange={true} wrapLines={true} onChange={text => {
             setLocator(text);
-            window.dispatch({ event: 'selectorUpdated', params: { selector: text } });
+            window.dispatch({ event: 'selectorUpdated', params: { selector: text, language: source.language } });
           }}></CodeMirrorWrapper>
           <ToolbarButton icon='files' title='Copy' onClick={() => {
             copy(locator);

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);
+});