From ba1a1bd99d7ee7a3d2809013a78f39f76f80e4fe Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 15 Nov 2022 10:50:46 -0800 Subject: [PATCH] cherry-pick(#18819): fix(locators): frameLocator().nth serialized correctly Fixes #18798. --- packages/playwright-core/src/server/frames.ts | 2 +- .../src/server/isomorphic/locatorGenerators.ts | 17 ++++++++++++++--- .../src/server/isomorphic/locatorParser.ts | 13 ++++++++++++- tests/library/locator-generator.spec.ts | 10 +++++----- tests/page/locator-frame.spec.ts | 4 ++-- 5 files changed, 34 insertions(+), 12 deletions(-) diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 0fe1c4fdabbcb..553ae51e5f579 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/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; diff --git a/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts b/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts index 9ce963f721c34..b76fa5c853c09 100644 --- a/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts +++ b/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'; diff --git a/packages/playwright-core/src/server/isomorphic/locatorParser.ts b/packages/playwright-core/src/server/isomorphic/locatorParser.ts index 1933cb2991035..76e0f31937f6c 100644 --- a/packages/playwright-core/src/server/isomorphic/locatorParser.ts +++ b/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; diff --git a/tests/library/locator-generator.spec.ts b/tests/library/locator-generator.spec.ts index 2835b107ca172..c684371461b77 100644 --- a/tests/library/locator-generator.spec.ts +++ b/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. diff --git a/tests/page/locator-frame.spec.ts b/tests/page/locator-frame.spec.ts index 06b19770c7873..2c7b13f88063a 100644 --- a/tests/page/locator-frame.spec.ts +++ b/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 }) => {