fix(idref): fallback to qsa #4844
Conversation
| @@ -12,8 +12,8 @@ import { getNodeFromTree } from '../../core/utils'; | |||
| * @return {string} | |||
| */ | |||
| function accessibleText(element, context) { | |||
| const virtualNode = getNodeFromTree(element); // throws an exception on purpose if axe._tree not correct | |||
There was a problem hiding this comment.
This comment is not true as getNodeFromTree does not throw, just returns null if the node isn't found.
lib/commons/dom/idrefs.js
Outdated
| } catch { | ||
| throw new TypeError('Cannot resolve id references for non-DOM nodes'); | ||
| const rootVNodes = getRootVNodes(vNode); |
There was a problem hiding this comment.
I think this is closer than previous attempts, but I think it's going to do the wrong thing with slotted elements; those appear in the virtual tree under the shadowId of the thing they're slotted into, but for idrefs purposes their id counts as being scoped to the root in which they're defined, not the one they're slotted into:
<!-- Behavior is consistent in current chrome/firefox/safari -->
<label for="slotted-input-a">out-of-shadow label</label>
<my-custom-elm>
<template shadowrootmode="open">
<label for="slotted-input-a">in-shadow label competing with out-of-shadow label</label>
<label for="slotted-input-b">in-shadow-only label</label>
<slot></slot>
</template>
<!-- Accessible name is "out-of-shadow label" -->
<input id="slotted-input-a"></input>
<!-- Accessible name is empty -->
<input id="slotted-input-b"></input>
</my-custom-elm>I'm not sure there's enough information in the virtual tree right now to distinguish that case correctly; it might be necessary for us to start tracking information about the "source shadow ID" or something for slotted elements.
There was a problem hiding this comment.
When thinking what to track, consider also this test case:
<label for="slotted-input">out-of-shadow label</label>
<level-a-custom-elm>
<template shadowrootmode="open">
<label for="slotted-input">shadow level A label</label>
<level-b-custom-elm>
<template shadowrootmode="open">
<label for="slotted-input">shadow level B label</label>
<slot></slot>
</template>
<slot></slot>
</level-b-custom-elm>
</template>
<!-- Accessible name is "out-of-shadow label" -->
<input id="slotted-input"></input>
</level-a-custom-elm>| let result = null; | ||
|
|
||
| for (const root of rootVNodes) { | ||
| const foundNode = querySelectorAll(root, `#${token}`)[0]; |
There was a problem hiding this comment.
For shadow root cases, this is going to essentially bypass QSA's selector-cache behavior that would otherwise make the ID lookup fast; we might want to consider making it smarter about that (findMatchingNodes looks like it's meant to understand how to do it, but getNodesMatchingExpression is going to early exit from the cached path anytime root is not the document root)
There was a problem hiding this comment.
Ya. We originally did the cache on the root to improve rule node selection. We cached the information on the root node since it was the only part that would have a complete picture of the entire tree. We could work around that limit for just id lookups, but it would require somehow passing the desired shadowId for the getNodesMatchingExpression query instead of grabbing it from the root node. Maybe we create a getElementById like function that allows you do this this? Something like:
function getById(id, shadowId) {
const root = axe._tree;
return getNodesMatchingExpression(root, id, () => {}, shadowId)[0]
}
// selector-cache.js
export function getNodesMatchingExpression(domTree, expressions, filter, shadowId) {
shadowId = shadowId ?? domTree[0].shadowId;
}
No description provided.