From b72e3a3eba39499f7ee807417ec846ff4e8db17d Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 17 Apr 2024 11:22:09 -0700 Subject: [PATCH] fix(role): explicitly hidden aria-labelledby should be recursively traversed (#30402) The accessible name computation spec has changed to explicitly mention this case: Step 2A. Hidden Not Referenced. If the current node is hidden and is: - Not part of an aria-labelledby or aria-describedby traversal, where the node directly referenced by that relation was hidden. - Nor part of a native host language text alternative element (e.g. label in HTML) or attribute traversal, where the root of that traversal was hidden. See https://w3c.github.io/accname/#computation-steps. Chromium, Firefox and Safari all agree with the spec. Fixes #29796. --- .../src/server/injected/roleUtils.ts | 55 ++++++++++--------- tests/assets/axe-core/accessible-text.js | 5 +- tests/library/role-utils.spec.ts | 20 +++++++ 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/packages/playwright-core/src/server/injected/roleUtils.ts b/packages/playwright-core/src/server/injected/roleUtils.ts index 6cecb1d5a5..56a2833548 100644 --- a/packages/playwright-core/src/server/injected/roleUtils.ts +++ b/packages/playwright-core/src/server/injected/roleUtils.ts @@ -389,9 +389,9 @@ export function getElementAccessibleName(element: Element, includeHidden: boolea accessibleName = normalizeAccessbileName(getElementAccessibleNameInternal(element, { includeHidden, visitedElements: new Set(), - embeddedInLabelledBy: 'none', - embeddedInLabel: 'none', - embeddedInTextAlternativeElement: false, + embeddedInLabelledBy: undefined, + embeddedInLabel: undefined, + embeddedInNativeTextAlternative: undefined, embeddedInTargetElement: 'self', })); } @@ -404,9 +404,9 @@ export function getElementAccessibleName(element: Element, includeHidden: boolea type AccessibleNameOptions = { includeHidden: boolean, visitedElements: Set, - embeddedInLabelledBy: 'none' | 'self' | 'descendant', - embeddedInLabel: 'none' | 'self' | 'descendant', - embeddedInTextAlternativeElement: boolean, + embeddedInLabelledBy: { element: Element, hidden: boolean } | undefined, + embeddedInLabel: { element: Element, hidden: boolean } | undefined, + embeddedInNativeTextAlternative: { element: Element, hidden: boolean } | undefined, embeddedInTargetElement: 'none' | 'self' | 'descendant', }; @@ -416,13 +416,17 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN const childOptions: AccessibleNameOptions = { ...options, - embeddedInLabel: options.embeddedInLabel === 'self' ? 'descendant' : options.embeddedInLabel, - embeddedInLabelledBy: options.embeddedInLabelledBy === 'self' ? 'descendant' : options.embeddedInLabelledBy, embeddedInTargetElement: options.embeddedInTargetElement === 'self' ? 'descendant' : options.embeddedInTargetElement, }; - // step 2a. - if (!options.includeHidden && options.embeddedInLabelledBy !== 'self' && isElementHiddenForAria(element)) { + // step 2a. Hidden Not Referenced: If the current node is hidden and is: + // Not part of an aria-labelledby or aria-describedby traversal, where the node directly referenced by that relation was hidden. + // Nor part of a native host language text alternative element (e.g. label in HTML) or attribute traversal, where the root of that traversal was hidden. + if (!options.includeHidden && + !options.embeddedInLabelledBy?.hidden && + !options?.embeddedInNativeTextAlternative?.hidden && + !options?.embeddedInLabel?.hidden && + isElementHiddenForAria(element)) { options.visitedElements.add(element); return ''; } @@ -430,13 +434,13 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN const labelledBy = getAriaLabelledByElements(element); // step 2b. - if (options.embeddedInLabelledBy === 'none') { + if (!options.embeddedInLabelledBy) { const accessibleName = (labelledBy || []).map(ref => getElementAccessibleNameInternal(ref, { ...options, - embeddedInLabelledBy: 'self', + embeddedInLabelledBy: { element: ref, hidden: isElementHiddenForAria(ref) }, embeddedInTargetElement: 'none', - embeddedInLabel: 'none', - embeddedInTextAlternativeElement: false, + embeddedInLabel: undefined, + embeddedInNativeTextAlternative: undefined, })).join(' '); if (accessibleName) return accessibleName; @@ -445,7 +449,8 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN const role = getAriaRole(element) || ''; // step 2c. - if (options.embeddedInLabel !== 'none' || options.embeddedInLabelledBy !== 'none') { + // TODO: should we check embeddedInLabel here? + if (!!options.embeddedInLabel || !!options.embeddedInLabelledBy) { const isOwnLabel = [...(element as (HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement)).labels || []].includes(element as any); const isOwnLabelledBy = (labelledBy || []).includes(element); if (!isOwnLabel && !isOwnLabelledBy) { @@ -519,7 +524,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN if (element.tagName === 'INPUT' && (element as HTMLInputElement).type === 'image') { options.visitedElements.add(element); const labels = (element as HTMLInputElement).labels || []; - if (labels.length && options.embeddedInLabelledBy === 'none') + if (labels.length && !options.embeddedInLabelledBy) return getAccessibleNameFromAssociatedLabels(labels, options); const alt = element.getAttribute('alt') || ''; if (alt.trim()) @@ -576,7 +581,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN if (child.tagName === 'LEGEND') { return getElementAccessibleNameInternal(child, { ...childOptions, - embeddedInTextAlternativeElement: true, + embeddedInNativeTextAlternative: { element: child, hidden: isElementHiddenForAria(child) }, }); } } @@ -591,7 +596,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN if (child.tagName === 'FIGCAPTION') { return getElementAccessibleNameInternal(child, { ...childOptions, - embeddedInTextAlternativeElement: true, + embeddedInNativeTextAlternative: { element: child, hidden: isElementHiddenForAria(child) }, }); } } @@ -619,7 +624,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN if (child.tagName === 'CAPTION') { return getElementAccessibleNameInternal(child, { ...childOptions, - embeddedInTextAlternativeElement: true, + embeddedInNativeTextAlternative: { element: child, hidden: isElementHiddenForAria(child) }, }); } } @@ -650,7 +655,7 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN if (child.tagName.toUpperCase() === 'TITLE' && (child as SVGElement).ownerSVGElement) { return getElementAccessibleNameInternal(child, { ...childOptions, - embeddedInLabelledBy: 'self', + embeddedInLabelledBy: { element: child, hidden: isElementHiddenForAria(child) }, }); } } @@ -666,8 +671,8 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN // step 2f + step 2h. if (allowsNameFromContent(role, options.embeddedInTargetElement === 'descendant') || - options.embeddedInLabelledBy !== 'none' || options.embeddedInLabel !== 'none' || - options.embeddedInTextAlternativeElement) { + !!options.embeddedInLabelledBy || !!options.embeddedInLabel || + !!options.embeddedInNativeTextAlternative) { options.visitedElements.add(element); const tokens: string[] = []; const visit = (node: Node, skipSlotted: boolean) => { @@ -838,9 +843,9 @@ function hasExplicitAriaDisabled(element: Element | undefined): boolean { function getAccessibleNameFromAssociatedLabels(labels: Iterable, options: AccessibleNameOptions) { return [...labels].map(label => getElementAccessibleNameInternal(label, { ...options, - embeddedInLabel: 'self', - embeddedInTextAlternativeElement: false, - embeddedInLabelledBy: 'none', + embeddedInLabel: { element: label, hidden: isElementHiddenForAria(label) }, + embeddedInNativeTextAlternative: undefined, + embeddedInLabelledBy: undefined, embeddedInTargetElement: 'none', })).filter(accessibleName => !!accessibleName).join(' '); } diff --git a/tests/assets/axe-core/accessible-text.js b/tests/assets/axe-core/accessible-text.js index 9c84cd1cea..f8d17c1b5e 100644 --- a/tests/assets/axe-core/accessible-text.js +++ b/tests/assets/axe-core/accessible-text.js @@ -73,10 +73,7 @@ module.exports = [ '' + '', target: '#t1', - // accessibleText: 'This is a hidden secret', - // Note: axe-core insists on child nodes being used as visible, although - // spec 2A says "directly referenced by aria-labelledby". - accessibleText: 'This is a', + accessibleText: 'This is a hidden secret', }, { diff --git a/tests/library/role-utils.spec.ts b/tests/library/role-utils.spec.ts index b9d2c53923..e926018233 100644 --- a/tests/library/role-utils.spec.ts +++ b/tests/library/role-utils.spec.ts @@ -306,6 +306,26 @@ test('display:contents should be visible when contents are visible', async ({ pa await expect(page.getByRole('button')).toHaveCount(1); }); +test('label/labelled-by aria-hidden with descendants', async ({ page }) => { + await page.setContent(` + +
+ + +
+
+ + +
+ + `); + await page.$$eval('#label1, #label2', els => { + els.forEach(el => el.attachShadow({ mode: 'open' }).appendChild(document.createElement('slot'))); + }); + expect.soft(await getNameAndRole(page, '#case1 button')).toEqual({ role: 'button', name: 'Label1' }); + expect.soft(await getNameAndRole(page, '#case2 button')).toEqual({ role: 'button', name: 'Label2' }); +}); + function toArray(x: any): any[] { return Array.isArray(x) ? x : [x]; }