From f48861ddeeb3ea7d46e485836c7320980c537bdd Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Mon, 23 Oct 2023 18:23:28 +0200 Subject: [PATCH] fix(inspector): highlight xpath/css locators without engine prefix (#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 https://github.com/microsoft/playwright/issues/27707#issue-1952360264 Fixes https://github.com/microsoft/playwright-dotnet/issues/2718#issuecomment-1771073816 --------- Signed-off-by: Max Schmitt --- .../src/utils/isomorphic/locatorGenerators.ts | 9 +++++- .../src/utils/isomorphic/selectorParser.ts | 11 +++++-- tests/library/locator-generator.spec.ts | 29 ++++++++++++++++--- tests/page/page-wait-for-selector-2.spec.ts | 2 +- 4 files changed, 43 insertions(+), 8 deletions(-) diff --git a/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts b/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts index 8de433b4904fc..3715537f1dbc6 100644 --- a/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts +++ b/packages/playwright-core/src/utils/isomorphic/locatorGenerators.ts @@ -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); diff --git a/packages/playwright-core/src/utils/isomorphic/selectorParser.ts b/packages/playwright-core/src/utils/isomorphic/selectorParser.ts index 68fa9f26293f2..64ba3c0c4df14 100644 --- a/packages/playwright-core/src/utils/isomorphic/selectorParser.ts +++ b/packages/playwright-core/src/utils/isomorphic/selectorParser.ts @@ -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(' >> '); } diff --git a/tests/library/locator-generator.spec.ts b/tests/library/locator-generator.spec.ts index 9b77403c9e452..1e8f575fcf0bb 100644 --- a/tests/library/locator-generator.spec.ts +++ b/tests/library/locator-generator.spec.ts @@ -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 () => { @@ -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'; diff --git a/tests/page/page-wait-for-selector-2.spec.ts b/tests/page/page-wait-for-selector-2.spec.ts index f99ee7142bfa1..eb71c784cf4d5 100644 --- a/tests/page/page-wait-for-selector-2.spec.ts +++ b/tests/page/page-wait-for-selector-2.spec.ts @@ -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); });