-
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
Design questions about IDREF/Element attribute reflection #4925
Comments
In the discussion we also had the constraint that the elements have the same node document. It would be hard to implement without cycle collector (https://blog.kylehuey.com/post/27564411715/cycle-collection has a reasonable introduction on the concept) otherwise. WebKit does not have a cycle collector. It's not entirely clear to me if same-document is good with HTML modules however. If you always cloned those elements into your document it's probably fine, but it's hard to predict usage patterns in advance. |
Could you help me understand the connection there? What would the failure case look like? |
Trying to address some of the open questions:
|
Thanks for the answers, and sorry for being so slow to respond.
Yes, thank you!
What rules are you referring to?
Sorry, I should have been clearer here. I was referring to a potential API for <label id="label">Address:</label>
<input id="input" type="text"> label.for = input; This would cause the <label id="label" for="input">Address:</label>
<input id="input" type="text"> If we then removed
Currently they are reasonably closely coupled, cf. the example above, and also the reverse direction e.g. (starting from the original html example above) label.setAttribute("for", "input");
console.log(label.for); // logs a reference to the input element However, there are some caveats:
I need to think about this some more. |
https://w3ctag.github.io/design-principles/ for instance.
This seems problematic if there's also an element with ID |
I’m not clear on what is problematic, could you clarify? (Sorry for not quoting properly, replying from phone) |
Well, the |
Hm, I think my example was confusing - the input had an ID of |
Ah, my bad. I don't really have a good sense for how to best overload the |
That being said, in answer to your question: if the ID can't be used (e.g. if an element doesn't have one, or it's in a different ID scope), the input.removeAttribute("id");
label.for = input;
console.log(label.getAttribute("for")); // "" If you remove the attribute, it removes the association as well: label.removeAttribute("for");
console.log(label.for); // null |
I wrote up some options for moving forward: https://gist.github.com/alice/174ae481dacdae9c934e3ecb2f752ccb |
Just to keep you all posted, I've been thinking about this problem more & consulting with more WebKit engineers about the potential implementation challenges. We're entering Thanksgiving break soon but we hope to give back a feedback soon. |
Here's an idea, from a discussion with @justinfagnani. We could introduce a concept of an opaque Element (or Node) reference. This would be a handle to an Element/Node which could be used in place of an Element/Node for this type of API. You could have a Document-level factory method, something like: document.getOpaqueReference(el); which would return a "weak" reference to the element, say Like Then, if something has access to something inside of a Shadow root, it can use this method to avoid leaking a reference when setting up a relationship. class XContainer extends HTMLElement {
constructor() {
super();
let shadow = this.attachShadow({mode: 'open'});
let input = document.createElement('input');
shadow.append(input);
const inputRef = document.getOpaqueRef(input);
this.ariaActiveDescendant = inputRef;
}
clear() {
// inputRef is weak, so this doesn't cause a memory leak:
this.shadowRoot.innerHTML = '';
}
} (In this instance, setting the relationship on Justin points out that, assuming an |
So OpaqueReference is a weak reference to an element? How does one get the element out of OpaqueReference? Call some method on Document? |
I would imagine it would be a one-way street: if you call The User Agent, however, could use it to look up the Node/Element internally. |
+1 If there's a use case for it, maybe there can be an API on ShadowRoot or ElementInternals to dereference an OpaqueReference from that element's shadow root. This would allow an element to send out and receive back a reference and do something valuable with it. |
Sorry about the delayed response. We've carefully reviewed the plausibility of implementing option (1) in https://gist.github.com/alice/174ae481dacdae9c934e3ecb2f752ccb. Implementing option (1) in WebKit would require a significant and complex rearchitecture of the way DOM objects are managed with our garbage collector. Since this is the only feature which requires such a rearchitecting, we’re not interested in pursuing this approach. We're also concerned that allowing cross-document referencing would result in accidental whole document leaks. Using nodes from another global object has other kinds of hazards like instanceof not working and prototype object being different. Given this, we don't think we should proceed with option (1). |
We met today to discuss the implementation issues mentioned in the above comment. It seems like some (but not all) of the issues are resolved if we disallow cross-document references. I added a discussion of the options we discussed to the gist - @rniwa could you possibly take a look and check I've accurately represented what we talked about? I also added a brief comment to the Option 1 "Cons" list to try and capture the gist of the issue with cross-document references:
Does that roughly capture the issue we discussed? |
I'm supportive to no cross-document references since 1) this is a simple design: easy to understand and easy to implement and 2) it matches aria-activedescendant behavior. Indeed, if aria-activedescendant element is moved to another document then all its references are recomputed I believe: if ID exists in a new document, then reference is updated to new element, if ID doesn't exist, then value is considered invalid. However I'm not clear on what happens/should happen with references when aria-acivedescendant element is moved to another document. Do the references are simply dropped or does behavior match aria-activedescendant, i.e. if a new document has an element with ID matching an old referred element ID, then use that element. The latter behavior perhaps is obscure, so I suppose I'd vote to simply drop references.
Which issues are left? |
@asurkov Nice to hear from you!
I agree, I would expect references to be dropped... unless there is an element in the new document which has a "classic" style IDREF attribute (i.e. set via content/ e.g.
I can't remember, unfortunately. |
thank you!
I suppose it's aligned with ARIA spec, so agreed no point to change the behaviour. Not sure though I can see a valid use case for this.
My understanding is this is the only issue left to sort out to resolve all concerns for the spec, and get the green light for implementations. Thus curious, how this issue can be resolved. What is the process? |
Sorry for the delayed response.
I agree those are two good options to pursue. However, the example for WeakReference version is not right. WeakReference doesn't become null just because it got moved to a different document. Instead, it depends on GC and can be null in the future if the element is no longer kept alive by anyone. So e.g. const iframe = document.body.appendChild(document.createElement('iframe'));
const doc = iframe.contentDocument;
let el = doc.body.appendChild(document.createElement('div'));
// not keeping a JS reference to the newly created element
el.ariaActiveDescendantElement = doc.body.appendChild(doc.createElement('div'));
console.log(el.ariaActiveDescendantElement); // definitely logs newly created element from above
iframe.remove();
setTimeout(() => {
// may or may not return null depending on whether the finalization has happed or not.
console.log(el.ariaActiveDescendantElement);
}, 1000); |
I created a new gist to capture the "1.1" proposal from the old gist, which we all seem to agree on. I filed a new issue #5401 to track the discussion around shadow roots. One remaining issue is the issue of garbage collection:
|
To sum up discussion points for the F2F meeting: We have a core use case for allowing attr-associated Elements to be a descendant of a "host" Element's shadow including ancestor (i.e relationships can cross into a "lighter" shadow DOM, but cannot penetrate a "darker" shadow DOM). The common motivating use case for this is an autocomplete widget that has a <custom-combobox>
#shadow-root (open)
| <!-- this doesn't work! -->
| <input aria-activedescendant="opt1"></input>
| <slot></slot>
<custom-optionlist>
<x-option id="opt1">Option 1</x-option>
<x-option id="opt2">Option 2</x-option>
<x-option id='opt3'>Option 3</x-option>
</custom-optionlist>
</custom-combobox> Note that for this use case, IDREFS are insufficient in order to set the relationship. There has so far been consensus that an Element should only be allowed to be an attr-associated Element if it belongs to the same document as the "host" Element (the one that has the relationship set). Element Reflection was discussed at TPAC last year, and we have a few open questions to address:
console.log(el.ariaActiveDescendantElement); // ???
console.log(el.getAttribute('aria-activedescendant')); // ?
let el = document.body.appendChild(document.createElement('div'));
el.ariaActiveDescendantElement = document.body.appendChild(document.createElement('div'));
el.ariaActiveDescendantElement.remove(); // do we gc now?
console.log(el.ariaActiveDescendantElement); // ???
console.log(el.getAttribute('aria-activedescendant')); // ???
let el = document.body.appendChild(document.createElement('div'));
el.ariaActiveDescendantElement = document.createElement('div');
console.log(el.ariaActiveDescendantElement); // ??? |
We discussed this as a group yesterday, and captured notes in the agenda document. In the discussion, we considered the scenario A→B, where node A references node B through element reflection, and B does not reference A. The conclusions of the discussion were:
If anyone who attended the meeting feels I haven't captured things correctly, please chime in. |
What was the reasoning for this decision, as opposed to returning B? |
The idea was that when B is not in a document, AT won't see / can't use the fact B being referenced by A so we'd like to match what AT sees in this case and return |
Was that discussion minuted? It would be helpful to understand the context and trade-offs here. |
Also, was there any consensus reached on how element references will interact with Shadow DOM? |
Just made a comment on Issue 5401 with the results of that conversation. |
#7934 was merged; closing. |
Relating to #3515 and #3917.
Relevant spec language from the current PR:
https://whatpr.org/html/3917/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:element
We discussed this during the Web Components meeting at TPAC (minutes).
@zcorpan summarised some requirements for the design of the API:
Here are what I think are the open questions:
<label>
for
relationship where the<label>
has been removed from the tree (or is yet to be added) and thus can't act as a label.a.foo = b
.ariaOwnsElements
?The text was updated successfully, but these errors were encountered: