-
Notifications
You must be signed in to change notification settings - Fork 59
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
ARIA element reflection across non-descendant/ancestor shadow roots #192
Comments
First, I don't think we can make that assumption. But more importantly, this is less about security and more about encapsulation. As I understand it, one of the major benefits of shadow DOM is that it encapsulates internal details and prevents any leaking or impact of those details (in either direction) except via specifically defined mechanisms. The whole point of shadow DOM is to isolate implementation details. Element references exposed from shadow DOM via the platform would violate that principle.
I totally understand that this is tricky and tedious. However:
That's fair, but in the light DOM, you also don't have other restrictions (and the benefits that come with those) provided by shadow DOM. If isolating internal details is not a requirement, that suggests you don't need to use shadow DOM. |
Hi @jcsteh, thanks for the response. A few thoughts:
In open shadow DOM, it is already possible today to traverse the entire DOM tree. (E.g. kagekiri implements a So to me, it seems uncontroversial to allow two open shadow roots to link elements with I guess for me the question is: "Why is it acceptable for elements in separate shadow roots to be linked with As for closed shadow roots, it does seem tricky to provide a safe way for elements in separate shadow roots to be linked with
At Salesforce, we use shadow DOM on our platform to provide encapsulation guarantees. However, we would still like a well-defined path to relax these guarantees to enable certain use cases, such as accessibility. Drawing on the previous example, the |
Porous, yes, but clearly defined with regard to leaking element references. That is, you need to explicitly choose to dive into the shadowRoot to access elements inside it; it doesn't just happen as a side effect of some other API. The problem with ARIA element reflection is that this boundary crossing would not at all be explicit.
Initially, the objection raised by Mozilla was that there should never be any shadow boundary crossing at all. I guess this was relaxed as a compromise.
The issue here is "well-defined path to relax these guarantees". If the change you suggest is implemented, there would be no explicit choice to relax them with ARIA element reflection. They would just always be relaxed. CC @annevk. |
@jcsteh Thanks again for the feedback and context.
In open shadow DOM, with my proposal, you would still need to explicitly traverse into a shadow root (e.g. using For closed shadow DOM, the component would need to offer an explicit API to provide access to the element that it wants to be linked. E.g.: customElements.define('fancy-option', class extends HTMLElement {
#shadow = null
constructor() {
super()
this.#shadow = this.attachShadow({ mode: 'closed' })
this.#shadow.innerHTML = '<li role="option"></li>'
}
get option() {
return this.#shadow.querySelector('li')
}
})
otherElement.ariaActiveDescendantElement = $('fancy-option').option Otherwise it's impossible for anyone else to access its |
Once it's linked, though, you can fetch the element from that property. For
example, if you set foo.ariaActiveDescendantElement to something inside a
shadow DOM, anyone with access to foo can then get that shadow element via
foo.ariaActiveDescendantElement, potentially without even knowing they've
crossed the shadow boundary. There was a proposal a while ago to avoid
exposing the value in this instance, but this isn't ideal for authors
because they don't have a good model of what an AT actually sees if the
properties sometimes report null when in reality they refer to some shadow
element.
|
There are a bunch of previous discussions quite similar to this one. See for example whatwg/html#6063 which points to #169. Or other things not directly related to ARIA properties: whatwg/html#3219 & WICG/webcomponents#179 |
Thanks, I think I understand the concern now. I'm still not clear on why descendant/ancestor relationships are a special case, but the problem we're trying to solve is clear to me. |
As I understand it you can go upwards currently, but not downwards. That preserves encapsulation. As in, if you have a single shadow tree it's fine to link from there to outside of it, but you shouldn't have a link from the outside to inside the shadow tree. |
@annevk Thank you! After re-reading the language in Alice's PR to element1.ariaActiveDescendantElement = element2 ... That does preserve the encapsulation you mention (i.e. exposing the same information you could get from walking up the tree, which works even for closed shadow roots). But I think unfortunately it makes the combobox situation worse. I don't see how a web developer could reasonably structure it so that the active listbox option is in a shadow ancestor of the <fancy-listbox>
#shadow-root
<ul role="listbox">
<fancy-option>
#shadow-root
<li role="option">List item 1</li> <!-- shadow ancestor of the <input> -->
#shadow-root
<fancy-input>
#shadow-root
<input type="text">
</fancy-input>
</fancy-option>
<fancy-option>
#shadow-root
<li role="option">List item 2</li>
</fancy-option>
</ul>
</fancy-listbox> // This works because listOption1 is in a shadow ancestor of the input
input.ariaActiveDescendantElement = listOption1 As you can see, the |
I'm not sure if this has been discussed elsewhere, but just to propose a solution, could we enforce the descendant/ancestor restriction only on closed shadow roots? This would preserve the original intention of the restriction, which is to avoid exposing an element that isn't already exposed. In other words: element1.ariaActiveDescendantElement = element2
In the case of open shadow roots, both elements are already exposed. Whereas in the case of closed shadow roots, only ancestors are exposed (by walking up the tree). So this restriction would exactly match the status quo in terms of exposure. |
Yeah that has been discussed and dismissed. We want a solution that works for open/closed. I recommend studying the links in @mrego's comment above: #192 (comment). In particular #169 and WICG/webcomponents#917. |
Thanks, I read through the comment threads. In particular I see this comment:
This is a fair point, but open shadow roots are the default for most web component frameworks that I'm aware of, and it seems odd to force open shadow roots to match closed shadow root semantics moving forward. WICG/webcomponents#917 talks about cross-root ARIA delegation, which doesn't address my scenario because it's not enough to merely delegate the Looking back through previous discussion, Alice sketched out a combobox in this comment. But this example assumes that the list option and listbox share the same shadow root, and that furthermore this shadow root is an ancestor of the With the current limitation, putting the list options and listbox into the same shadow root, and ensuring that that shadow root is at least an ancestor of the |
I see, sorry you had to explain that again. I think in this case |
We discussed this a bit in the AOM meeting today. A high-level summary:
|
During my tests on the meeting yesterday I was focusing on the wrong thing, sorry about my misleading comments. So this is the proposed spec text:
I was focusing in the 2nd step there, but that's the one that updates the attribute to reflect the relationship. That one is not set in Chromium and WebKit when you do But still 4th step above happens, and internally the explicitly set attr-element is set, and it points to |
It looks like this is not a good idea and people don't like it. Even from what I heard this might have been discussed and discarded in the past already. This looked like a kind of hacky workaround anyway, so we should discard this and look for better alternatives related to delegation approaches. The proposed spec text is explicit about this:
|
@nolanlawson should we close this issue and carry on the related work on: https://github.com/leobalter/cross-root-aria-delegation/ and https://github.com/Westbrook/cross-root-aria-reflection/ trying to find a solution for these use cases? |
I found my way here from #195 (and whatwg/html#8544 and whatwg/html#8932), and I wanted to ask if someone could more explicitly state why this isn't a good solution? I.e. just allow the a11y tree to see/use explicitly set attr-element while attr-associated element still returns <div id=el1>
<template shadowrootmode=open><div id=el2></div></template>
</div>
<div id=el3></div>
<script>
el1.ariaActiveDescendantElement === undefined // undefined means no reference has been established yet
const el2 = el1.shadowRoot.firstElementChild;
el1.ariaActiveDescendantElement = el2;
el1.ariaActiveDescendantElement === null // null means a reference has been established, but you can't see it because it's shadow-isolated
el1.ariaActiveDescendantElement = el3;
el1.ariaActiveDescendantElement === el3 // non-shadow-isolated values work as they do today
</script> |
This is what I recall from the AOM meeting, yes. I think someone used the word "gross." 🙂 el1.ariaActiveDescendantElement === undefined // undefined means no reference has been established yet Doesn't this violate the way most properties on const el = document.createElement('div')
el.role // null
el.ariaLabel // null
el.tabIndex // -1
el.title // ''
el.falalalala // undefined Alternatively, I guess you could figure out that "it worked" by checking the el.getAttribute('aria-activedescendant') === null // null means no reference has been established yet
el.ariaActiveDescendantElement = el.shadowRoot.firstElementChild
el.getAttribute('aria-activedescendant') === '' // empty string means reference established |
Ok, thanks. It's different, but I'm not sure it's all the way to "gross". I mean whatwg/html#8932 is not exactly gross either but it's getting more and more complicated.
Yeah, this is also "gross". I guess my point was that perhaps we could get around the problem that you can't tell if a reference was established, in some simple way.
I think this is ok also! It meets the use case, and (other than potential web compat issues) I don't see any use cases it breaks. |
What @mfreed7 proposes reveals the existence of a shadow root as far as I can tell, so that does not seem workable. Apart from web developers having to do a lot of custom bookkeeping, which also seems undesirable. #192 (comment) still seems like the way forward here. |
I'm not against #195, per se. I was just poking at this to see if there's a simpler solution. #195 seems more complicated than just tweaking the IDL or content attribute behavior, and it is also limited to just custom elements.
Is the standard that we don't even reveal the presence of a shadow root? I'm unclear why we need to hold that level of encapsulation. The presence of a shadow root is already easily detectable just by observing layout, right? |
Yes, that's been a design goal since the beginning. |
Hasn't that ship long sailed? This (or many similar techniques) works for all but trivial shadow roots, even if they're closed: function hasShadowRoot(maybeHost) {
const oldChildren = Array.from(maybeHost.childNodes);
Array.from(oldChildren).forEach(el => el.remove());
const testBox = document.createElement('div');
testBox.style.height = '100px'
maybeHost.appendChild(testBox);
const oldHeight = maybeHost.style.height;
maybeHost.style.height = 'min-content';
const hasShadow = getComputedStyle(maybeHost).height !== "100px";
maybeHost.replaceChildren(...oldChildren);
maybeHost.style.height = oldHeight;
return hasShadow;
} I'm sure this was discussed, so perhaps you can point me to the conversation for context. But why is that a design goal? |
Hm, it wasn't my intention that it be limited to custom elements - either I explained it poorly, or there's some implication in my reasoning that I failed to notice. In terms of complication - I'm doubting myself now, but I figured it would be pretty analogous to this change - either way, computing attr-associated elements needs to change (since attr-associated elements represent the internal list which is exposed to AT), so it would be a change to the getter steps, either to null out elements within shadow roots, or to retarget them. |
Apologies! I didn't read #195 closely, and I (for some reason) assumed it was the driver for whatwg/html#8932. Those are relatively disconnected, and I missed that. I like the approach of #195, and it indeed seems general and not limited to custom elements. It is more complicated than the comments I've made above, but it seems tractable. Retargeting seems straightforward, and providing an API (like |
WCCG had their spring F2F in which this was discussed. You can read the full notes of the discussion (WICG/webcomponents#978 (comment)) in which this was discussed, heading entitled "ARIA Mixin & Cross Root ARIA" - where this issue was specifically discussed. In the meeting, present members of WCCG reached a consensus to discuss further in breakout sessions. I'd like to call out that WICG/webcomponents#1005 is the tracking issue for that breakout, in which this will likely be discussed. |
In the current proposed implementation of ARIA element reflection in WebKit, as well as the relevant PR on the spec (whatwg/html#3917) and this WPT test, element reflection only works for elements whose shadow roots are either the same, or in a descendant/ancestor relationship:
As discussed previously, though (whatwg/html#4925 (comment)), there are use cases where the elements' shadow roots may have sibling (or cousin, aunt/niece, etc.) relationships.
For instance, following the ARIA 1.1 Combobox with Listbox Popup example, an implementation with web components might look like:
In this case, the web author may want to set the
<li>
to be theariaActiveDescendant
of the<input>
. However, their shadow roots are not in a descendant-ancestor relationship, so attempting to set theariaActiveDescendant
would be a no-op.With some tinkering of the DOM hierarchy, it's possible to work around this restriction:
However, this requires the
<fancy-listbox>
to be inside the<fancy-input>
's shadow root, which increases the scope of<fancy-input>
beyond just enhancing the<input>
.Another potential alternative is to use cross-root ARIA delegation to have the
<input>
delegate itsaria-activedescendant
to the<fancy-input>
host, but AFAICT the current version of that spec doesn't handle element ID ref attributes likearia-activedescendant
.If I understand the previous summary (whatwg/html#5401 (comment)), this restriction was put in place to avoid leaking internal component details. If we assume all of the shadow roots are open, though, then I'm not sure that that argument is still as strong. It feels like the restriction makes wiring up relationships across shadow boundaries very tricky, whereas in the light DOM world we don't have any similar restrictions.
Would this group be open to relaxing the restriction on cross-shadow ARIA relationships?
/cc @leobalter @mrego
The text was updated successfully, but these errors were encountered: