Skip to content

Commit

Permalink
fix(role): embedded control inside the target element
Browse files Browse the repository at this point in the history
According to the spec, such controls should use the native value
as long as they have "aria-label". The relevant spec section is 2D.

However, there is an open issue that claims this should always apply,
and all browsers actually do that: w3c/accname#64
  • Loading branch information
dgozman committed Apr 17, 2024
1 parent c3e72be commit 488635c
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 7 deletions.
23 changes: 19 additions & 4 deletions packages/playwright-core/src/server/injected/roleUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,10 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN

const labelledBy = getAriaLabelledByElements(element);

// step 2b.
// step 2b. LabelledBy:
// Otherwise, if the current node has an aria-labelledby attribute that contains
// at least one valid IDREF, and the current node is not already part of an ongoing
// aria-labelledby or aria-describedby traversal, process its IDREFs in the order they occur...
if (!options.embeddedInLabelledBy) {
const accessibleName = (labelledBy || []).map(ref => getElementAccessibleNameInternal(ref, {
...options,
Expand All @@ -448,9 +451,15 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN

const role = getAriaRole(element) || '';

// step 2c.
// TODO: should we check embeddedInLabel here?
if (!!options.embeddedInLabel || !!options.embeddedInLabelledBy) {
// step 2c:
// if the current node is a control embedded within the label (e.g. any element directly referenced by aria-labelledby) for another widget...
//
// also step 2d "skip to rule Embedded Control" section:
// If traversal of the current node is due to recursion and the current node is an embedded control...
// Note this is not strictly by the spec, because spec only applies this logic when "aria-label" is present.
// However, browsers and and wpt test name_heading-combobox-focusable-alternative-manual.html follow this behavior,
// and there is an issue filed for this: https://github.com/w3c/accname/issues/64
if (!!options.embeddedInLabel || !!options.embeddedInLabelledBy || options.embeddedInTargetElement === 'descendant') {
const isOwnLabel = [...(element as (HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement)).labels || []].includes(element as any);
const isOwnLabelledBy = (labelledBy || []).includes(element);
if (!isOwnLabel && !isOwnLabelledBy) {
Expand All @@ -471,6 +480,12 @@ function getElementAccessibleNameInternal(element: Element, options: AccessibleN
const listbox = role === 'combobox' ? queryInAriaOwned(element, '*').find(e => getAriaRole(e) === 'listbox') : element;
selectedOptions = listbox ? queryInAriaOwned(listbox, '[aria-selected="true"]').filter(e => getAriaRole(e) === 'option') : [];
}
if (!selectedOptions.length && element.tagName === 'INPUT') {
// SPEC DIFFERENCE:
// This fallback is not explicitly mentioned in the spec, but all browsers and
// wpt test name_heading-combobox-focusable-alternative-manual.html do this.
return (element as HTMLInputElement).value;
}
return selectedOptions.map(option => getElementAccessibleNameInternal(option, childOptions)).join(' ');
}
if (['progressbar', 'scrollbar', 'slider', 'spinbutton', 'meter'].includes(role)) {
Expand Down
8 changes: 7 additions & 1 deletion tests/assets/axe-core/accessible-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,13 @@ module.exports = [
'<input type="text" id="tb1"></div>' +
'<label for="tb1">My form input</label>',
target: '#target',
accessibleText: 'My form input',
// accessibleText: 'My form input',
// All browsers and the spec (kind of) agree that input inside the target element should
// use it's value as an "embedded control", rather than a label.
// From the spec:
// If traversal of the current node is due to recursion and the current node
// is an embedded control, ignore aria-label and skip to rule Embedded Control.
accessibleText: '',
},

{
Expand Down
47 changes: 45 additions & 2 deletions tests/library/role-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ const ranges = [
for (let range = 0; range <= ranges.length; range++) {
test('wpt accname #' + range, async ({ page, asset, server, browserName }) => {
const skipped = [
// Spec clearly says to only use control's value when embedded in a label (step 2C).
'name_heading-combobox-focusable-alternative-manual.html',
// This test expects ::before + title + ::after, which is neither 2F nor 2I.
'name_test_case_659-manual.html',
// This test expects ::before + title + ::after, which is neither 2F nor 2I.
Expand Down Expand Up @@ -307,6 +305,8 @@ test('display:contents should be visible when contents are visible', async ({ pa
});

test('label/labelled-by aria-hidden with descendants', async ({ page }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29796' });

await page.setContent(`
<body>
<div id="case1">
Expand All @@ -326,6 +326,49 @@ test('label/labelled-by aria-hidden with descendants', async ({ page }) => {
expect.soft(await getNameAndRole(page, '#case2 button')).toEqual({ role: 'button', name: 'Label2' });
});

test('own aria-label concatenated with aria-labelledby', async ({ page }) => {
await page.setContent(`
<h1>Files</h1>
<ul>
<li>
<a id="file_row1" href="./files/Documentation.pdf">Documentation.pdf</a>
<span role="button" tabindex="0" id="del_row1" aria-label="Delete" aria-labelledby="del_row1 file_row1"></span>
</li>
<li>
<a id="file_row2" href="./files/HolidayLetter.pdf">HolidayLetter.pdf</a>
<span role="button" tabindex="0" id="del_row2" aria-label="Delete" aria-labelledby="del_row2 file_row2"></span>
</li>
</ul>
`);
expect.soft(await getNameAndRole(page, '#del_row1')).toEqual({ role: 'button', name: 'Delete Documentation.pdf' });
expect.soft(await getNameAndRole(page, '#del_row2')).toEqual({ role: 'button', name: 'Delete HolidayLetter.pdf' });
});

test('control embedded in a label', async ({ page }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28848' });

await page.setContent(`
<label for="flash">
<input type="checkbox" id="flash">
Flash the screen <span tabindex="0" role="textbox" aria-label="number of times" contenteditable>5</span> times.
</label>
`);
expect.soft(await getNameAndRole(page, 'input')).toEqual({ role: 'checkbox', name: 'Flash the screen 5 times.' });
expect.soft(await getNameAndRole(page, 'span')).toEqual({ role: 'textbox', name: 'number of times' });
expect.soft(await getNameAndRole(page, 'label')).toEqual({ role: null, name: '' });
});

test('control embedded in a target element', async ({ page }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28848' });

await page.setContent(`
<h1>
<input type="text" value="Foo bar">
</h1>
`);
expect.soft(await getNameAndRole(page, 'h1')).toEqual({ role: 'heading', name: 'Foo bar' });
});

function toArray(x: any): any[] {
return Array.isArray(x) ? x : [x];
}

0 comments on commit 488635c

Please sign in to comment.