Skip to content

Commit

Permalink
fix(inspector): highlight xpath/css locators without engine prefix (#…
Browse files Browse the repository at this point in the history
…27742)

Motivation: As of today when a user inspects a Locator which is a xpath,
it won't work if the user has not prefixed it with `xpath=` because we
internally compare the given with the generated locator.

Works: `locator('xpath=//div[contains(@Class, "foo")]')`
Does not work: `locator('//div[contains(@Class, "foo")]')`

Relates
#27707 (comment)
Fixes
microsoft/playwright-dotnet#2718 (comment)

---------

Signed-off-by: Max Schmitt <max@schmitt.mx>
  • Loading branch information
mxschmitt authored Oct 23, 2023
1 parent 9af667b commit f48861d
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,14 @@ function innerAsLocators(factory: LocatorFactory, parsed: ParsedSelector, isFram
}
}

tokens.push([locatorPart]);
// Selectors can be prefixed with engine name, e.g. xpath=//foo
let locatorPartWithEngine: string | undefined;
if (['xpath', 'css'].includes(part.name)) {
const selectorPart = stringifySelector({ parts: [part] }, /* forceEngineName */ true);
locatorPartWithEngine = factory.generateLocator(base, locatorType, selectorPart);
}

tokens.push([locatorPart, locatorPartWithEngine].filter(Boolean) as string[]);
}

return combineTokens(factory, tokens, maxOutputSize);
Expand Down
11 changes: 9 additions & 2 deletions packages/playwright-core/src/utils/isomorphic/selectorParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,18 @@ function selectorPartsEqual(list1: ParsedSelectorPart[], list2: ParsedSelectorPa
return stringifySelector({ parts: list1 }) === stringifySelector({ parts: list2 });
}

export function stringifySelector(selector: string | ParsedSelector): string {
export function stringifySelector(selector: string | ParsedSelector, forceEngineName?: boolean): string {
if (typeof selector === 'string')
return selector;
return selector.parts.map((p, i) => {
const prefix = p.name === 'css' ? '' : p.name + '=';
let includeEngine = true;
if (!forceEngineName && i !== selector.capture) {
if (p.name === 'css')
includeEngine = false;
else if (p.name === 'xpath' && p.source.startsWith('//') || p.source.startsWith('..'))
includeEngine = false;
}
const prefix = includeEngine ? p.name + '=' : '';
return `${i === selector.capture ? '*' : ''}${prefix}${p.source}`;
}).join(' >> ');
}
Expand Down
29 changes: 25 additions & 4 deletions tests/library/locator-generator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,10 +503,20 @@ it('asLocator internal:chain', async () => {

it('asLocator xpath', async () => {
const selector = `//*[contains(normalizer-text(), 'foo']`;
expect.soft(asLocator('javascript', selector, false)).toBe(`locator('xpath=//*[contains(normalizer-text(), \\'foo\\']')`);
expect.soft(asLocator('python', selector, false)).toBe(`locator(\"xpath=//*[contains(normalizer-text(), 'foo']\")`);
expect.soft(asLocator('java', selector, false)).toBe(`locator(\"xpath=//*[contains(normalizer-text(), 'foo']\")`);
expect.soft(asLocator('csharp', selector, false)).toBe(`Locator(\"xpath=//*[contains(normalizer-text(), 'foo']\")`);
expect.soft(asLocator('javascript', selector, false)).toBe(`locator('//*[contains(normalizer-text(), \\'foo\\']')`);
expect.soft(asLocator('python', selector, false)).toBe(`locator(\"//*[contains(normalizer-text(), 'foo']\")`);
expect.soft(asLocator('java', selector, false)).toBe(`locator(\"//*[contains(normalizer-text(), 'foo']\")`);
expect.soft(asLocator('csharp', selector, false)).toBe(`Locator(\"//*[contains(normalizer-text(), 'foo']\")`);
expect.soft(parseLocator('javascript', `locator('//*[contains(normalizer-text(), \\'foo\\']')`, 'data-testid')).toBe("//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('javascript', `locator("//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('javascript', `locator('xpath=//*[contains(normalizer-text(), \\'foo\\']')`, 'data-testid')).toBe("xpath=//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('javascript', `locator("xpath=//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("xpath=//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('python', `locator("//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('python', `locator("xpath=//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("xpath=//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('java', `locator("//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('java', `locator("xpath=//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("xpath=//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('csharp', `Locator("//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("//*[contains(normalizer-text(), 'foo']");
expect.soft(parseLocator('csharp', `Locator("xpath=//*[contains(normalizer-text(), 'foo']")`, 'data-testid')).toBe("xpath=//*[contains(normalizer-text(), 'foo']");
});

it('parseLocator quotes', async () => {
Expand All @@ -521,6 +531,17 @@ it('parseLocator quotes', async () => {
expect.soft(parseLocator('csharp', `Locator('text="bar"')`, '')).toBe(``);
});

it('parseLocator css', async () => {
expect.soft(parseLocator('javascript', `locator('.foo')`, '')).toBe(`.foo`);
expect.soft(parseLocator('javascript', `locator('css=.foo')`, '')).toBe(`css=.foo`);
expect.soft(parseLocator('python', `locator(".foo")`, '')).toBe(`.foo`);
expect.soft(parseLocator('python', `locator("css=.foo")`, '')).toBe(`css=.foo`);
expect.soft(parseLocator('java', `locator(".foo")`, '')).toBe(`.foo`);
expect.soft(parseLocator('java', `locator("css=.foo")`, '')).toBe(`css=.foo`);
expect.soft(parseLocator('csharp', `Locator(".foo")`, '')).toBe(`.foo`);
expect.soft(parseLocator('csharp', `Locator("css=.foo")`, '')).toBe(`css=.foo`);
});

it('parse locators strictly', () => {
const selector = 'div >> internal:has-text=\"Goodbye world\"i >> span';

Expand Down
2 changes: 1 addition & 1 deletion tests/page/page-wait-for-selector-2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ it('should respect timeout xpath', async ({ page, playwright }) => {
await page.waitForSelector('//div', { state: 'attached', timeout: 3000 }).catch(e => error = e);
expect(error).toBeTruthy();
expect(error.message).toContain('page.waitForSelector: Timeout 3000ms exceeded');
expect(error.message).toContain('waiting for locator(\'xpath=//div\')');
expect(error.message).toContain('waiting for locator(\'//div\')');
expect(error).toBeInstanceOf(playwright.errors.TimeoutError);
});

Expand Down

0 comments on commit f48861d

Please sign in to comment.