Skip to content
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

Open
alice opened this issue Mar 26, 2020 · 53 comments
Open

Element reflection and shadow roots #5401

alice opened this issue Mar 26, 2020 · 53 comments
Labels
a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@alice
Copy link
Contributor

alice commented Mar 26, 2020

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 element host.

host.ariaActiveDescendantElement = descendant;

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] from host?

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:

  • May cause problems if scripts "accidentally" walk into deeper shadow content via known properties
    e.g.
    // lightEl.ariaActiveDescendantElement was set by the component to be 
    // an element within the component's shadow DOM
    lightEl.ariaActiveDescendantElement.textContent = "new text";
    
    // ** Now it is possible to access the entire shadow tree for the component! **
    lightEl.ariaActiveDescendantElement
           .parent
           .appendChild(document.createTextNode("🆕"));
    • (Note that extension scripts often already have access to Shadow DOM in any case; the concern here would be page scripts.)

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.

  • Provides a surface for developers to "hack" their way into depending on implementation details of components, if components expose elements within shadow DOM, e.g.
    // In this method, component sets `for` on lightDOMElement 
    // to an <input> inside Shadow DOM.
    // The author uses this to get access to elements inside Shadow DOM,
    // creating an implicit dependency on Shadow DOM internals.
    component.setLabel(lightDOMLabelElement);
    lightDOMLabelElement.for.style.backgroundColor = "papayawhip";
    lightDOMLabelElement.for = null;

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.

  • May be cited as a precedent for other APIs which may also be construed as weakening Shadow DOM encapsulation.

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. return null instead).

It might make sense to return something like an opaque reference instead, to avoid confusion as to whether a reference has been set.

lightEl.ariaActiveDescendantElement = shadowHiddenElement;
assertEquals(lightEl.ariaActiveDescendantElement, 
             document.getOpaqueReference(shadowHiddenElement));

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.

    lightEl.ariaActiveDescendantElement = lightSibling;
    assertEquals(lightEl.ariaActiveDescendantElement, 
                 document.getOpaqueRef(lightSibling));
  • (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.

    lightEl.ariaActiveDescendantElement = document.getOpaqueRef(lightSibling);

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.)

lightEl.ariaActiveDescendantElement = shadowHiddenElement;
assertEquals(lightEl.ariaActiveDescendantElement, null);

This does not prevent authors from removing elements from the light DOM and re-adding them into shadow DOM, however.

lightEl.ariaActiveDescendantElement = lightSibling;
lightEl.shadowRoot.appendChild(lightSibling)
assertEquals(lightEl.ariaActiveDescendantElement, lightSibling);

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".

@annevk
Copy link
Member

annevk commented Mar 26, 2020

Well, appendChild() calls the internal adopt algorithm. If you're thinking of changes to adopt, that's likely where you'd make them, no?

@annevk annevk added the topic: shadow Relates to shadow trees (as defined in DOM) label Mar 26, 2020
@alice
Copy link
Contributor Author

alice commented Mar 26, 2020

@annevk I'm not sure I follow. Could you elaborate?

@annevk
Copy link
Member

annevk commented Mar 27, 2020

It sounded like you were considering making changes to adoptNode(). Typically any such changes would be made in https://dom.spec.whatwg.org/#concept-node-adopt, which also impacts tree mutation operations.

@alice
Copy link
Contributor Author

alice commented Mar 30, 2020

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.

@annevk
Copy link
Member

annevk commented Mar 30, 2020

Do you mean shadow tree? https://dom.spec.whatwg.org/#shadow-trees has all the relevant terminology.

@alice
Copy link
Contributor Author

alice commented Mar 30, 2020

Seems likely, thanks.

Any thoughts on my other comments?

@annevk
Copy link
Member

annevk commented Mar 30, 2020

At a high level, I don't think we should increase the number of differences between adoptNode() and appendChild(). And I think as rniwa and I argued before, leaking through expandos is very different from a platform API leaking. (Having said that, I'm having a bit of trouble interpreting what you wrote due to the non-standard terminology.)

@alice
Copy link
Contributor Author

alice commented Mar 30, 2020

Could you clarify what part you're having trouble with, and I can try and re-state it?

@alice
Copy link
Contributor Author

alice commented Mar 30, 2020

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.

@alice
Copy link
Contributor Author

alice commented Apr 8, 2020

I re-worked the issue description to present the alternatives I can see, and some sense of the trade-offs involved. Hope that helps.

@othermaciej
Copy link

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 ariaActiveDescendantElement that points to an element inside the Shadow DOM? Which solutions make sense would depend on the problem to be solved.

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).

@alice
Copy link
Contributor Author

alice commented Apr 8, 2020

@othermaciej Apologies for summoning you, and thank you for reading!

Why does code without access to the Shadow DOM need to see or manipulate an ariaActiveDescendantElement that points to an element inside the Shadow DOM?

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 would expect libraries that use Shadow DOM for encapsulation to manage ariaActiveDescendant themselves.

I assume you mean via the ElementInternals object? Agreed, that would be the more likely situation.

@bkardell
Copy link
Contributor

[Snip] 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.

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.

@alice
Copy link
Contributor Author

alice commented May 3, 2020

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?

@annevk annevk added a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility labels May 4, 2020
@annevk
Copy link
Member

annevk commented May 4, 2020

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?

@alice
Copy link
Contributor Author

alice commented May 4, 2020

The conversation went in a different direction and we didn't pick it up again.

Would that be your preference?

@annevk
Copy link
Member

annevk commented May 4, 2020

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.

@alice
Copy link
Contributor Author

alice commented May 4, 2020

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.

@annevk
Copy link
Member

annevk commented May 5, 2020

Very briefly, I was thinking something like elementA.idRef(elementB, { scope: "into-the-light" }) which gives you id-ref="into-the-light #uuid-or-some-such or some such on elementA and id="uuid-or-some-such" on elementB (or it reuses the existing id="" attribute on elementB, if any). Depending on the exact choices you'd make with such a design there would also be shortcomings of course, but all the lifetime issues go away.

@alice
Copy link
Contributor Author

alice commented May 5, 2020

Thanks for that.

Might you be able to spell out the specific issues you see with the other options, as well?

@annevk
Copy link
Member

annevk commented May 5, 2020

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.

@alice
Copy link
Contributor Author

alice commented May 5, 2020

We disagree on what "violates encapsulation" means. Can you possibly spell out the consequences of allowing authors to break their own encapsulation?

@annevk
Copy link
Member

annevk commented May 5, 2020

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.

@alice
Copy link
Contributor Author

alice commented May 5, 2020

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?

@alice
Copy link
Contributor Author

alice commented May 6, 2020

To answer my own question: @hober came up with a plausible (but improbable) scenario.

Alice sets up an ariaLabelledByElement relationship between inputA and labelB.

Bob's library, "Boomerang", moves labelB into a closed Shadow Root.

Carol's extension, "Label Embiggener", walks from inputA to labelB, and adds a style element adjacent to labelB to apply a custom style to the label.

@annevk
Copy link
Member

annevk commented May 19, 2020

Why would you want both id and globalid if id were made to work for the latter purpose? (FWIW, it's not clear to me we redefined ID and IDs are at least not allowed to have whitespace in them and some syntaxes make use of that.)

Also, even if we somehow could not use whitespace, it seems you could encode such information in the referencing attribute name too.

@alice
Copy link
Contributor Author

alice commented May 19, 2020

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?

@annevk
Copy link
Member

annevk commented May 19, 2020

Yeah, see #5401 (comment).

@atanassov
Copy link

@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?

@hober
Copy link
Contributor

hober commented Jun 9, 2020

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. return null instead).

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.

@annevk
Copy link
Member

annevk commented Jun 10, 2020

@atanassov I'm not sure I understand the question.

@atanassov
Copy link

@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 id-refs express element relationship through the notion of id belonging to the first matched element (element-to-id relationship) as opposed to a more direct element reference (element-to-element relationship).

In the micro syntax you suggested, id-refs are minted by:

which gives you id-ref="into-the-light #uuid-or-some-such or some such on elementA and id="uuid-or-some-such" on elementB (or it reuses the existing id="" attribute on elementB, if any).

That lead me to believe that in the absence of id attribute on elementB, a uuid-or-some-such is minted for elementB and is used as the id-ref. Would that make the relationship more like element-to-element because of the id-ref uniqueness?

@annevk
Copy link
Member

annevk commented Jun 12, 2020

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.

@alice
Copy link
Contributor Author

alice commented Sep 9, 2020

Summary of what I think was the consensus from recent AOM discussions:

https://gist.github.com/alice/5b755f7f4487fa614d9c70cdf82259fa

@rniwa does this match your recollection?

@rniwa
Copy link

rniwa commented Sep 9, 2020

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.

@JanMiksovsky
Copy link

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 activeItem property, say — so that combo-box can hand that element reference to the input. But this violates encapsulation. Even if option-list is willing to have its shadow open, it still feels odd that the option-list author must expose this public API to return the active item. Worse, the public API is proprietary; a different list component might expose a different API for the same purpose, making it challenging for the combo-box author to try different list components.

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 aria- attributes or element references. Step #2 would be accomplished through a TBD mechanism, likely elementInternals.

<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.

@dbatiste
Copy link

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 aria-activedescendant case is denoted as the classic case above, it's actually one that we don't run into that often. Labelling via aria-labelledby where either one or both the labelling element or element being labelled is within a different DOM context is probably the most common case we run into.

Another very common case we run into is using aria-describedby to describe an element by a tooltip component, again, where either element may be in a different DOM context.

A more complex use-case that I described in the F2F (in a more simplified way) relates to the use of aria-controls and aria-labelledby for wiring up tabs and tab panels, where the tabs may be rendered within a tab-list component's shadow DOM, with the panels defined in the light DOM. This case is somewhat different from the others, in that it relates many tabs to many panels across a single shadow boundary.

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. 😄

@dot-miniscule
Copy link

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:

  1. Both A and B are in the same document.
  2. B is in a "lighter" DOM (B is a descendant of a shadow including ancestor of A)
  3. Neither A nor B is in a document (this allows for authors to set and check these relationships before inserting either element into the DOM).

"Invalid scope" for two elements A→B:

  1. B is not in the document.
  2. B is in a "darker" shadow root.

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 element.attrElement, when B moves out of the document, the relationship persists, it just may not be observable, and hence querying either the IDL attr will return null.

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 "option1" but that's somewhat inconsistent with the behaviour Jan outlined in this comment on #4925.

@rniwa
Copy link

rniwa commented Sep 18, 2020

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:

  1. Both A and B are in the same document.
  2. B is in a "lighter" DOM (B is a descendant of a shadow including ancestor of A)
  3. Neither A nor B is in a document (this allows for authors to set and check these relationships before inserting either element into the DOM).

"Invalid scope" for two elements A→B:

  1. B is not in the document.
  2. B is in a "darker" shadow root.

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.

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.

@alice
Copy link
Contributor Author

alice commented Sep 21, 2020

@rniwa Do you recall, or can you give our own reasoning behind these decisions?

(Edit: Meant to type "your" above, oops)

@rniwa
Copy link

rniwa commented Sep 21, 2020

@rniwa Do you recall, or can you give our own reasoning behind these decisions?

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.

@alice
Copy link
Contributor Author

alice commented Sep 21, 2020

in the case of shadow trees we want to preserve encapsulation

Can you spell out how allowing the explicitly set attr-element to persist (but not be observed) fails to preserve encapsulation?

