From 48c44f2c787221523badd666ba25162dc17da4fc Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 21 Oct 2022 16:29:45 -0700 Subject: [PATCH] fix(selectors): hasText and getByText exact match should consider full text (#18260) Fixes #18259. --- .../playwright-core/src/client/locator.ts | 6 ++--- .../src/server/injected/consoleApi.ts | 6 ++--- .../src/server/injected/injectedScript.ts | 25 +++++++++++++++---- .../src/server/injected/selectorGenerator.ts | 2 +- .../server/isomorphic/locatorGenerators.ts | 15 ++++------- .../playwright-core/src/server/selectors.ts | 2 +- tests/library/locator-generator.spec.ts | 2 +- tests/library/selector-generator.spec.ts | 8 +++--- tests/page/selectors-text.spec.ts | 20 +++++++++++++++ 9 files changed, 56 insertions(+), 30 deletions(-) diff --git a/packages/playwright-core/src/client/locator.ts b/packages/playwright-core/src/client/locator.ts index a8a518cbf6..b0a0339a44 100644 --- a/packages/playwright-core/src/client/locator.ts +++ b/packages/playwright-core/src/client/locator.ts @@ -50,10 +50,8 @@ export class Locator implements api.Locator { this._frame = frame; this._selector = selector; - if (options?.hasText) { - const textSelector = 'internal:text=' + escapeForTextSelector(options.hasText, false); - this._selector += ` >> internal:has=${JSON.stringify(textSelector)}`; - } + if (options?.hasText) + this._selector += ` >> internal:has-text=${escapeForTextSelector(options.hasText, false)}`; if (options?.has) { const locator = options.has; diff --git a/packages/playwright-core/src/server/injected/consoleApi.ts b/packages/playwright-core/src/server/injected/consoleApi.ts index a173ed7086..c728b233c8 100644 --- a/packages/playwright-core/src/server/injected/consoleApi.ts +++ b/packages/playwright-core/src/server/injected/consoleApi.ts @@ -26,10 +26,8 @@ function createLocator(injectedScript: InjectedScript, initial: string, options? constructor(selector: string, options?: { hasText?: string | RegExp, has?: Locator }) { this.selector = selector; - if (options?.hasText) { - const textSelector = 'internal:text=' + escapeForTextSelector(options.hasText, false); - this.selector += ` >> internal:has=${JSON.stringify(textSelector)}`; - } + if (options?.hasText) + this.selector += ` >> internal:has-text=${escapeForTextSelector(options.hasText, false)}`; if (options?.has) this.selector += ` >> internal:has=` + JSON.stringify(options.has.selector); const parsed = injectedScript.parseSelector(this.selector); diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 7152e5ac2a..183b3cabfb 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -111,6 +111,7 @@ export class InjectedScript { this._engines.set('internal:has', this._createHasEngine()); this._engines.set('internal:label', this._createInternalLabelEngine()); this._engines.set('internal:text', this._createTextEngine(true, true)); + this._engines.set('internal:has-text', this._createInternalHasTextEngine()); this._engines.set('internal:attr', this._createNamedAttributeEngine()); this._engines.set('internal:role', RoleEngine); @@ -250,7 +251,7 @@ export class InjectedScript { private _createTextEngine(shadow: boolean, internal: boolean): SelectorEngine { const queryList = (root: SelectorRoot, selector: string): Element[] => { - const { matcher, kind } = createTextMatcher(selector, false, internal); + const { matcher, kind } = createTextMatcher(selector, internal); const result: Element[] = []; let lastDidNotMatchSelf: Element | null = null; @@ -261,7 +262,7 @@ export class InjectedScript { const matches = elementMatchesText(this._evaluator._cacheText, element, matcher); if (matches === 'none') lastDidNotMatchSelf = element; - if (matches === 'self' || (matches === 'selfAndChildren' && kind === 'strict')) + if (matches === 'self' || (matches === 'selfAndChildren' && kind === 'strict' && !internal)) result.push(element); }; @@ -280,11 +281,25 @@ export class InjectedScript { }; } + private _createInternalHasTextEngine(): SelectorEngine { + const evaluator = this._evaluator; + return { + queryAll: (root: SelectorRoot, selector: string): Element[] => { + if (root.nodeType !== 1 /* Node.ELEMENT_NODE */) + return []; + const element = root as Element; + const text = elementText(evaluator._cacheText, element); + const { matcher } = createTextMatcher(selector, true); + return matcher(text) ? [element] : []; + } + }; + } + private _createInternalLabelEngine(): SelectorEngine { const evaluator = this._evaluator; return { queryAll: (root: SelectorRoot, selector: string): Element[] => { - const { matcher } = createTextMatcher(selector, true, true); + const { matcher } = createTextMatcher(selector, true); const result: Element[] = []; const labels = this._evaluator._queryCSS({ scope: root as Document | Element, pierceShadow: true }, 'label') as HTMLLabelElement[]; for (const label of labels) { @@ -1302,7 +1317,7 @@ function cssUnquote(s: string): string { return r.join(''); } -function createTextMatcher(selector: string, strictMatchesFullText: boolean, internal: boolean): { matcher: TextMatcher, kind: 'regex' | 'strict' | 'lax' } { +function createTextMatcher(selector: string, internal: boolean): { matcher: TextMatcher, kind: 'regex' | 'strict' | 'lax' } { if (selector[0] === '/' && selector.lastIndexOf('/') > 0) { const lastSlash = selector.lastIndexOf('/'); const matcher: TextMatcher = createRegexTextMatcher(selector.substring(1, lastSlash), selector.substring(lastSlash + 1)); @@ -1324,7 +1339,7 @@ function createTextMatcher(selector: string, strictMatchesFullText: boolean, int strict = true; } if (strict) - return { matcher: strictMatchesFullText ? createStrictFullTextMatcher(selector) : createStrictTextMatcher(selector), kind: 'strict' }; + return { matcher: internal ? createStrictFullTextMatcher(selector) : createStrictTextMatcher(selector), kind: 'strict' }; return { matcher: createLaxTextMatcher(selector), kind: 'lax' }; } diff --git a/packages/playwright-core/src/server/injected/selectorGenerator.ts b/packages/playwright-core/src/server/injected/selectorGenerator.ts index 3c65b2cd42..22914538f7 100644 --- a/packages/playwright-core/src/server/injected/selectorGenerator.ts +++ b/packages/playwright-core/src/server/injected/selectorGenerator.ts @@ -221,7 +221,7 @@ function buildTextCandidates(injectedScript: InjectedScript, element: Element, i } else { candidate.push({ engine: 'css', selector: element.nodeName.toLowerCase(), score: 10 }); } - candidate.push({ engine: 'internal:has', selector: JSON.stringify('internal:text=' + escaped), score: 0 }); + candidate.push({ engine: 'internal:has-text', selector: escaped, score: 0 }); candidates.push(candidate); return candidates; } diff --git a/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts b/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts index 4fa3c86ecd..75d3eb3092 100644 --- a/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts +++ b/packages/playwright-core/src/server/isomorphic/locatorGenerators.ts @@ -16,7 +16,6 @@ import { escapeWithQuotes, toSnakeCase, toTitleCase } from '../../utils/isomorphic/stringUtils'; import { parseAttributeSelector, parseSelector, stringifySelector } from '../isomorphic/selectorParser'; -import type { NestedSelectorBody } from '../isomorphic/selectorParser'; import type { ParsedSelector } from '../isomorphic/selectorParser'; export type Language = 'javascript' | 'python' | 'java' | 'csharp'; @@ -50,6 +49,11 @@ function innerAsLocator(factory: LocatorFactory, selector: string, isFrameLocato tokens.push(factory.generateLocator(base, 'text', text, { exact })); continue; } + if (part.name === 'internal:has-text') { + const { exact, text } = detectExact(part.body as string); + tokens.push(factory.generateLocator(base, 'has-text', text, { exact })); + continue; + } if (part.name === 'internal:label') { const { exact, text } = detectExact(part.body as string); tokens.push(factory.generateLocator(base, 'label', text, { exact })); @@ -63,15 +67,6 @@ function innerAsLocator(factory: LocatorFactory, selector: string, isFrameLocato tokens.push(factory.generateLocator(base, 'role', attrSelector.name, { attrs })); continue; } - if (part.name === 'internal:has') { - const nested = (part.body as NestedSelectorBody).parsed; - if (nested?.parts?.[0]?.name === 'internal:text') { - const result = detectExact(nested.parts[0].body as string); - tokens.push(factory.generateLocator(base, 'has-text', result.text, { exact: result.exact })); - continue; - } - } - if (part.name === 'internal:attr') { const attrSelector = parseAttributeSelector(part.body as string, true); const { name, value, caseSensitive } = attrSelector.attributes[0]; diff --git a/packages/playwright-core/src/server/selectors.ts b/packages/playwright-core/src/server/selectors.ts index 88c5d5dd40..a082f898a4 100644 --- a/packages/playwright-core/src/server/selectors.ts +++ b/packages/playwright-core/src/server/selectors.ts @@ -45,7 +45,7 @@ export class Selectors { 'data-testid', 'data-testid:light', 'data-test-id', 'data-test-id:light', 'data-test', 'data-test:light', - 'nth', 'visible', 'internal:control', 'internal:has', + 'nth', 'visible', 'internal:control', 'internal:has', 'internal:has-text', 'role', 'internal:attr', 'internal:label', 'internal:text', 'internal:role', ]); this._builtinEnginesInMainWorld = new Set([ diff --git a/tests/library/locator-generator.spec.ts b/tests/library/locator-generator.spec.ts index 5d8e24460f..65e701b395 100644 --- a/tests/library/locator-generator.spec.ts +++ b/tests/library/locator-generator.spec.ts @@ -190,7 +190,7 @@ it.describe('selector generator', () => { await (context as any)._enableRecorder({ language: 'javascript' }); }); - it('reverse engineer internal:has locators', async ({ page }) => { + it('reverse engineer internal:has-text locators', async ({ page }) => { await page.setContent(`
Hello world
Hello world diff --git a/tests/library/selector-generator.spec.ts b/tests/library/selector-generator.spec.ts index 57f0c0222d..efc2bceec0 100644 --- a/tests/library/selector-generator.spec.ts +++ b/tests/library/selector-generator.spec.ts @@ -129,13 +129,13 @@ it.describe('selector generator', () => { expect(await generate(page, 'div[mark="1"]')).toBe(`div >> nth=1`); }); - it('should use internal:has', async ({ page }) => { + it('should use internal:has-text', async ({ page }) => { await page.setContent(`
Hello world
Hello world Goodbye world `); - expect(await generate(page, 'a:has-text("Hello")')).toBe(`a >> internal:has=\"internal:text=\\\"Hello world\\\"i\"`); + expect(await generate(page, 'a:has-text("Hello")')).toBe(`a >> internal:has-text="Hello world"i`); }); it('should chain text after parent', async ({ page }) => { @@ -143,7 +143,7 @@ it.describe('selector generator', () => {
Hello world
Hello world `); - expect(await generate(page, '[mark="1"]')).toBe(`b >> internal:has=\"internal:text=\\\"Hello world\\\"i\" >> span`); + expect(await generate(page, '[mark="1"]')).toBe(`b >> internal:has-text="Hello world"i >> span`); }); it('should use parent text', async ({ page }) => { @@ -151,7 +151,7 @@ it.describe('selector generator', () => {
Hello world
Goodbye world
`); - expect(await generate(page, '[mark="1"]')).toBe(`div >> internal:has=\"internal:text=\\\"Goodbye world\\\"i\" >> span`); + expect(await generate(page, '[mark="1"]')).toBe(`div >> internal:has-text="Goodbye world"i >> span`); }); it('should separate selectors by >>', async ({ page }) => { diff --git a/tests/page/selectors-text.spec.ts b/tests/page/selectors-text.spec.ts index e32d556fa9..5cad1fc484 100644 --- a/tests/page/selectors-text.spec.ts +++ b/tests/page/selectors-text.spec.ts @@ -460,3 +460,23 @@ it('should work with paired quotes in the middle of selector', async ({ page }) // Should double escape inside quoted text. await expect(page.locator(`div >> text='pattern "^-?\\\\d+$"'`)).toBeVisible(); }); + +it('hasText and internal:text should match full node text in strict mode', async ({ page }) => { + await page.setContent(` +
helloworld
+
hello
+ `); + await expect(page.getByText('helloworld', { exact: true })).toHaveId('div1'); + await expect(page.getByText('hello', { exact: true })).toHaveId('div2'); + await expect(page.locator('div', { hasText: /^helloworld$/ })).toHaveId('div1'); + await expect(page.locator('div', { hasText: /^hello$/ })).toHaveId('div2'); + + await page.setContent(` +
helloworld
+
hello
+ `); + await expect(page.getByText('helloworld', { exact: true })).toHaveId('div1'); + expect(await page.getByText('hello', { exact: true }).evaluateAll(els => els.map(e => e.id))).toEqual(['span1', 'span2']); + await expect(page.locator('div', { hasText: /^helloworld$/ })).toHaveId('div1'); + await expect(page.locator('div', { hasText: /^hello$/ })).toHaveId('div2'); +});