feat: support alternative quotes in js parseLocator() (#27718)

Fixes #27707.
microsoft · Oct 20, 2023 · 6fe31ab · 6fe31ab
1 parent 9fcfe68
commit 6fe31ab
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 15 deletions.
diff --git a/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts b/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts
@@ -21,6 +21,7 @@ import type { ParsedSelector } from './selectorParser';
 export type Language = 'javascript' | 'python' | 'java' | 'csharp' | 'jsonl';
 export type LocatorType = 'default' | 'role' | 'text' | 'label' | 'placeholder' | 'alt' | 'title' | 'test-id' | 'nth' | 'first' | 'last' | 'has-text' | 'has-not-text' | 'has' | 'hasNot' | 'frame' | 'and' | 'or' | 'chain';
 export type LocatorBase = 'page' | 'locator' | 'frame-locator';
+export type Quote = '\'' | '"' | '`';
 
 type LocatorOptions = {
   attrs?: { name: string, value: string | boolean | number }[],
@@ -38,16 +39,16 @@ export function asLocator(lang: Language, selector: string, isFrameLocator: bool
   return asLocators(lang, selector, isFrameLocator, playSafe)[0];
 }
 
-export function asLocators(lang: Language, selector: string, isFrameLocator: boolean = false, playSafe: boolean = false, maxOutputSize = 20): string[] {
+export function asLocators(lang: Language, selector: string, isFrameLocator: boolean = false, playSafe: boolean = false, maxOutputSize = 20, preferredQuote?: Quote): string[] {
   if (playSafe) {
     try {
-      return innerAsLocators(generators[lang], parseSelector(selector), isFrameLocator, maxOutputSize);
+      return innerAsLocators(new generators[lang](preferredQuote), parseSelector(selector), isFrameLocator, maxOutputSize);
     } catch (e) {
       // Tolerate invalid input.
       return [selector];
     }
   } else {
-    return innerAsLocators(generators[lang], parseSelector(selector), isFrameLocator, maxOutputSize);
+    return innerAsLocators(new generators[lang](preferredQuote), parseSelector(selector), isFrameLocator, maxOutputSize);
   }
 }
 
@@ -249,6 +250,8 @@ function detectExact(text: string): { exact?: boolean, text: string | RegExp } {
 }
 
 export class JavaScriptLocatorFactory implements LocatorFactory {
+  constructor(private preferredQuote?: Quote) {}
+
   generateLocator(base: LocatorBase, kind: LocatorType, body: string | RegExp, options: LocatorOptions = {}): string {
     switch (kind) {
       case 'default':
@@ -336,7 +339,7 @@ export class JavaScriptLocatorFactory implements LocatorFactory {
   }
 
   private quote(text: string) {
-    return escapeWithQuotes(text, '\'');
+    return escapeWithQuotes(text, this.preferredQuote ?? '\'');
   }
 }
 
@@ -658,12 +661,12 @@ export class JsonlLocatorFactory implements LocatorFactory {
   }
 }
 
-const generators: Record<Language, LocatorFactory> = {
-  javascript: new JavaScriptLocatorFactory(),
-  python: new PythonLocatorFactory(),
-  java: new JavaLocatorFactory(),
-  csharp: new CSharpLocatorFactory(),
-  jsonl: new JsonlLocatorFactory(),
+const generators: Record<Language, new (preferredQuote?: Quote) => LocatorFactory> = {
+  javascript: JavaScriptLocatorFactory,
+  python: PythonLocatorFactory,
+  java: JavaLocatorFactory,
+  csharp: CSharpLocatorFactory,
+  jsonl: JsonlLocatorFactory,
 };
 
 function isRegExp(obj: any): obj is RegExp {

diff --git a/packages/playwright-core/src/utils/isomorphic/locatorParser.ts b/packages/playwright-core/src/utils/isomorphic/locatorParser.ts
@@ -16,11 +16,11 @@
 
 import { escapeForAttributeSelector, escapeForTextSelector } from '../../utils/isomorphic/stringUtils';
 import { asLocators } from './locatorGenerators';
-import type { Language } from './locatorGenerators';
+import type { Language, Quote } from './locatorGenerators';
 import { parseSelector } from './selectorParser';
 
 type TemplateParams = { quote: string, text: string }[];
-function parseLocator(locator: string, testIdAttributeName: string): string {
+function parseLocator(locator: string, testIdAttributeName: string): { selector: string, preferredQuote: Quote | undefined } {
   locator = locator
       .replace(/AriaRole\s*\.\s*([\w]+)/g, (_, group) => group.toLowerCase())
       .replace(/(get_by_role|getByRole)\s*\(\s*(?:["'`])([^'"`]+)['"`]/g, (_, group1, group2) => `${group1}(${group2.toLowerCase()}`);
@@ -92,7 +92,8 @@ function parseLocator(locator: string, testIdAttributeName: string): string {
       .replace(/regex=/g, '=')
       .replace(/,,/g, ',');
 
-  return transform(template, params, testIdAttributeName);
+  const preferredQuote = params.map(p => p.quote).filter(quote => '\'"`'.includes(quote))[0] as Quote | undefined;
+  return { selector: transform(template, params, testIdAttributeName), preferredQuote };
 }
 
 function countParams(template: string) {
@@ -217,8 +218,8 @@ export function locatorOrSelectorAsSelector(language: Language, locator: string,
   } catch (e) {
   }
   try {
-    const selector = parseLocator(locator, testIdAttributeName);
-    const locators = asLocators(language, selector);
+    const { selector, preferredQuote } = parseLocator(locator, testIdAttributeName);
+    const locators = asLocators(language, selector, undefined, undefined, undefined, preferredQuote);
     const digest = digestForComparison(locator);
     if (locators.some(candidate => digestForComparison(candidate) === digest))
       return selector;

diff --git a/tests/library/locator-generator.spec.ts b/tests/library/locator-generator.spec.ts
@@ -509,6 +509,18 @@ it('asLocator xpath', async () => {
   expect.soft(asLocator('csharp', selector, false)).toBe(`Locator(\"xpath=//*[contains(normalizer-text(), 'foo']\")`);
 });
 
+it('parseLocator quotes', async () => {
+  expect.soft(parseLocator('javascript', `locator('text="bar"')`, '')).toBe(`text="bar"`);
+  expect.soft(parseLocator('javascript', `locator("text='bar'")`, '')).toBe(`text='bar'`);
+  expect.soft(parseLocator('javascript', "locator(`text='bar'`)", '')).toBe(`text='bar'`);
+  expect.soft(parseLocator('python', `locator("text='bar'")`, '')).toBe(`text='bar'`);
+  expect.soft(parseLocator('python', `locator('text="bar"')`, '')).toBe(``);
+  expect.soft(parseLocator('java', `locator("text='bar'")`, '')).toBe(`text='bar'`);
+  expect.soft(parseLocator('java', `locator('text="bar"')`, '')).toBe(``);
+  expect.soft(parseLocator('csharp', `Locator("text='bar'")`, '')).toBe(`text='bar'`);
+  expect.soft(parseLocator('csharp', `Locator('text="bar"')`, '')).toBe(``);
+});
+
 it('parse locators strictly', () => {
   const selector = 'div >> internal:has-text=\"Goodbye world\"i >> span';