From 5821c547aacfd542259c8a9707b1eee119f0b692 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 22 Jun 2023 08:34:08 -0700 Subject: [PATCH] fix(selector generator): use the same label definition as getByLabel (#23846) This extracts `getElementLabels` helper function to be used both for generating and querying. --- .../src/server/injected/injectedScript.ts | 14 +++---------- .../src/server/injected/selectorGenerator.ts | 15 +++++++------- .../src/server/injected/selectorUtils.ts | 19 ++++++++++++++++++ tests/library/selector-generator.spec.ts | 20 +++++++++++++++---- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 752bab77db..0e425efbb5 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -22,14 +22,14 @@ import { createRoleEngine } from './roleSelectorEngine'; import { parseAttributeSelector } from '../../utils/isomorphic/selectorParser'; import type { NestedSelectorBody, ParsedSelector, ParsedSelectorPart } from '../../utils/isomorphic/selectorParser'; import { visitAllSelectorParts, parseSelector, stringifySelector } from '../../utils/isomorphic/selectorParser'; -import { type TextMatcher, elementMatchesText, elementText, type ElementText } from './selectorUtils'; +import { type TextMatcher, elementMatchesText, elementText, type ElementText, getElementLabels } from './selectorUtils'; import { SelectorEvaluatorImpl, sortInDOMOrder } from './selectorEvaluator'; import { enclosingShadowRootOrDocument, isElementVisible, parentElementOrShadowHost } from './domUtils'; import type { CSSComplexSelectorList } from '../../utils/isomorphic/cssParser'; import { generateSelector, type GenerateSelectorOptions } from './selectorGenerator'; import type * as channels from '@protocol/channels'; import { Highlight } from './highlight'; -import { getChecked, getAriaDisabled, getAriaLabelledByElements, getAriaRole, getElementAccessibleName } from './roleUtils'; +import { getChecked, getAriaDisabled, getAriaRole, getElementAccessibleName } from './roleUtils'; import { kLayoutSelectorNames, type LayoutSelectorName, layoutSelectorScore } from './layoutSelectorUtils'; import { asLocator } from '../../utils/isomorphic/locatorGenerators'; import type { Language } from '../../utils/isomorphic/locatorGenerators'; @@ -325,15 +325,7 @@ export class InjectedScript { const { matcher } = createTextMatcher(selector, true); const allElements = this._evaluator._queryCSS({ scope: root as Document | Element, pierceShadow: true }, '*'); return allElements.filter(element => { - let labels: Element[] | NodeListOf | null | undefined = getAriaLabelledByElements(element); - if (labels === null) { - const ariaLabel = element.getAttribute('aria-label'); - if (ariaLabel !== null && !!ariaLabel.trim()) - return matcher({ full: ariaLabel, immediate: [ariaLabel] }); - } - if (labels === null) - labels = (element as HTMLInputElement).labels; - return !!labels && [...labels].some(label => matcher(elementText(this._evaluator._cacheText, label))); + return getElementLabels(this._evaluator._cacheText, element).some(label => matcher(label)); }); } }; diff --git a/packages/playwright-core/src/server/injected/selectorGenerator.ts b/packages/playwright-core/src/server/injected/selectorGenerator.ts index a2917faf44..5e07739954 100644 --- a/packages/playwright-core/src/server/injected/selectorGenerator.ts +++ b/packages/playwright-core/src/server/injected/selectorGenerator.ts @@ -18,7 +18,7 @@ import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalize import { closestCrossShadow, isInsideScope, parentElementOrShadowHost } from './domUtils'; import type { InjectedScript } from './injectedScript'; import { getAriaRole, getElementAccessibleName, beginAriaCaches, endAriaCaches } from './roleUtils'; -import { elementText } from './selectorUtils'; +import { elementText, getElementLabels } from './selectorUtils'; type SelectorToken = { engine: string; @@ -214,12 +214,13 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element, candidates.push({ engine: 'internal:attr', selector: `[placeholder=${escapeForAttributeSelector(input.placeholder, false)}]`, score: kPlaceholderScore }); candidates.push({ engine: 'internal:attr', selector: `[placeholder=${escapeForAttributeSelector(input.placeholder, true)}]`, score: kPlaceholderScoreExact }); } - const label = input.labels?.[0]; - if (label) { - const labelText = elementText(injectedScript._evaluator._cacheText, label).full.trim(); - candidates.push({ engine: 'internal:label', selector: escapeForTextSelector(labelText, false), score: kLabelScore }); - candidates.push({ engine: 'internal:label', selector: escapeForTextSelector(labelText, true), score: kLabelScoreExact }); - } + } + + const labels = getElementLabels(injectedScript._evaluator._cacheText, element); + for (const label of labels) { + const labelText = label.full.trim(); + candidates.push({ engine: 'internal:label', selector: escapeForTextSelector(labelText, false), score: kLabelScore }); + candidates.push({ engine: 'internal:label', selector: escapeForTextSelector(labelText, true), score: kLabelScoreExact }); } const ariaRole = getAriaRole(element); diff --git a/packages/playwright-core/src/server/injected/selectorUtils.ts b/packages/playwright-core/src/server/injected/selectorUtils.ts index e05fb5ae29..45e27451ef 100644 --- a/packages/playwright-core/src/server/injected/selectorUtils.ts +++ b/packages/playwright-core/src/server/injected/selectorUtils.ts @@ -15,6 +15,7 @@ */ import type { AttributeSelectorPart } from '../../utils/isomorphic/selectorParser'; +import { getAriaLabelledByElements } from './roleUtils'; export function matchesComponentAttribute(obj: any, attr: AttributeSelectorPart) { for (const token of attr.jsonPath) { @@ -103,3 +104,21 @@ export function elementMatchesText(cache: Map return 'selfAndChildren'; return 'self'; } + +export function getElementLabels(textCache: Map, element: Element): ElementText[] { + const labels = getAriaLabelledByElements(element); + if (labels) + return labels.map(label => elementText(textCache, label)); + const ariaLabel = element.getAttribute('aria-label'); + if (ariaLabel !== null && !!ariaLabel.trim()) + return [{ full: ariaLabel, immediate: [ariaLabel] }]; + + // https://html.spec.whatwg.org/multipage/forms.html#category-label + const isNonHiddenInput = element.nodeName === 'INPUT' && (element as HTMLInputElement).type !== 'hidden'; + if (['BUTTON', 'METER', 'OUTPUT', 'PROGRESS', 'SELECT', 'TEXTAREA'].includes(element.nodeName) || isNonHiddenInput) { + const labels = (element as HTMLInputElement).labels; + if (labels) + return [...labels].map(label => elementText(textCache, label)); + } + return []; +} diff --git a/tests/library/selector-generator.spec.ts b/tests/library/selector-generator.spec.ts index acc9759c9f..44093d0a5e 100644 --- a/tests/library/selector-generator.spec.ts +++ b/tests/library/selector-generator.spec.ts @@ -355,7 +355,7 @@ it.describe('selector generator', () => { expect(await generate(page, 'ng\\:switch')).toBe('ng\\:switch'); await page.setContent(``); - await page.$eval('button', button => button.setAttribute('aria-label', `!#'!?:`)); + await page.$eval('span', span => span.textContent = `!#'!?:`); expect(await generate(page, 'button')).toBe(`internal:role=button[name="!#'!?:"i]`); expect(await page.$(`role=button[name="!#'!?:"]`)).toBeTruthy(); @@ -380,7 +380,7 @@ it.describe('selector generator', () => { it('should accept valid aria-label for candidate consideration', async ({ page }) => { await page.setContent(``); - expect(await generate(page, 'button')).toBe('internal:role=button[name=\"ariaLabel\"i]'); + expect(await generate(page, 'button')).toBe('internal:label="ariaLabel"i'); }); it('should ignore empty role for candidate consideration', async ({ page }) => { @@ -404,8 +404,20 @@ it.describe('selector generator', () => { }); it('should generate label selector', async ({ page }) => { - await page.setContent(``); - expect(await generate(page, 'input')).toBe('internal:label="Country"i'); + await page.setContent(` + + + + 70% + +
text
+ `); + expect(await generate(page, '#target1')).toBe('internal:label="Target1"i'); + expect(await generate(page, '#target2')).toBe('internal:label="Target2"i'); + expect(await generate(page, '#target3')).toBe('internal:label="Target3"i'); + expect(await generate(page, '#target4')).toBe('internal:label="Target4"i'); + expect(await generate(page, '#target5')).toBe('#target5'); + expect(await generate(page, '#target6')).toBe('internal:text="text"i'); await page.setContent(``); expect(await generate(page, 'input')).toBe('internal:label="Coun\\\"try"i');