Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/frames.ts
Expand Up @@ -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>;
Expand Down
Expand Up @@ -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';

Expand Down Expand Up @@ -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';
Expand Down
13 changes: 12 additions & 1 deletion packages/playwright-core/src/server/isomorphic/locatorParser.ts
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions tests/library/locator-generator.spec.ts
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions tests/page/locator-frame.spec.ts
Expand Up @@ -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 }) => {
Expand Down