-
Notifications
You must be signed in to change notification settings - Fork 779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(aria-input-field-name): skip combobox popups #3886
Changes from all commits
d21d278
d735b52
a92c6af
427c9c1
50bf376
d3c0664
163911d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import getRole from './get-role'; | ||
import ariaAttrs from '../../standards/aria-attrs'; | ||
import { getRootNode } from '../../core/utils'; | ||
|
||
/** | ||
* Whether an element is the popup for a combobox | ||
* @method isComboboxPopup | ||
* @memberof axe.commons.aria | ||
* @instance | ||
* @param {VirtualNode} virtualNode | ||
* @param {Object} options | ||
* @property {String[]} popupRoles Overrides which roles can be popup. Defaults to aria-haspopup values | ||
* @returns {boolean} | ||
*/ | ||
export default function isComboboxPopup(virtualNode, { popupRoles } = {}) { | ||
const role = getRole(virtualNode); | ||
popupRoles ??= ariaAttrs['aria-haspopup'].values; | ||
if (!popupRoles.includes(role)) { | ||
return false; | ||
} | ||
|
||
// in ARIA 1.1 the container has role=combobox | ||
const vParent = nearestParentWithRole(virtualNode); | ||
if (isCombobox(vParent)) { | ||
return true; | ||
} | ||
|
||
const { id } = virtualNode.props; | ||
if (!id) { | ||
return false; | ||
} | ||
|
||
if (!virtualNode.actualNode) { | ||
throw new Error('Unable to determine combobox popup without an actualNode'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't think we needed additional virtualNode tests, but let me know if I missed anything. |
||
} | ||
const root = getRootNode(virtualNode.actualNode); | ||
const ownedCombobox = root.querySelectorAll( | ||
// aria-owns was from ARIA 1.0, aria-controls was from ARIA 1.2 | ||
`[aria-owns~="${id}"][role~="combobox"]:not(select), | ||
[aria-controls~="${id}"][role~="combobox"]:not(select)` | ||
); | ||
|
||
return Array.from(ownedCombobox).some(isCombobox); | ||
} | ||
|
||
const isCombobox = node => node && getRole(node) === 'combobox'; | ||
|
||
function nearestParentWithRole(vNode) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We were using |
||
while ((vNode = vNode.parent)) { | ||
straker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (getRole(vNode, { noPresentational: true }) !== null) { | ||
return vNode; | ||
} | ||
} | ||
return null; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,75 +1,21 @@ | ||
import { hasContentVirtual } from '../commons/dom'; | ||
import { getExplicitRole } from '../commons/aria'; | ||
import { | ||
querySelectorAll, | ||
getScroll, | ||
closest, | ||
getRootNode, | ||
tokenList | ||
} from '../core/utils'; | ||
import ariaAttrs from '../standards/aria-attrs'; | ||
|
||
function scrollableRegionFocusableMatches(node, virtualNode) { | ||
/** | ||
* Note: | ||
* `excludeHidden=true` for this rule, thus considering only elements in the accessibility tree. | ||
*/ | ||
|
||
/** | ||
* if not scrollable -> `return` | ||
*/ | ||
if (!!getScroll(node, 13) === false) { | ||
return false; | ||
} | ||
|
||
/** | ||
* ignore scrollable regions owned by combobox. limit to roles | ||
* ownable by combobox so we don't keep calling closest for every | ||
* node (which would be slow) | ||
* @see https://github.com/dequelabs/axe-core/issues/1763 | ||
*/ | ||
const role = getExplicitRole(virtualNode); | ||
if (ariaAttrs['aria-haspopup'].values.includes(role)) { | ||
// in ARIA 1.1 the container has role=combobox | ||
if (closest(virtualNode, '[role~="combobox"]')) { | ||
return false; | ||
} | ||
|
||
// in ARIA 1.0 and 1.2 the combobox owns (1.0) or controls (1.2) | ||
// the listbox | ||
const id = virtualNode.attr('id'); | ||
if (id) { | ||
const doc = getRootNode(node); | ||
const owned = Array.from( | ||
doc.querySelectorAll(`[aria-owns~="${id}"], [aria-controls~="${id}"]`) | ||
); | ||
const comboboxOwned = owned.some(el => { | ||
const roles = tokenList(el.getAttribute('role')); | ||
return roles.includes('combobox'); | ||
}); | ||
|
||
if (comboboxOwned) { | ||
return false; | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* check if node has visible contents | ||
*/ | ||
const nodeAndDescendents = querySelectorAll(virtualNode, '*'); | ||
const hasVisibleChildren = nodeAndDescendents.some(elm => | ||
hasContentVirtual( | ||
elm, | ||
true, // noRecursion | ||
true // ignoreAria | ||
) | ||
import hasContentVirtual from '../commons/dom/has-content-virtual'; | ||
import isComboboxPopup from '../commons/aria/is-combobox-popup'; | ||
import { querySelectorAll, getScroll } from '../core/utils'; | ||
|
||
export default function scrollableRegionFocusableMatches(node, virtualNode) { | ||
return ( | ||
// The element scrolls | ||
getScroll(node, 13) !== undefined && | ||
// It's not a combobox popup, which commonly has keyboard focus added | ||
isComboboxPopup(virtualNode) === false && | ||
// And there's something actually worth scrolling to | ||
isNoneEmptyElement(virtualNode) | ||
); | ||
if (!hasVisibleChildren) { | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
export default scrollableRegionFocusableMatches; | ||
function isNoneEmptyElement(vNode) { | ||
return querySelectorAll(vNode, '*').some(elm => | ||
// (elm, noRecursion, ignoreAria) | ||
hasContentVirtual(elm, true, true) | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
describe('isComboboxPopup', () => { | ||
const { isComboboxPopup } = axe.commons.aria; | ||
const { queryFixture } = axe.testUtils; | ||
|
||
it('does not match non-popup roles', () => { | ||
const roles = ['main', 'combobox', 'textbox', 'button']; | ||
for (const role of roles) { | ||
const vNode = queryFixture( | ||
`<div role="combobox" aria-controls="target"></div> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
} | ||
}); | ||
|
||
for (const role of ['menu', 'listbox', 'tree', 'grid', 'dialog']) { | ||
describe(role, () => { | ||
it('is false when not related to the combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox"></div> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
|
||
describe('using aria-controls (ARIA 1.2 pattern)', () => { | ||
it('is true when referenced', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox" aria-controls="target"></div> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isTrue(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is false when controlled by a select element', () => { | ||
const vNode = queryFixture( | ||
`<select role="combobox" aria-controls="target"></select> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is false when not controlled by a combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="button combobox" aria-controls="target"></div> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
}); | ||
|
||
describe('using parent owned (ARIA 1.1 pattern)', () => { | ||
it('is true when its a child of the combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox"> | ||
<div role="${role}" id="target"></div> | ||
</div>` | ||
); | ||
assert.isTrue(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is false when its not a child of a real combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="button combobox"> | ||
<div role="${role}" id="target"></div> | ||
</div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is false when its nearest parent with a role is not a combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox"> | ||
<div role="region"> | ||
<div role="${role}" id="target"></div> | ||
</div> | ||
</div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is true when its nearest parent with a role is not a combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox"> | ||
<div> | ||
<div role="none"> | ||
<div role="presentation"> | ||
<div role="${role}" id="target"></div> | ||
</div> | ||
</div> | ||
</div> | ||
</div>` | ||
); | ||
assert.isTrue(isComboboxPopup(vNode)); | ||
}); | ||
}); | ||
|
||
describe('when using aria-owns (ARIA 1.0 pattern)', () => { | ||
it('is true when referenced', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox" aria-owns="target"></div> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isTrue(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is false when owned by a select element', () => { | ||
const vNode = queryFixture( | ||
`<select role="combobox" aria-owns="target"></select> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
|
||
it('is false when not owned by a combobox', () => { | ||
const vNode = queryFixture( | ||
`<div role="button combobox" aria-owns="target"></div> | ||
<div role="${role}" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
}); | ||
}); | ||
}); | ||
} | ||
|
||
describe('options.popupRoles', () => { | ||
it('allows custom popup roles', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox" aria-controls="target"></div> | ||
<div role="button" id="target"></div>` | ||
); | ||
assert.isFalse(isComboboxPopup(vNode)); | ||
assert.isTrue(isComboboxPopup(vNode, { popupRoles: ['button'] })); | ||
}); | ||
|
||
it('overrides the default popup roles', () => { | ||
const vNode = queryFixture( | ||
`<div role="combobox" aria-controls="target"></div> | ||
<div role="listbox" id="target"></div>` | ||
); | ||
assert.isTrue(isComboboxPopup(vNode)); | ||
assert.isFalse(isComboboxPopup(vNode, { popupRoles: ['button'] })); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a typo