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

Reference Target: How should we treat invalid reference targets for relations set via Element IDL attributes? #1089

Open
alice opened this issue Dec 5, 2024 · 7 comments

Comments

@alice
Copy link
Member

alice commented Dec 5, 2024

#1071 discusses how invalid reference targets should be handled when computing the browser-internal ("true") target of an ID-based relationship. The consensus in that issue is that an invalid reference target:

  • should be treated browser-internally like an invalid ID, i.e. as if the attribute value was itself an invalid ID; and
  • should be treated likewise by read-only IDL attribute getters like label.control; i.e. those getters should return null even though an element with the ID in question technically exists in the same tree.

How should attributes set via IDL attributes work in this situation?

  1. I think you could make an argument that you should always get back the element that you set, assuming that element is a descendant of a shadow-including ancestor.
  2. However, I think you could also make an argument that it should only return the element that you set if it is also a valid value - i.e. that it has no referenceTarget set, or has a valid referenceTarget set. This would be analogous to how you can set an IDL attribute to be an element that is not a descendant of a shadow-including ancestor, but it won't be returned from the getter.

My personal slight preference would be for (2), though I could easily be persuaded otherwise. This would make those IDL attribute getters marginally slower, as we would need to check validity before returning the appropriate value. However, it would also give an indication that the relationship has not been set successfully, and work analogously to the read-only IDL attributes.


To address the original question in this issue, there seems to be a consensus based on the discussion on the earlier text of this issue, and also the WPT tests, that:

  • relationships set up via IDL attribute setters like:
    el.ariaLabelledByElements = [ customElementWithReferenceTarget ];
    should work identically to relationships set via content attributes like:
    <input aria-labelledby="id-for-custom-element">
    i.e. that the referenceTarget should be followed in both cases; and
  • when the reference target is valid, that the IDL attribute getters should return the element[s] that were set:
    const labels = [ customElementWithReferenceTarget ];
    el.ariaLabelledByElements = labels;
    el.ariaLabelledByElements[0] === labels[0];
    i.e. that the getters should not return elements inside shadow roots, even if those elements are the "true" target of the relationship.
Original text

The explainer proposes that IDL attribute getters not return the referenceTarget element, and I strongly agree there.

However, this leaves open the question of what should happen with IDL attribute setters.

<input id="input">
<fancy-listbox id="listbox">
  <template
    shadowrootmode="open"
    shadowrootreferencetarget="real-listbox"
  >
    <div id="real-listbox" role="listbox">
      <div id="option-1" role="option">Option 1</div>
      <div id="option-2" role="option">Option 2</div>
    </div>
  </template>
</fancy-listbox>
input.ariaControlsElements = [ listbox ];

In this case, should the internal aria-controls relationship (i.e. what is exposed to the accessibility tree) be with the <fancy-listbox>, or the <div role="listbox">?

@alice
Copy link
Member Author

alice commented Dec 10, 2024

Another question related to this and to #1071: what should happen in this case?

<input id="input">
<fancy-listbox id="listbox">
  <template
    shadowrootmode="open"
    shadowrootreferencetarget="BAD_ID"
  >
    <div id="real-listbox" role="listbox">
      <div id="option-1" role="option">Option 1</div>
      <div id="option-2" role="option">Option 2</div>
    </div>
  </template>
</fancy-listbox>
input.ariaControlsElements = [ listbox ];
console.log(input.ariaControlsElements);  // what does this log?

Should it log the fancy-listbox, even though the reference target is invalid? Or should it log an empty list, because the reference target is invalid?

@annevk
Copy link
Collaborator

annevk commented Dec 11, 2024

I think APIs should generally return the custom element (that handles the forwarding into its shadow tree). It seems weird if the APIs didn't work just because you used a custom element (with forwarding).

For the later question I think we have to answer another question. When do we consider a custom element a viable ID target?

  1. Always.
  2. Only when it has a reference target defined.
  3. Only when it has a valid reference target defined.

It seems potentially somewhat performance-sensitive to do 3, but not sure.

@annevk
Copy link
Collaborator

annevk commented Dec 11, 2024

This might also duplicate #1071?

@alice
Copy link
Member Author

alice commented Dec 12, 2024

I'm not quite sure I follow your answers, sorry @annevk.

The question I was trying to ask in the issue description, which I didn't explain very well in hindsight, was whether relationships set via Element IDL attributes (as opposed to content attributes which necessarily use ID strings) should also follow reference target forwarding when computing the relationships.

The explainer doesn't address this; it only says:

This feature is intended to work with all attributes that refer to another element by ID string.

and

These [IDL attribute getters] will never directly return the referenceTarget element that's inside the shadow tree.

The explainer is clear on how the IDL attribute getters should work when the referenceTarget is valid: they should return the element in the appropriate scope (i.e. the element which was set via the setter, which must be a descendant of a shadow-including ancestor of the element hosting the attribute).

While the explainer doesn't address how the IDL attribute setters should work, the WPTs indicate that the intent was for attribute set via IDL attributes to work similarly to attribute set via content attributes.

I think APIs should generally return the custom element (that handles the forwarding into its shadow tree). It seems weird if the APIs didn't work just because you used a custom element (with forwarding).

When you say the APIs should return the custom element, are you answering the later question in #1089 (comment), regarding whether the IDL attribute getters should return null/empty list for an invalid referenceTarget?

This might also duplicate #1071?

The reason I think that this is a slightly different question to #1071 is that #1071 (part 2) uses a getter-only API as an example. #1071 seems to conclude that an invalid referenceTarget is analogous to a bad ID, so even if the ID reference to the host is valid, the attribute is invalid. Because the attribute is invalid, the IDL getter returns null, just as it would if the content attribute was a bad ID.

With an IDL attribute setter, there is no ID involved, so I think you could make an argument that you should always get back the element that you set, assuming that element is a descendant of a shadow-including ancestor.

However, I think you could also make an argument that it should only return the element that you set if it is also a valid value, per my reasoning below. This would be analogous to how you can set an IDL attribute to be an element that is not a descendant of a shadow-including ancestor, but it won't be returned from the getter.

For the later question I think we have to answer another question. When do we consider a custom element a viable ID target?

I think a custom element can be a viable ID target when either:

  1. It is, itself, a viable ID target, and doesn't have a referenceTarget set.
    For example:

    <button popovertarget="custom">Show popover</button>
    <custom-popover id="custom">
      <template shadowrootmode="open">
        <!-- no referenceTarget -->
        Some popover content.
      </template>
    </custom-popover>
  2. Or, it contains a viable ID target, and has a valid referenceTarget set referring to the viable target.
    For example:

    <label for="custom">Given name</label>
    <custom-input id="custom">
      <template shadowrootmode="open" shadowrootreferencetarget="real-input">
        <input id="real-input">
      </template>
    </custom-input>

    Without the referenceTarget forwarding to the <input>, custom is not a valid target for for, as it is not a labelable element. It would be equally invalid whether the referenceTarget was missing, or was a bad ID, or referred to an element which was not a labelable element.

It seems potentially somewhat performance-sensitive to do 3, but not sure.

Depending on the implementation, this could be performance sensitive when resolving an ID target (note that this is different from either getElementById() or a query selector), or require extra storage to keep track of both the "shallow" and the "deep" version of each ID reference, plus update them on relevant mutations. However, I think in practice the performance difference should be negligible. It would at worst require a small number of extra lookups (two per shadow root - one to check the presence of a referenceTarget, and one to resolve it) when using certain DOM APIs.

Regardless of the decision we make for DOM APIs, we will still need to implement that validity logic for whatever the consequences are of the relation. For example, a for attribute on a <label> only has any effect when it refers to a labelable element. When determining the activation behaviour of the <label>, we will still need to determine what "deep" element it refers to.

@dandclark
Copy link
Contributor

I'm still a little uncertain on what the invalid ID behavior should look like, but assuming valid IDs I think attribute setters should cause referenceTarget associations to be followed. I don't see any reason not to support this, and it seems like it'd be confusing for developers if referenceTarget only worked when the element relationships were established via attribute.

@alice alice changed the title Reference Target: What about IDL attributes? Reference Target: How should we treat invalid reference targets for relations set via Element IDL attributes? Dec 15, 2024
@alice
Copy link
Member Author

alice commented Dec 15, 2024

@dandclark Yeah, I agree. I've updated the issue to focus more on the invalid referenceTarget question, and to make note of the consensus on the earlier question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants