fix(codegen): don't generate page.frame() calls anymore (#27820)

Fixes https://github.com/microsoft/playwright/issues/27650

packages/playwright-core/src/server/injected/selectorGenerator.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalizeWhiteSpace } from '../../utils/isomorphic/stringUtils';
+import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalizeWhiteSpace, quoteCSSAttributeValue } from '../../utils/isomorphic/stringUtils';
 import { closestCrossShadow, isInsideScope, parentElementOrShadowHost } from './domUtils';
 import type { InjectedScript } from './injectedScript';
 import { getAriaRole, getElementAccessibleName, beginAriaCaches, endAriaCaches } from './roleUtils';
@@ -198,7 +198,7 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element,
   {
     for (const attr of ['data-testid', 'data-test-id', 'data-test']) {
       if (attr !== options.testIdAttributeName && element.getAttribute(attr))
-        candidates.push({ engine: 'css', selector: `[${attr}=${quoteAttributeValue(element.getAttribute(attr)!)}]`, score: kOtherTestIdScore });
+        candidates.push({ engine: 'css', selector: `[${attr}=${quoteCSSAttributeValue(element.getAttribute(attr)!)}]`, score: kOtherTestIdScore });
     }
 
     const idAttr = element.getAttribute('id');
@@ -211,12 +211,12 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element,
   if (element.nodeName === 'IFRAME') {
     for (const attribute of ['name', 'title']) {
       if (element.getAttribute(attribute))
-        candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[${attribute}=${quoteAttributeValue(element.getAttribute(attribute)!)}]`, score: kIframeByAttributeScore });
+        candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[${attribute}=${quoteCSSAttributeValue(element.getAttribute(attribute)!)}]`, score: kIframeByAttributeScore });
     }
 
     // Locate by testId via CSS selector.
     if (element.getAttribute(options.testIdAttributeName))
-      candidates.push({ engine: 'css', selector: `[${options.testIdAttributeName}=${quoteAttributeValue(element.getAttribute(options.testIdAttributeName)!)}]`, score: kTestIdScore });
+      candidates.push({ engine: 'css', selector: `[${options.testIdAttributeName}=${quoteCSSAttributeValue(element.getAttribute(options.testIdAttributeName)!)}]`, score: kTestIdScore });
 
     penalizeScoreForLength([candidates]);
     return candidates;
@@ -246,11 +246,11 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element,
     candidates.push({ engine: 'internal:role', selector: ariaRole, score: kRoleWithoutNameScore });
 
   if (element.getAttribute('name') && ['BUTTON', 'FORM', 'FIELDSET', 'FRAME', 'IFRAME', 'INPUT', 'KEYGEN', 'OBJECT', 'OUTPUT', 'SELECT', 'TEXTAREA', 'MAP', 'META', 'PARAM'].includes(element.nodeName))
-    candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[name=${quoteAttributeValue(element.getAttribute('name')!)}]`, score: kCSSInputTypeNameScore });
+    candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[name=${quoteCSSAttributeValue(element.getAttribute('name')!)}]`, score: kCSSInputTypeNameScore });
 
   if (['INPUT', 'TEXTAREA'].includes(element.nodeName) && element.getAttribute('type') !== 'hidden') {
     if (element.getAttribute('type'))
-      candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[type=${quoteAttributeValue(element.getAttribute('type')!)}]`, score: kCSSInputTypeNameScore });
+      candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[type=${quoteCSSAttributeValue(element.getAttribute('type')!)}]`, score: kCSSInputTypeNameScore });
   }
 
   if (['INPUT', 'TEXTAREA', 'SELECT'].includes(element.nodeName) && element.getAttribute('type') !== 'hidden')
@@ -378,10 +378,6 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element, opt
   return makeStrict(uniqueCSSSelector()!);
 }
 
-function quoteAttributeValue(text: string): string {
-  return `"${cssEscape(text).replace(/\\ /g, ' ')}"`;
-}
-
 function penalizeScoreForLength(groups: SelectorToken[][]) {
   for (const group of groups) {
     for (const token of group) {

packages/playwright-core/src/server/recorder.ts

@@ -43,6 +43,7 @@ import { EventEmitter } from 'events';
 import { raceAgainstDeadline } from '../utils/timeoutRunner';
 import type { Language, LanguageGenerator } from './recorder/language';
 import { locatorOrSelectorAsSelector } from '../utils/isomorphic/locatorParser';
+import { quoteCSSAttributeValue } from '../utils/isomorphic/stringUtils';
 import { eventsHelper, type RegisteredListener } from './../utils/eventsHelper';
 import type { Dialog } from './dialog';
 
@@ -530,7 +531,6 @@ class ContextRecorder extends EventEmitter {
     return {
       pageAlias: this._pageAliases.get(page)!,
       isMainFrame: true,
-      url: page.mainFrame().url(),
     };
   }
 
@@ -545,16 +545,6 @@ class ContextRecorder extends EventEmitter {
     if (chain.length === 1)
       return this._describeMainFrame(page);
 
-    const hasUniqueName = page.frames().filter(f => f.name() === frame.name()).length === 1;
-    const fallback: actions.FrameDescription = {
-      pageAlias,
-      isMainFrame: false,
-      url: frame.url(),
-      name: frame.name() && hasUniqueName ? frame.name() : undefined,
-    };
-    if (chain.length > 3)
-      return fallback;
-
     const selectorPromises: Promise<string | undefined>[] = [];
     for (let i = 0; i < chain.length - 1; i++)
       selectorPromises.push(findFrameSelector(chain[i + 1]));
@@ -562,11 +552,24 @@ class ContextRecorder extends EventEmitter {
     const result = await raceAgainstDeadline(() => Promise.all(selectorPromises), monotonicTime() + 2000);
     if (!result.timedOut && result.result.every(selector => !!selector)) {
       return {
-        ...fallback,
+        pageAlias,
+        isMainFrame: false,
         selectorsChain: result.result as string[],
       };
     }
-    return fallback;
+    // Best effort to find a selector for the frame.
+    const selectorsChain = [];
+    for (let i = 0; i < chain.length - 1; i++) {
+      if (chain[i].name())
+        selectorsChain.push(`iframe[name=${quoteCSSAttributeValue(chain[i].name())}]`);
+      else
+        selectorsChain.push(`iframe[src=${quoteCSSAttributeValue(chain[i].url())}]`);
+    }
+    return {
+      pageAlias,
+      isMainFrame: false,
+      selectorsChain,
+    };
   }
 
   testIdAttributeName(): string {

packages/playwright-core/src/server/recorder/codeGenerator.ts

@@ -141,12 +141,11 @@ export class CodeGenerator extends EventEmitter {
       return;
     }
 
-    if (signal.name === 'navigation') {
+    if (signal.name === 'navigation' && frame._page.mainFrame() === frame) {
       this.addAction({
         frame: {
           pageAlias,
-          isMainFrame: frame._page.mainFrame() === frame,
+          isMainFrame: true,
-          url: frame.url(),
         },
         committed: true,
         action: {

packages/playwright-core/src/server/recorder/csharp.ts

@@ -76,13 +76,9 @@ export class CSharpLanguageGenerator implements LanguageGenerator {
     let subject: string;
     if (actionInContext.frame.isMainFrame) {
       subject = pageAlias;
-    } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') {
+    } else {
       const locators = actionInContext.frame.selectorsChain.map(selector => `.FrameLocator(${quote(selector)})`);
       subject = `${pageAlias}${locators.join('')}`;
-    } else if (actionInContext.frame.name) {
-      subject = `${pageAlias}.Frame(${quote(actionInContext.frame.name)})`;
-    } else {
-      subject = `${pageAlias}.FrameByUrl(${quote(actionInContext.frame.url)})`;
     }
 
     const signals = toSignalMap(action);

packages/playwright-core/src/server/recorder/java.ts

@@ -48,14 +48,10 @@ export class JavaLanguageGenerator implements LanguageGenerator {
     let inFrameLocator = false;
     if (actionInContext.frame.isMainFrame) {
       subject = pageAlias;
-    } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') {
+    } else {
       const locators = actionInContext.frame.selectorsChain.map(selector => `.frameLocator(${quote(selector)})`);
       subject = `${pageAlias}${locators.join('')}`;
       inFrameLocator = true;
-    } else if (actionInContext.frame.name) {
-      subject = `${pageAlias}.frame(${quote(actionInContext.frame.name)})`;
-    } else {
-      subject = `${pageAlias}.frameByUrl(${quote(actionInContext.frame.url)})`;
     }
 
     const signals = toSignalMap(action);

packages/playwright-core/src/server/recorder/javascript.ts

@@ -56,13 +56,9 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator {
     let subject: string;
     if (actionInContext.frame.isMainFrame) {
       subject = pageAlias;
-    } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') {
+    } else {
       const locators = actionInContext.frame.selectorsChain.map(selector => `.frameLocator(${quote(selector)})`);
       subject = `${pageAlias}${locators.join('')}`;
-    } else if (actionInContext.frame.name) {
-      subject = `${pageAlias}.frame(${formatObject({ name: actionInContext.frame.name })})`;
-    } else {
-      subject = `${pageAlias}.frame(${formatObject({ url: actionInContext.frame.url })})`;
     }
 
     const signals = toSignalMap(action);

packages/playwright-core/src/server/recorder/python.ts

@@ -63,13 +63,9 @@ export class PythonLanguageGenerator implements LanguageGenerator {
     let subject: string;
     if (actionInContext.frame.isMainFrame) {
       subject = pageAlias;
-    } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') {
+    } else {
       const locators = actionInContext.frame.selectorsChain.map(selector => `.frame_locator(${quote(selector)})`);
       subject = `${pageAlias}${locators.join('')}`;
-    } else if (actionInContext.frame.name) {
-      subject = `${pageAlias}.frame(${formatOptions({ name: actionInContext.frame.name }, false)})`;
-    } else {
-      subject = `${pageAlias}.frame(${formatOptions({ url: actionInContext.frame.url }, false)})`;
     }
 
     const signals = toSignalMap(action);

packages/playwright-core/src/server/recorder/recorderActions.ts

@@ -128,10 +128,13 @@ export type DialogSignal = BaseSignal & {
 
 export type Signal = NavigationSignal | PopupSignal | DownloadSignal | DialogSignal;
 
-export type FrameDescription = {
+type FrameDescriptionMainFrame = {
-  pageAlias: string;
+  isMainFrame: true;
-  isMainFrame: boolean;
-  url: string;
-  name?: string;
-  selectorsChain?: string[];
 };
+
+type FrameDescriptionChildFrame = {
+  isMainFrame: false;
+  selectorsChain: string[];
+};
+
+export type FrameDescription = { pageAlias: string } & (FrameDescriptionMainFrame | FrameDescriptionChildFrame);

packages/playwright-core/src/utils/isomorphic/stringUtils.ts

@@ -47,6 +47,10 @@ export function cssEscape(s: string): string {
   return result;
 }
 
+export function quoteCSSAttributeValue(text: string): string {
+  return `"${cssEscape(text).replace(/\\ /g, ' ')}"`;
+}
+
 function cssEscapeOne(s: string, i: number): string {
   // https://drafts.csswg.org/cssom/#serialize-an-identifier
   const c = s.charCodeAt(i);

tests/library/inspector/cli-codegen-3.spec.ts

@@ -163,44 +163,67 @@ test.describe('cli codegen', () => {
     ]);
 
     expect.soft(sources.get('JavaScript')!.text).toContain(`
-  await page.frame({
+  await page.frameLocator('#frame1').frameLocator('iframe').frameLocator('iframe[name="one"]').getByText('HelloNameOne').click();`);
-    name: 'one'
-  }).getByText('HelloNameOne').click();`);
 
     expect.soft(sources.get('Java')!.text).toContain(`
-      page.frame("one").getByText("HelloNameOne").click();`);
+      page.frameLocator("#frame1").frameLocator("iframe").frameLocator("iframe[name=\\"one\\"]").getByText("HelloNameOne").click();`);
 
     expect.soft(sources.get('Python')!.text).toContain(`
-    page.frame(name=\"one\").get_by_text(\"HelloNameOne\").click()`);
+    page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe[name=\\"one\\"]").get_by_text("HelloNameOne").click()`);
 
     expect.soft(sources.get('Python Async')!.text).toContain(`
-    await page.frame(name=\"one\").get_by_text(\"HelloNameOne\").click()`);
+    await page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe[name=\\"one\\"]").get_by_text("HelloNameOne").click()`);
 
     expect.soft(sources.get('C#')!.text).toContain(`
-        await page.Frame(\"one\").GetByText(\"HelloNameOne\").ClickAsync();`);
+        await page.FrameLocator("#frame1").FrameLocator("iframe").FrameLocator("iframe[name=\\"one\\"]").GetByText("HelloNameOne").ClickAsync();`);
-
 
     [sources] = await Promise.all([
-      recorder.waitForOutput('JavaScript', 'url:'),
+      recorder.waitForOutput('JavaScript', 'HelloNameAnonymous'),
       frameAnonymous.click('text=HelloNameAnonymous'),
     ]);
 
     expect.soft(sources.get('JavaScript')!.text).toContain(`
-  await page.frame({
+  await page.frameLocator('#frame1').frameLocator('iframe').frameLocator('iframe >> nth=2').getByText('HelloNameAnonymous').click();`);
-    url: 'about:blank'
-  }).getByText('HelloNameAnonymous').click();`);
 
     expect.soft(sources.get('Java')!.text).toContain(`
-      page.frameByUrl("about:blank").getByText("HelloNameAnonymous").click();`);
+      page.frameLocator("#frame1").frameLocator("iframe").frameLocator("iframe >> nth=2").getByText("HelloNameAnonymous").click();`);
 
     expect.soft(sources.get('Python')!.text).toContain(`
-    page.frame(url=\"about:blank\").get_by_text(\"HelloNameAnonymous\").click()`);
+    page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe >> nth=2").get_by_text("HelloNameAnonymous").click()`);
 
     expect.soft(sources.get('Python Async')!.text).toContain(`
-    await page.frame(url=\"about:blank\").get_by_text(\"HelloNameAnonymous\").click()`);
+    await page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe >> nth=2").get_by_text("HelloNameAnonymous").click()`);
 
     expect.soft(sources.get('C#')!.text).toContain(`
-        await page.FrameByUrl(\"about:blank\").GetByText(\"HelloNameAnonymous\").ClickAsync();`);
+        await page.FrameLocator("#frame1").FrameLocator("iframe").FrameLocator("iframe >> nth=2").GetByText("HelloNameAnonymous").ClickAsync();`);
+  });
+
+  test('should generate frame locators with special characters in name attribute', async ({ page, openRecorder, server }) => {
+    const recorder = await openRecorder();
+    await recorder.setContentAndWait(`
+      <iframe srcdoc="<button>Click me</button>">
+    `, server.EMPTY_PAGE, 2);
+    await page.$eval('iframe', (frame: HTMLIFrameElement) => {
+      frame.name = 'foo<bar\'"`>';
+    });
+    const [sources] = await Promise.all([
+      recorder.waitForOutput('JavaScript', 'Click me'),
+      page.frameLocator('iframe[name="foo<bar\'\\"`>"]').getByRole('button', { name: 'Click me' }).click(),
+    ]);
+    expect.soft(sources.get('JavaScript')!.text).toContain(`
+  await page.frameLocator('iframe[name="foo\\\\<bar\\\\\\'\\\\"\\\\\`\\\\>"]').getByRole('button', { name: 'Click me' }).click();`);
+
+    expect.soft(sources.get('Java')!.text).toContain(`
+      page.frameLocator("iframe[name=\\"foo\\\\<bar\\\\'\\\\\\"\\\\\`\\\\>\\"]").getByRole(AriaRole.BUTTON, new FrameLocator.GetByRoleOptions().setName("Click me")).click()`);
+
+    expect.soft(sources.get('Python')!.text).toContain(`
+    page.frame_locator("iframe[name=\\"foo\\\\<bar\\\\'\\\\\\"\\\\\`\\\\>\\"]").get_by_role("button", name="Click me").click()`);
+
+    expect.soft(sources.get('Python Async')!.text).toContain(`
+    await page.frame_locator("iframe[name=\\"foo\\\\<bar\\\\'\\\\\\"\\\\\`\\\\>\\"]").get_by_role("button", name="Click me").click()`);
+
+    expect.soft(sources.get('C#')!.text).toContain(`
+        await page.FrameLocator("iframe[name=\\"foo\\\\<bar\\\\'\\\\\\"\\\\\`\\\\>\\"]").GetByRole(AriaRole.Button, new() { Name = "Click me" }).ClickAsync();`);
   });
 
   test('should generate frame locators with title attribute', async ({ page, openRecorder, server }) => {