and in the case of documents, we want to avoid leaks.

Again, can you spell out what the leak situation would be?

@rniwa
Copy link

rniwa commented Sep 21, 2020

in the case of shadow trees we want to preserve encapsulation

Can you spell out how allowing the explicitly set attr-element to persist (but not be observed) fails to preserve encapsulation?

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 delegatesFocus to which we (Apple) agreed.

and in the case of documents, we want to avoid leaks.

Again, can you spell out what the leak situation would be?

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.

@alice
Copy link
Contributor Author

alice commented Sep 21, 2020

Can you spell out how allowing the explicitly set attr-element to persist (but not be observed) fails to preserve encapsulation?

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 delegatesFocus to which we (Apple) agreed.

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.

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'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?

@rniwa
Copy link

rniwa commented Sep 21, 2020

Can you spell out how allowing the explicitly set attr-element to persist (but not be observed) fails to preserve encapsulation?

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 delegatesFocus to which we (Apple) agreed.

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.

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.

I'm not following how that implies that the explicitly set attr element has to drop off, though.

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.

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'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?

Cross-document referencing is a memory leak issue, not an encapsulation issue.

@alice
Copy link
Contributor Author

alice commented Sep 21, 2020

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.

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?

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.

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).

Cross-document referencing is a memory leak issue, not an encapsulation issue.

Can you expand on this?

@chrisosaurus
Copy link

[...] 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.

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.

I thought the consensus was:

  • we should only expose meaningful relationships to AT (e.g. not expose an out-of-document referenced element)
  • the JS observable value of an element's attributes should match what we expose to AT
  • the underlying reference would remain intact
  • getters/accessors would lie (returning null or empty-string) if the referenced element was not exposed to AT
  • an implementation may collect the referenced element if there are no other references to it (e.g. the element could never in the future be exposed to AT), accessors must continue lying (to not expose GC timing)

We enumerated 4 cases where we would remove a reference from A to B for attribute 'aria-foo':

  1. When B [or A] is moved to a different document, or
  2. A.ariaFooElement = null;, or
  3. A.removeAttribute('aria-foo');, or
  4. A.setAttribute('aria-foo', '');.

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 null or empty-string otherwise (while keeping the reference intact).

Moving B into a deeper/darker shadow was not one of those cases, and so the underlying reference should be kept intact (but accessors should return null/empty-string).

[...] that even in the case of referencing to an outer tree, a better approach is to use a delegation mechanism [...]

If we disallow references from lighter to darker then we are no more capable than ID based references.
I thought the room was okay with references from darker to lighter shadow root, apologies if I missed something.

The room was uncertain on how exactly to handle a reference from lighter to darker shadow root.
It was agreed upon that a discussion should happen to distill use cases and to possibly come up with a separate mechanism for a shadow to expose/forward parts/attributes in order to allow limited forms of lighter to darker references.

@chrisosaurus
Copy link

chrisosaurus commented Oct 14, 2020

Those of us present last week agreed upon the following.

Definitions:

  • The explicitly set attr-element is an internal reference within the implementation, authors can set this via the DOM API.
  • The attr-associated element is computed from sources including the explicitly set attr-element reference, and is what we expose to JS and AT.

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.
The attr-associated element will always be consistent between JS and AT.

All examples here refer to an element A which has an explicitly set attribute element reference to an element B for the made-up attribute ‘aria-foo’ (e.g. A.ariaFooElement = B;).

The explicitly set attr-element reference will remain intact unless cleared through one of these 3 enumerated cases:

  1. A.ariaFooElement = null;, or
  2. A.removeAttribute(‘aria-foo’);, or
  3. A.setAttribute(‘aria-foo’, ‘’);.

We consider an explicitly set attr-element to be valid scope iff:
Both A and B are owned by the same document, and
B is a descendant of a shadow-including-ancestor of A (either A and B are in the same tree, or B is in a "lighter" shadow root than A).
Otherwise the reference is considered invalid and the computed attr-associated element will be null.

Some examples of valid scope:

  1. A and B are in the same tree:

    <div id="A">A</div>
    <div id="B">B</div>
    A.ariaFooElement = B;  // Siblings
    console.log(A.ariaFooElement);  // B
  2. B is a descendant of a shadow-including ancestor of A (a "lighter" tree) [1]:

    <div id="B">
    <div id="shadow-host">
    # shadowRoot 
    |  <div id="A">
    </div>
    A.ariaFooElement = B; // Lighter tree
    console.log(A.ariaFooElement);  // B
  3. A and B are in the same tree and owned by the same document, but not connected to the document root

    <div id="parent">
      <div id="A">A</div>
      <div id="B">B</div>
    </div>
    parent.remove();  // entire subtree is no longer connected to the document tree
    A.ariaFooElement = B;  // Siblings
    console.log(A.ariaFooElement);  // B

Examples of invalid scope:

  1. B is in a Darker shadow root than A [2].

    <div id="A">A</div>
    <div id="shadow-host">
    # shadowRoot 
    |  <div id="B">B</div>
    </div>
    A.ariaFooElement = B;  // sets the explicitly-set aria-foo element
    console.log(A.ariaFooElement);  // null, invalid scope because B is in a darker shadow 
    A.parentNode.append(B);
    console.log(A.ariaFooElement);  // B
  2. A and B are in different documents.

    <div id='A'>A</div>
    var newDoc = document.implementation.createHTMLDocument('new document');
    B = newDoc.createElement('div');
    newDoc.body.appendChild(B);
    
    A.ariaFooElement = B;
    
    console.log(A.ariaFooElement); // null, invalid scope because cross-document
  3. A and B are in "sibling" shadow roots.

    <div id="shadow-host-1">
    #shadowRoot
    |  <div id="A">A</div>
    </div>
    <div id="shadow-host-2">
    # shadowRoot 
    |  <div id="B">B</div>
    </div>
    A.ariaFooElement = B;  // sets the explicitly-set aria-foo element
    console.log(A.ariaFooElement);  // null, invalid scope because of shadow-host-2
    
    A.parentNode.append(B);
    console.log(A.ariaFooElement);  // B
  4. A is inserted into the document tree while B is not

    <div id="A">A</div>
    <div id="B">B</div>
    B.remove();
    a.ariaFooElement = B;
    console.log(a.ariaFooElement);  // null, B is not a descendant of any of A's shadow-including ancestors
    
    A.parentNode.append(B);
    console.log(a.ariaFooElement);  // B
  5. Neither A nor B is in the document tree, and they have no ancestor in common

    <div id="A">A</div>
    <div id="B">B</div>
    A.remove();
    B.remove();
    A.ariaFooElement = B;
    console.log(A.ariaFooElement);  // null, no ancestor in common
    
    A.append(B);
    console.log(A.ariaFooElement);  // B, now in the same node tree (A is the ancestor in common)

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 B then B may be considered for garbage collection.
This does not expose garbage collection timing as in order for there to be no other references to B we must already be in a state where the computed attr-associated element is null (e.g. there is no way to observe the explicitly set attr-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.
We have broken out discussion on this consensus on issue #6063

[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.
Ryosuke has begun describing such a mechanism in WICG/aom#169

@annevk
Copy link
Member

annevk commented Oct 14, 2020

This does not expose garbage collection timing

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 B, there is no way to put A in a state where its explicitly set attr-element (B) becomes its attr-associated element. (The phrasing you used initially led me to think one could simply put A in the same subtree as B, but that would not work as that subtree would hold B alive.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y-tracker Group bringing to attention of a11y, or tracked by the a11y Group but not needing response. accessibility Affects accessibility topic: shadow Relates to shadow trees (as defined in DOM)
Development

No branches or pull requests