cherry-pick(#18819): fix(locators): frameLocator().nth serialized correctly

Fixes #18798.

Author: dgozman

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

@@ -1667,7 +1667,7 @@ export class Frame extends SdkObject {
     for (let i = 0; i < frameChunks.length - 1 && progress.isRunning(); ++i) {
       const info = this._page.parseSelector(frameChunks[i], options);
       const task = dom.waitForSelectorTask(info, 'attached', false, i === 0 ? scope : undefined);
-      progress.log(`  waiting for frameLocator('${stringifySelector(frameChunks[i])}')`);
+      progress.log(`  waiting for ${this._asLocator(stringifySelector(frameChunks[i]) + ' >> internal:control=enter-frame')}`);
       const handle = i === 0 && scope ? await frame._runWaitForSelectorTaskOnce(progress, stringifySelector(info.parsed), info.world, task)
         : await frame._scheduleRerunnableHandleTask(progress, info.world, task);
       const element = handle.asElement() as dom.ElementHandle<Element>;

packages/playwright-core/src/server/isomorphic/locatorGenerators.ts

@@ -32,10 +32,21 @@ export function asLocator(lang: Language, selector: string, isFrameLocator: bool
 }
 
 function innerAsLocator(factory: LocatorFactory, parsed: ParsedSelector, isFrameLocator: boolean = false): string {
+  const parts = [...parsed.parts];
+  // frameLocator('iframe').first is actually "iframe >> nth=0 >> internal:control=enter-frame"
+  // To make it easier to parse, we turn it into "iframe >> internal:control=enter-frame >> nth=0"
+  for (let index = 0; index < parts.length - 1; index++) {
+    if (parts[index].name === 'nth' && parts[index + 1].name === 'internal:control' && (parts[index + 1].body as string) === 'enter-frame') {
+      // Swap nth and enter-frame.
+      const [nth] = parts.splice(index, 1);
+      parts.splice(index + 1, 0, nth);
+    }
+  }
+
   const tokens: string[] = [];
   let nextBase: LocatorBase = isFrameLocator ? 'frame-locator' : 'page';
-  for (let index = 0; index < parsed.parts.length; index++) {
-    const part = parsed.parts[index];
+  for (let index = 0; index < parts.length; index++) {
+    const part = parts[index];
     const base = nextBase;
     nextBase = 'locator';
 
@@ -111,7 +122,7 @@ function innerAsLocator(factory: LocatorFactory, parsed: ParsedSelector, isFrame
 
     let locatorType: LocatorType = 'default';
 
-    const nextPart = parsed.parts[index + 1];
+    const nextPart = parts[index + 1];
     if (nextPart && nextPart.name === 'internal:control' && (nextPart.body as string) === 'enter-frame') {
       locatorType = 'frame';
       nextBase = 'frame-locator';

packages/playwright-core/src/server/isomorphic/locatorParser.ts

@@ -152,8 +152,19 @@ function transform(template: string, params: TemplateParams, testIdAttributeName
       .replace(/,exact=true/g, 's')
       .replace(/\,/g, '][');
 
+  const parts = template.split('.');
+  // Turn "internal:control=enter-frame >> nth=0" into "nth=0 >> internal:control=enter-frame"
+  // because these are swapped in locators vs selectors.
+  for (let index = 0; index < parts.length - 1; index++) {
+    if (parts[index] === 'internal:control=enter-frame' && parts[index + 1].startsWith('nth=')) {
+      // Swap nth and enter-frame.
+      const [nth] = parts.splice(index, 1);
+      parts.splice(index + 1, 0, nth);
+    }
+  }
+
   // Substitute params.
-  return template.split('.').map(t => {
+  return parts.map(t => {
     if (!t.startsWith('internal:') || t === 'internal:control')
       return t.replace(/\$(\d+)/g, (_, ordinal) => { const param = params[+ordinal - 1]; return param.text; });
     t = t.includes('[') ? t.replace(/\]/, '') + ']' : t;

tests/library/locator-generator.spec.ts

@@ -316,14 +316,14 @@ it('reverse engineer frameLocator', async ({ page }) => {
   const locator = page
       .frameLocator('iframe')
       .getByText('foo', { exact: true })
-      .frameLocator('frame')
+      .frameLocator('frame').first()
       .frameLocator('iframe')
       .locator('span');
   expect.soft(generate(locator)).toEqual({
-    csharp: `FrameLocator("iframe").GetByText("foo", new() { Exact = true }).FrameLocator("frame").FrameLocator("iframe").Locator("span")`,
-    java: `frameLocator("iframe").getByText("foo", new FrameLocator.GetByTextOptions().setExact(true)).frameLocator("frame").frameLocator("iframe").locator("span")`,
-    javascript: `frameLocator('iframe').getByText('foo', { exact: true }).frameLocator('frame').frameLocator('iframe').locator('span')`,
-    python: `frame_locator("iframe").get_by_text("foo", exact=True).frame_locator("frame").frame_locator("iframe").locator("span")`,
+    csharp: `FrameLocator("iframe").GetByText("foo", new() { Exact = true }).FrameLocator("frame").First.FrameLocator("iframe").Locator("span")`,
+    java: `frameLocator("iframe").getByText("foo", new FrameLocator.GetByTextOptions().setExact(true)).frameLocator("frame").first().frameLocator("iframe").locator("span")`,
+    javascript: `frameLocator('iframe').getByText('foo', { exact: true }).frameLocator('frame').first().frameLocator('iframe').locator('span')`,
+    python: `frame_locator("iframe").get_by_text("foo", exact=True).frame_locator("frame").first.frame_locator("iframe").locator("span")`,
   });
 
   // Note that frame locators with ">>" are not restored back due to ambiguity.

tests/page/locator-frame.spec.ts

@@ -97,8 +97,8 @@ it('should work for $ and $$', async ({ page, server }) => {
 
 it('should wait for frame', async ({ page, server }) => {
   await page.goto(server.EMPTY_PAGE);
-  const error = await page.frameLocator('iframe').locator('span').click({ timeout: 1000 }).catch(e => e);
-  expect(error.message).toContain('waiting for frameLocator(\'iframe\')');
+  const error = await page.locator('body').frameLocator('iframe').locator('span').click({ timeout: 1000 }).catch(e => e);
+  expect(error.message).toContain(`waiting for locator('body').frameLocator('iframe')`);
 });
 
 it('should wait for frame 2', async ({ page, server }) => {