-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Element reflection and shadow roots #5401
Comments
Well, |
@annevk I'm not sure I follow. Could you elaborate? |
It sounded like you were considering making changes to |
Thanks for the pointer. It seems like this would be an extra step in the "If document is not oldDocument" branch. I take the point that we could add a check for the shadow scope (is there a better term than this? I realise it's not spec language) at that point as well, but I'm still not convinced of the value of doing that as compared to the potential runtime cost of recursively checking the relative shadow scope every time a node is inserted in the document. I would rather caution authors against breaking their own encapsulation, and give them an option to safely refer into deeper shadow scopes without leaking implementation details. |
Do you mean shadow tree? https://dom.spec.whatwg.org/#shadow-trees has all the relevant terminology. |
Seems likely, thanks. Any thoughts on my other comments? |
At a high level, I don't think we should increase the number of differences between |
Could you clarify what part you're having trouble with, and I can try and re-state it? |
Regarding the difference with expandos - granted, but the comparison is with authors having the ability to create leaks, as opposed to leaks arising from normal usage like the case with event paths. |
I re-worked the issue description to present the alternatives I can see, and some sense of the trade-offs involved. Hope that helps. |
This issue enumerates solutions without directly explaining what the problem is. Why does code without access to the Shadow DOM need to see or manipulate an Maybe it's meant to be obvious why authors would want to do such a thing but it isn't obvious to me. I would expect libraries that use Shadow DOM for encapsulation to manage ariaActiveDescendant themselves. Or maybe it's just that there needs to be some defined behavio for this caser, without it mattering much what it is. Or perhaps this issue is assuming context that I don't have. (Reading it only because I was @-referenced). |
@othermaciej Apologies for summoning you, and thank you for reading!
It's more about defining what should happen, as you suggest later in your comment. The "typical" case is more likely not to involve crossing shadow roots, but unless we explicitly define what should happen in that case, we end up with the situation described in option 0. (And maybe that's fine, if we don't expect authors to actually end up doing this.)
I assume you mean via the |
You would expect them to manage all of these things themselves, yes. Also, it seems like generally a bad idea to expose any reference (opaque seems fine and kind of consistent with some other parts of the platform), yes. But that doesn't mean someone won't design something like that. I think is the idea - the same way they could with any other node reference on a closed shadow root today. But if you do that it's kind of on you, you violated your own stated desire for encapsulation, and we don't explicitly prevent it. So the question is, I think, what to do with when someone in the Shadow connects some ref to something in the light Dom or used a leaked ref to a node as a reference for one of these. Something defined, presumably, but what? I think option 0 or .1 make sense from this perspective. |
This is a really critical feature for accessibility of Web Components, and this is the last remaining issue to sort out before we have a design we can actually implement and ship. How can we get this discussion moving somewhere productive? |
It seems the opaque reference discussion in #4925 got quite far. Any reason that isn't being fleshed out more? Did it stall on something? |
The conversation went in a different direction and we didn't pick it up again. Would that be your preference? |
I'm not sure I have a strong sense of the right solution here. All seem to have drawbacks of some kind. The other solution that seemed somewhat interesting to me is to continue with ID-based references, but making it easier to mint them. |
Do you have a specific proposal for an ID-based API? Note that an ID-based API wouldn't allow crossing Shadow boundaries in any direction, including to "lighter" DOM. |
Very briefly, I was thinking something like |
Thanks for that. Might you be able to spell out the specific issues you see with the other options, as well? |
0) Violates encapsulation. 1) Unclear, seems interesting and I think the opaque references are worth flushing out. 2) Violates encapsulation. Also brittle. 3) Not sure. 4) If the runtime cost is real I suspect this wouldn't be acceptable to people. 5) This seems very surprising for web developers. |
We disagree on what "violates encapsulation" means. Can you possibly spell out the consequences of allowing authors to break their own encapsulation? |
A single developer might know what they are doing, but when multiple developers and libraries are involved, encapsulation is good to have. Making private state globally accessible isn't a good idea. |
I agree that making private state globally accessible isn't a good idea, but even option 0 doesn't do that without a developer explicitly giving access. Do you have a scenario in mind under which that would happen? |
To answer my own question: @hober came up with a plausible (but improbable) scenario. Alice sets up an Bob's library, "Boomerang", moves Carol's extension, "Label Embiggener", walks from |
Why would you want both Also, even if we somehow could not use whitespace, it seems you could encode such information in the referencing attribute name too. |
So your proposal is to add the indirection I referred to? i.e. some type of syntax to explain hopping out of shadow roots as necessary? I take it moving into shadow roots would be impossible in this scenario? |
Yeah, see #5401 (comment). |
@annevk if I understand correctly, you're advocating for basic relationship between elements to be expressed as an Element-to-id relation vs Element-to-Element. If this is the case, does your proposed syntax in #5401 (comment) assume that both elements are known in advance? ... or not? |
This seems to me like the simplest solution that preserves encapsulation. I think I'd rather we not block this on coming up with a globalid feature. |
@atanassov I'm not sure I understand the question. |
Ah, my bad for omitting some of the background in my question! Reading through a related PR discussion #3917 (comment) and considering that In the micro syntax you suggested,
That lead me to believe that in the absence of |
I see, I should probably have left the "syntax sugar" out. That the user agent assists in generating IDs is only a convenience to make linking easier. A web developer could mint them themselves and it would work the same way. |
Summary of what I think was the consensus from recent AOM discussions: https://gist.github.com/alice/5b755f7f4487fa614d9c70cdf82259fa @rniwa does this match your recollection? |
Thanks for the summary! I'm not sure which part of that is something of which we've reached a consensus but it does seem to capture the latest proposal / discussions we've had. |
We discussed this at yesterday's web components meeting. In general, the group was keen to avoid putting component authors in a situation where they would need to have a component leak a reference to an inner shadow element in pursuit of accessibility. To ground the discussion, we looked at a concrete use case of a combo box component with an inner list component whose shadow contains the list items of interest to the combo box. <combo-box>
#shadow-root
<input aria-activedescendant=(how to point to an option inside option-list???)>
<option-list>
#shadow-root
<option id="option1">1</option>
<option id="option2">2</option>
<option id="option3">3</option>
</option-list>
</combo-box> We could ask the option-list component to expose a reference to one of the options hiding in its shadow — option-list would expose a We could address these problems by introducing a layer of indirection through delegation: 1) the input indicates that it wants to get its activedescendant from the option-list component and 2) the option-list indicates that, if it is being used as an activedescendant, it wants to delegate that responsibility to an inner element. Step #1 could be accomplished via existing <combo-box>
#shadow-root
<input aria-activedescendant="optionList">
<option-list id="optionList">
.elementInternals.elementToUseAsActiveDescendant = option1 // set programmatically
#shadow-root
<option id="option1">1</option>
<option id="option2">2</option>
<option id="option3">3</option>
</option-list>
</combo-box> The goal of this approach would be to let option-list participate in the input's accessibility as an intermediary, without having to expose a reference to an element in its shadow. Additionally, the API which option-list supports would be a standard one, so the author of combo-box could swap in a different list component and get the same results. Another use case we discussed was having a component host serve as a label for another element — but the host would then delegate its label responsibility to an inner shadow element which is not publicly disclosed. There was general interest in this approach. @jcsteh, @shleewhite, and I expressed willingness to pull together some more use cases, then work towards a concrete proposal. If that gains traction, we could obviate the need to deal with references to elements in closed shadow trees. |
I am also willing to contribute to concrete use-cases - this is an issue we've run into many times over the recent years. While the Another very common case we run into is using A more complex use-case that I described in the F2F (in a more simplified way) relates to the use of While I prefer a solution that preserves encapsulation, as a component author I am putting our users first, which unfortunately means doing whatever is necessary to make the component accessible, encapsulation aside. In some cases, this may mean looking for work-arounds that may compromise the component API. So, I am happy to share more details or contribute further to that discussion. Feel free to include me. 😄 |
Thanks for that summary, Jan! I'd like to clarify the behaviour around moving explicitly set element references around the document/scope. "Valid scope" for two elements, A→B, is currently understood to mean:
"Invalid scope" for two elements A→B:
The group seemed in favour of keeping these relationships alive even as elements move between scopes, as long as at the time of getting, the relationship is valid. (apologies for the contrived/weird example). <div id="container">
<input id="inputElement"></input>
<option id="opt1">Option 1</option>
#shadow-root (open)
| <slot></slot>
</div> // Same document/scope
inputElement.ariaActiveDescendantElement = option1;
console.log(inputElement.ariaActiveDescendantElement); // logs <Option1>
console.log(inputElement.getAttribute("aria-activedescendant"); // logs "option1"
// Referred to element gets adopted into deeper shadow root.
// The relationship still exists, but is not observable/gettable.
shadowRoot.appendChild(option1);
console.log(inputElement.ariaActiveDescendantElement); // logs null
// Content attribute is synchronised on setting only
console.log(inputElement.getAttribute("aria-activedescendant"); // logs "option1"
// Re-adopt the referred to element back into light DOM/the same scope.
// The relationship does not need to be re-established.
optionList.appendChild(option1);
console.log(inputElement.ariaActiveDescendantElement); // logs <Option1>
console.log(inputElement.getAttribute("aria-activedescendant"); // logs "option1"
</script> The explicitly set element association still exists when the referred to element passes through an "invalid" state, it is just not allowed to be the attr-associated element (the relationship exists, but is not observable). The same rules would apply for if the referred to element is not in the same document. If it has been explicitly set via At the moment the spec only synchronises the content attribute with the IDL attribute when the relationship is set, so when the relationship passes through an invalid state it would actually return the id |
That is not my understanding of the discussion. I'm pretty certain we were in favor of keeping the relationship when B is not in a document (1) but wanted to severe the relationship when B moves into an inner / deeper shadow root (2) the same way we don't want to keep the relationship when B is inserted into another document. |
@rniwa Do you recall, or can you give our own reasoning behind these decisions? (Edit: Meant to type "your" above, oops) |
We want to keep the relationship because of the use case that we want to be able to move the “related” node from one place in document to another. This would not be possible if the relationship was severed immediately at the time of a referenced or referencee getting disconnected from a document. In the case when a node is inserted into a shadow tree or another document, we should be clearing the relationship since in the case of shadow trees we want to preserve encapsulation and in the case of documents, we want to avoid leaks. |
Can you spell out how allowing the explicitly set attr-element to persist (but not be observed) fails to preserve encapsulation?
Again, can you spell out what the leak situation would be? |
Not making it observable would solve encapsulation problem but Mozilla raised the concern that any use case that involves such a relationship is better served by a delegation mechanism akin to
The leak would be the entire document of the node being referenced despite of the fact AT will not be able to respect such a relationship (cross document) anyway. |
I think we're all on board with trying to find an alternative solution to allow exposing a reference to an element in deeper shadow DOM. I'm not following how that implies that the explicitly set attr element has to drop off, though. (Edit:) ... since the neither the JS getter nor the computed attr-associated element will make it observable until the element moves into lighter Shadow DOM.
I'm sorry, I'm still not following - if the relationship is not valid, the getter won't return the element (per Meredith's comment). Is this a memory leak issue, or an encapsulation leak issue? |
No, Mozilla was making an argument that even in the case of referencing to an outer tree, a better approach is to use a delegation mechanism. I think that's largely true. The CSS shadow parts has an explicit part forwarding mechanism to address the use cases like that instead of replying on scripts to establish the relationship.
It's largely because developers in the room preferred to have the observed behavior of the element reflections match that of what assistive technology sees.
Cross-document referencing is a memory leak issue, not an encapsulation issue. |
Hm, that's the first I've heard of this argument. I didn't see this captured anywhere. So this would rule out referring to lighter Shadow DOM as well?
Right, and that would be the computed attr-associated element, not the explicitly set attr-element (which is a piece of data used to compute the computed attr-associated element).
Can you expand on this? |
I thought the consensus was:
We enumerated 4 cases where we would remove a reference from
In all other cases the underlying reference would be kept, but accesssors/getters would only return the reference if it met the scoping requirements (shadow-including-ancestors), and would return Moving
If we disallow references from lighter to darker then we are no more capable than ID based references. The room was uncertain on how exactly to handle a reference from lighter to darker shadow root. |
Those of us present last week agreed upon the following. Definitions:
The attr-associated element will only expose the explicitly set attr-element reference if the referenced element is in a valid scope, otherwise the attr-associated element will be null. All examples here refer to an element The explicitly set attr-element reference will remain intact unless cleared through one of these 3 enumerated cases:
We consider an explicitly set attr-element to be valid scope iff: Some examples of valid scope:
Examples of invalid scope:
The explicitly set attr-element reference does not constitute a reference for the purposes of garbage collection. If there are no other references to the referenced-to element <div id="A">A</div>
<div>B</div> A.ariaFooElement = A.nextSibling;
A.ariaFooElement.remove(); // remove "B" from the document tree
console.log(A.ariaFooElement); // null
A.parentNode.appendChild(A.ariaFooElement); // Does nothing, A.ariaFooElement is null
// "B" may be garbage collected at any point, with no way to observe this. [1] Allowing references from darker to lighter shadow was a core use-case of this feature, however we aren’t sure if everyone present at the F2F agreed to this. [2] During the F2F there was interest in referring from a lighter to darker shadow through some kind of delegation/port mechanism, we agreed to break out this work into a separate bug for now and to not block progress on the wider feature. |
I think this is correct, but the rationale was a bit misleading for me. I think that the reason this works is because if the explicitly set attr-element is the only reference to |
What is the relationship between
Element
type IDL attributes and Shadow DOM encapsulation?(Previous general discussion, and concepts and terminology)
For example, suppose an author wants to set an
aria-activedescendant
-associated element,descendant
, on an elementhost
.To what extent, for what reasons, and at what cost, should the user agent prevent the author from setting this relationship if
descendant
is, say, closed-shadow-hidden[1] fromhost
?Why prevent referring to closed-shadow-hidden elements?
In general, shadow DOM is intended to hide implementation details in order to prevent authors depending on those details, and thus causing things to break if those implementation details change.
From https://gist.github.com/alice/174ae481dacdae9c934e3ecb2f752ccb:
This roughly corresponds to "type 1 encapsulation", as described by @othermaciej and @annevk: "Encapsulation against accidental exposure — DOM nodes from the shadow tree are not leaked via pre-existing generic APIs — for example, events flowing out of a shadow tree don't expose shadow nodes as the event target."
The caveat is that in this case, an author would have to have explicitly set the
ariaLabelledByElement
property to be a closed-shadow-hidden element; that is, the author would have to already have access to those nodes, rather than the nodes being leaked with no action from the author.However, the author did not deliberately provide access to every element inside shadow DOM.
This corresponds to "type 2 encapsulation" in the same framework: "Encapsulation against deliberate access — no API is provided which lets code outside the component poke at the shadow DOM. Only internals that the component chooses to expose are exposed."
The same caveat from above applies.
This might be called the "rule of inference" problem: the potential to open the door to other APIs which allow access to closed-shadow-hidden elements.
Possible solutions and trade-offs
0. Do nothing/authoring advice
Advise authors against setting closed-shadow-hidden elements as attr-associated elements for elements in light DOM, but do nothing to prevent them.
Optional (0.1): allow (but do not require) using opaque references in place of element references. This allows authors to preserve their own encapsulation in the case where they wish to make a semantic connection between two elements where the connection would otherwise leak implementation details. (Obviously, this requires setting up the spec and implementation machinery for opaque references.)
1. Allow access but prevent tree walking
Allow setting references to closed-shadow-hidden elements, but prevent using them to access the rest of the shadow tree.
This might look something like: at
get
time, check whether the attr-associated element is closed-shadow-hidden relative to the host element, and if so, do not return it (i.e. returnnull
instead).It might make sense to return something like an opaque reference instead, to avoid confusion as to whether a reference has been set.
Variations:
(1.1) Always return an opaque reference, rather than only when the attr-associated element is closed-shadow-hidden. This would avoid having a polymorphic getter.
(1.2) Make the API fully opaque reference based, requiring authors to generate an opaque reference before setting an explicitly-set attr-associated element, e.g.
2. Check on setting
Fail silently by setting the reference to
null
if the given value is not a descendant of any of the host element's shadow-including ancestors. (This is what is in the current spec PR.)This does not prevent authors from removing elements from the light DOM and re-adding them into shadow DOM, however.
3. Check on getting
This is similar to (1), but also affects the computed attr-associated element, impacting accessibility APIs etc.
That being the case, this runs into issues since some of those APIs need timely updates when attr-associated elements change (for example, setting
aria-activedescendant
is equivalent to a focus change for some API consumers) so if an element were to be re-parented causing the association to be effectively lost, there would not be a timely update since there is no "get" in that case.4. Check on setting and during the "adopt node" algorithm
In addition to preventing attr-associations being created where the attr-associated element is closed-shadow-hidden relative to the host node, add steps to the "adopt a node" algorithm to check that for each attr-association an element is participating in (either a host or as an attr-associated element), the attr-associated element isn't currently closed-shadow-hidden relative to the host.
This potentially involves a significant run-time cost, as checking whether one element is closed shadow-hidden relative to another can involve an ancestor walk.
5. Check on setting, and disallow references to elements which are not connected
This would effectively prevent references being created to closed-shadow-hidden elements, but at the cost of not being able to create associations to elements which are not yet inserted into the DOM, which was one of the requirements described by @zcorpan at TPAC.
[1] The same logic probably applies to open shadow roots, in general, but there doesn't seem to be a named concept for "open shadow hidden".
The text was updated successfully, but these errors were encountered: