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

Add element reflection counterparts to id-reference counterparts #3515

Closed
domenic opened this issue Feb 27, 2018 · 93 comments
Closed

Add element reflection counterparts to id-reference counterparts #3515

domenic opened this issue Feb 27, 2018 · 93 comments
Labels
accessibility Affects accessibility addition/proposal New features or enhancements impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@domenic
Copy link
Member

domenic commented Feb 27, 2018

HTML has several content attributes which associate two elements with each other:

  • label's for (single element)
  • output's for (multiple elements)
  • button/fieldset/input/object/output/select/textarea's form (single element)
  • input's list (single element)

Right now these are reflected into IDL attributes in a variety of ways:

  • label's for: DOMString IDL attribute directly reflecting the attribute
  • output's for: DOMTokenList IDL attribute directly reflecting the attribute
  • button/fieldset/input/object/output/select/textarea's form: readonly HTMLFormElement? IDL attribute that computes the form element in question
  • input's list: readonly HTMLElement? IDL attribute that computes the datalist element in question

We'd like to add a way for these kind of element associations to be done across shadow trees, and without depending on IDs. The basic idea is that we'd like to add a read-write labelEl.htmlForElement property; by setting it, you can create the label association for any element.

The details of this are a bit tricky. We used to have one model for this, used by the contextMenu global IDL attribute, but that was only ever implemented in one browser, and removed in f0f7a14. Nevertheless, it provides a good precedent. Some questions to answer when developing such a model (not exhaustive):

  • What happens to inputEl.htmlFor and the for="" content attribute if inputEl.htmlForElement is set to...
    • ... an element with an ID...
    • ... an element with no ID...
    • ... and the for="" content attribute was previously set
    • ... and the for="" content attribute was not previously set
  • Similar questions for outputEl.for, complicated by it being a DOMTokenList instead of a DOMString

I believe @alice and others had a proposed model for this. It's worth comparing it with the model in the commit that was removed.

The other tricky part is figuring out how to integrate with the readonly variants currently in place for button/fieldset/input/object/output/select/textarea's form and input's list. Can we extend them to be read-write with general semantics? I'm not sure. Also they don't end in Element :-/. Maybe they aren't as important to worry about, compared to label's for?

One big motivation for this is that ARIA would like to add cross-shadow-root-capable, non-ID-based element reflection for many of its properties. This will require both single-element (input's for) and list-of-elements (output's for) reflection.

@alice
Copy link
Contributor

alice commented Feb 27, 2018

Our model is to have a pattern where any attribute which can take an IDREF has a corresponding property which can take an element reference, and the element reference property takes precedence while not interfering with the IDREF property:

// Reflects to "aria-labelledby" DOM attribute
button.ariaLabelledBy = "id1";

// DOM result: <button aria-labelledby="id1">

// Takes precedence; does not affect reflected attribute value
el.ariaLabelledByElements = [el2, el3];

// DOM remains unchanged: <button aria-labelledby="id1">

This is fleshed out in detail in this pull request to the AOM explainer.

@domenic
Copy link
Member Author

domenic commented Feb 27, 2018

Great. Would you be up for contributing spec text to HTML's reflection section for those kind of semantics? And perhaps working through the questions with regard to how this integrates with the existing attributes I listed in the OP?

@alice
Copy link
Contributor

alice commented Feb 27, 2018

Absolutely! Where is best to start?

@domenic
Copy link
Member Author

domenic commented Feb 28, 2018

I did some thinking and I think the best plan is the following:

  • Work on the general framework for element reflection
  • Use it for input.htmlForElement and output.htmlForElements in this spec to validate that it works well
  • Leave the other cases in the OP for later:
    • We may be able to make input.list use the general framework, but it's not trivial because currently it only ever returns datalist elements; if the ID is set to a non-datalist element, it returns null. That's not needed for the htmlFor and ARIA cases so let's leave it out for now.
    • Given the complicated definitions of the .form attribute getters, the best we'll be able to do is add setters that behave similar to our general framework, leaving the getters with their current complex behavior.

Given this, we should be able to dive right in and just define the element reflection. To do that, write spec text in the similar style to all of the other reflection paragraphs in https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflect. The deleted f0f7a14 is probably a reasonable guide, although it has some style issues that we could deal with in review. Also the semantics there are probably different than the ones you're suggesting, but just in terms of paragraph structure etc. it can be helpful.

If you can get a pull request started with that kind of definition, we can collaborate on polishing it and getting it merged. Feel free to reach out if anything's unclear!

@alice
Copy link
Contributor

alice commented Feb 28, 2018

Hm - I don't think we're defining a type of reflection here at all. The DOMString IDREF attribute will continue to reflect to a string; a non-reflected property will sit alongside and take precedence.

I will try and make a PR to that effect...

@domenic
Copy link
Member Author

domenic commented Feb 28, 2018

Anything that ties together a property (IDL attribute) and an attribute (content attribute) is reflection. See the other examples in that section.

It's definitely possible for there to be multiple properties reflecting a single attribute, as long as their interaction is well-defined. For example, rel and relList, or className and classList.

@alice
Copy link
Contributor

alice commented Feb 28, 2018

The only interaction is precedence. The value of one does not in any way affect the value of the other, in either direction.

@alice
Copy link
Contributor

alice commented Feb 28, 2018

For example:

// Reflects to "aria-labelledby" DOM attribute
button.ariaLabelledBy = "id1";

// DOM result: <button aria-labelledby="id1">

console.log(el.ariaLabelledByElements);  // prints "[]"

@domenic
Copy link
Member Author

domenic commented Feb 28, 2018

Oh... That seems like unfortunate design :/. I was hoping there would be a way to get the element using the element API even when people use markup to create the associations, like was done in the contextMenu commit referenced above.

@alice
Copy link
Contributor

alice commented Feb 28, 2018

@cookiecrook was opposed to the reflection piece - perhaps he has more to say about it?

@alice
Copy link
Contributor

alice commented Feb 28, 2018

Meanwhile I could try and pull something together to describe the reflection situation we had discussed earlier to see how it flies 😄

@annevk
Copy link
Member

annevk commented Feb 28, 2018

This seems like a superset of #3219. For label in particular I still think

An alternative that might make sense and preserves the encapsulation boundary is that we allow a shadow host to be labeled, and that it can decide somehow who gets focus.

makes a lot of sense.

In fact, anything that crosses the shadow boundary without preserving encapsulation is likely a non-starter given past positions on that topic from Apple and Mozilla.

@domenic
Copy link
Member Author

domenic commented Feb 28, 2018

Encapsulation is preserved; you can only access elements you're given access to. Nothing "leaks" out of the shadow root; you can only assign htmlForElement to something you already have a reference to, i.e. not things in closed shadow roots (unless they are explicitly re-exposed by the author).

@annevk
Copy link
Member

annevk commented Feb 28, 2018

That is fair, but I think a design where you reference the shadow host and its shadow root gets to delegate is a lot cleaner and makes it easier to reuse components.

@domenic
Copy link
Member Author

domenic commented Feb 28, 2018

I disagree; I think that's a lot of extra machinery to add to the straightforward label association stuff in existence. If we want to make an element (which may or may not be a shadow host) labelable that is not currently one of the labelable ones, that should require opt-in of the sort being discussed in the web components repo right now, so that it can also get the many other properties that come along with it (e.g. being form associated or having form data).

@annevk
Copy link
Member

annevk commented Feb 28, 2018

I think all of those make sense on top of shadow roots. And allow for an experience that is equivalent to built-in elements. Which might also consist of multiple elements, and focus one of them when label-clicked, but don't reveal any of that to the outside.

@domenic
Copy link
Member Author

domenic commented Feb 28, 2018

They make sense on top of shadow roots, yes, but only because they also make sense on top of any element. (Or at least, any element which is designated to be form-associated; currently we do that by designating a certain class of elements, by local name, as form-associated. We don't decide that by whether or not they have a shadow root.)

@mikerissi

This comment has been minimized.

@cookiecrook
Copy link

@alice wrote:

@cookiecrook was opposed to the reflection piece - perhaps he has more to say about it?

Clarifying:

I'm fine with string attribute reflection and with defining element references. I was opposed to specifying element reference reflection outside the context of the larger web platform context. In other words, I am opposed to defining it in the ARIA spec, and even the the AOM spec, because the general pattern belongs in HTML, not in an accessibility-specific module.

I'm not opposed to element ref reflection if we can get implementors to agree on how it should work in the broader context.

@alice
Copy link
Contributor

alice commented Aug 9, 2018

This took me way too long, but I made a start on trying to write up the Element/IDREF reflection piece: https://github.com/whatwg/html/compare/master...alice:idref-reflection?expand=1

@domenic Could you possibly take a look and let me know if I'm on the right track? If so, I'll tackle the sequence<Element> case.

@domenic
Copy link
Member Author

domenic commented Aug 9, 2018

@alice probably best to send a pull request so I can do inline comments. High level comments:

  • No need to talk about the relationship between attr and attrElement. It's a bit confusing, because stating that something is done "as normal" makes one wonder---are there other parts of the spec that might not behave as normal, unless explicitly stated?
  • You can just define it based on type, not name, like all the other reflections are. So anything that does a reflection on an IDL attribute whose type is HTMLElement, just like the other sections talk about reflections on IDL attributes whose types are DOMString or similar.
  • State the "on getting" and "on setting" steps as an algorithm. In particular you can use things like "return", instead of "(stopping at the first point where a value is returned)" and "the IDL attribute attrElement must return null".
  • When there is no matching element with id, do you want to set the id content attribute to the empty string, or remove it entirely?

@alice
Copy link
Contributor

alice commented Aug 10, 2018

probably best to send a pull request
Will do.

No need to talk about the relationship between attr and attrElement.

To be clear, I'm describing a three way relationship between:

  • attr content attribute
  • attr IDL attribute (converted from snake-case to camelCase, which is not obvious here)
  • attrElement IDL attribute

Is there a better way to describe what's going on?

It's a bit confusing, because stating that something is done "as normal" makes one wonder---are there other parts of the spec that might not behave as normal, unless explicitly stated?

Should I just remove "as normal"?

You can just define it based on type, not name, like all the other reflections are. So anything that does a reflection on an IDL attribute whose type is HTMLElement, just like the other sections talk about reflections on IDL attributes whose types are DOMString or similar.

Right, but there are three different things here, so it's not like the other reflections.

State the "on getting" and "on setting" steps as an algorithm. In particular you can use things like "return", instead of "(stopping at the first point where a value is returned)" and "the IDL attribute attrElement must return null".

Will do - this language was copied from the original language, happy to bring it up to date.

When there is no matching element with id, do you want to set the id content attribute to the empty string, or remove it entirely?

Good question. The original spec (f0f7a14) had it being set to the empty string, but we needn't go with that. I think probably the empty string at least indicates that a value might exist.

Either way this is going to mess with everyone's accessibility linters, but perhaps the empty string will mess with them slightly less.

@domenic
Copy link
Member Author

domenic commented Aug 10, 2018

There are not three interrelated things here. There are two separate IDL attributes, both of which affect and are affected by the underlying content attribute. The description of the HTMLElement-typed one should not reference the other one. It should stand independently on top of the content attribute, just like the existing reflection algorithms do.

In other words, there's no need to mention the existing IDL attributes, because they already work as expected, via the existing definition plus the various sentences that say things like "the htmlFor IDL attribute reflects the for content attribute."

Your goal is to be able to insert a sentence like "the htmlForElement IDL attribute reflects the for content attribute", and have that be meaningful, because you've defined what "reflects" means for an IDL attribute whose type is HTMLElement.

@domenic
Copy link
Member Author

domenic commented Aug 10, 2018

Hmm, sorry, upon re-reviewing, I realized there's a deeper issue here. Which is that, as specced in your PR (either with or without with my suggested edits), the following will not work:

labelEl.htmlForElement = document.createElement("input");

That is:

  • Since the value being set doesn't have an ID attribute, what will happen if you follow the "on setting" algorithm in the PR is that it sets labelEl's for content attribute to the empty string.
  • Then, if you do console.log(labelEl.htmlForElement), following the "on getting" algorithm in the PR will return null, since there is no element with id equal to the empty string.

I assume the goal was to allow association beyond just with an ID, right? For shadow DOM and such.

In that case, we'd need some deeper surgery.

Here's one approach:

  • We want to make htmlForElement actually impact the element's labeled control computation. We choose to not have any impact on the for="" content attribute.
  • Thus, we add a new concept, e.g. "label elements can have an explicitly-set labeled control, initially null."
  • We define the getter for htmlForElement to return this label element's explicitly-set labeled control. And we define the setter to set its explicitly-set labeled control. We don't use any notion of "reflection" to define htmlForElement at all.
  • We update the algorithm for computing the "labeled control" (currently specified, somewhat confusingly, by the two paragraphs following "Except where otherwise specified by the following rules, a label element has no labeled control."). In particular we just add as a first step something like "If the label element has a non-null explicitly-set labeled control, then its labeled control is given by its explicitly-set labeled control."

I'm not sure this is the best approach, though. It has the following drawbacks:

  • Doesn't easily generalize, like we were hoping. Although in theory every attribute like this should have a counterpart concept (like "labeled control"), in practice editing all such concept-computation algorithms is annoying, assuming they exist at all.
  • Totally ignores the for="" content attribute, so even if you set htmlForElement to something that does have an ID, the for="" content attribute doesn't get updated. (And thus the htmlFor IDL attribute also doesn't get updated.)

I can try to think of something better tomorrow. It indeed may require intertwingling all three concepts as you were envisioning. Or maybe just generalizing the above framework to e.g. install an "explicitly set X-attribute element" on the element whenever such an _attr_Element IDL attribute exists. Thoughts and suggestions welcome, if you're able to get to this before me :).

@rniwa
Copy link

rniwa commented Aug 10, 2018

I'd repeat what I stated during an in-person meeting a while ago.

We'd be opposed to proposals in which elements inside a shadow tree would be exposed when using one of these script-based associations. That's a stop stopper issue for us.

@domenic
Copy link
Member Author

domenic commented Aug 10, 2018

Thanks rniwa. It's good to know not every browser would implement this feature.

@alice
Copy link
Contributor

alice commented Aug 10, 2018

@rniwa To clarify, I recall you saying that setting a relationship in the other direction - i.e. from inside a shadow root to an element in the host's tree - was ok. Is that still the case, and/or did I mis-remember?

@rniwa
Copy link

rniwa commented Aug 10, 2018

Yes, if a node A is in a shadow tree of shadow host B, then accessing any node C which is in the same tree of B or its ancestor tree (i.e. only getting out of a shadow tree but not getting in) is okay.

@jessebeach
Copy link
Contributor

Let me rephrase: if you set attrElement to null, it removes the attr content attribute.

@annevk would that trigger a DOM mutation event?

@smaug----
Copy link

Mutation events aren't spec'ed, but yes, changing a DOM attribute should fire a mutation event and create a MutationRecord too (for MutationObserver).

@jessebeach
Copy link
Contributor

@alice I'm having a hard time understanding what attrElement refers to. Could you add an example to the summary?

@alice
Copy link
Contributor

alice commented Sep 4, 2018

@jessebeach Updated the long "summary", please take a look!

#3515 (comment)

@robdodson
Copy link

Reading through the summary (thank you Alice!) I think I'm leaning toward option 1.

I can use attr for any SSR work. But once my client side js has booted I can use attrElement both for querying (since it is a source of truth) and for setting (since it takes Element refs).

Like @azakus, I don't trust attributes on the client and I'd always refer to the property. I only think attributes are useful for initial configuration, which option 1 seems to solve.

The legacy warts are a huge bummer... I guess I lean toward not compromising the design of the new thing for the sake of those warts.

@alice
Copy link
Contributor

alice commented Sep 4, 2018

Option 1 would work with the legacy attributes as well, e.g.

console.log(el.htmlFor);         // logs the string
console.log(el.htmlForElement);  // logs the element

@jessebeach
Copy link
Contributor

Agree with @alice and @robdodson (and others in the backscroll who condone Option 1). We know not to trust HTML attributes once they're been parsed to DOM.

One snafu that just came to mind though. Arg, I'm bummed to even bring this up.

Not all desktop assistive tech software interacts with the platform APIs. Some, like JAWS + IE, still scrape the HTML for information. At least, I think they scrape the HTML and not the DOM. @mcking65 might now the specifics of this behavior.

@alice
Copy link
Contributor

alice commented Sep 5, 2018

I'm confused as to whether you mean option 1 or option 2a.

  • Option 1 has both string and element reflection.
  • Both write back to the content attribute where possible.

Could you possibly clarify the comments around trusting content attributes, since it seems to affect any of the options I outlined?

@robdodson
Copy link

I spoke with Alice about option 2a and I think it's what I want.

I can set attr to a string for initial SSR, and after my JS boots, I can work with it like a regular javascript property. So long as el.attr is always an element reference or null on the client then I think I'd be happy.

@alice
Copy link
Contributor

alice commented Sep 5, 2018

Should we use option 1 for existing attributes, and 2a for new attributes, then?

@domenic
Copy link
Member Author

domenic commented Sep 6, 2018

That seems pretty harmonious to me.

In particular, if you ignore the presence of the .attr IDL attribute, the interaction between the attr="" content attribute and the .attrElement IDL attribute in (1) is the same as the interaction in (2a). Right?

@alice
Copy link
Contributor

alice commented Sep 6, 2018

Yup, exactly.

@alice
Copy link
Contributor

alice commented Sep 6, 2018

I guess a tiny question remains of whether the IDL attribute in 2a should be called .attr or .attrElement. I'm happy with either.

@domenic
Copy link
Member Author

domenic commented Sep 6, 2018

(Oh no, this thread got long enough that the most important post in it got hidden by default.)

Taking into account how we want to treat the existing 4 cases from the OP, I see a few options.

  • (A) attr for new things; attrElement for existing things (alongside their existing attrs)
  • (B) attrElement for new things and existing things (alongside existing attrs for existing things)
  • (C) attr for new things; try to be clever for existing things:
    • for="" (both label and output): Introduce a new for IDL attribute with the (1) semantics. Then we have for (new style) and htmlFor (old style)
    • form="": introduce a new setter, whose algorithm is the same as (1)'s setter. The getter behaves the same as it currently does, which is a lot more complicated than (1).
    • list="": introduce a new setter, whose algorithm is the same as (1)'s setter. The getter behaves the same as it currently does, which is a tiny bit more complicated than (1)---it returns null if the target element is not a <datalist>. (Maybe we even want this return-null-when-target-is-invalid semantic more generally, so this would no longer be a special case?)

(C) has the advantage of surface simplicity and teachability: every one of these element-association content attributes has a corresponding IDL attribute which is named the same as the content attribute and works on elements, not strings. This is a nice story, with only a bit of lurking complexity. You have to ignore the existence of htmlFor, and ignore some of the edge-case complexities of the .form and .list getters, but that's not too bad.

Note that if we did (C) I think we'd only create a general framework for new things, and we'd just add custom in-place spec text for the getters/setters of the existing things.

@alice
Copy link
Contributor

alice commented Sep 6, 2018

So output is a slightly different case, because it's a list, not a single IDREF. My suggestion for lists is to set an empty string for the content attribute unless (at setting time) every single element has a usable ID (i.e. exists and is correctly scoped). Sound ok?

Also, I thought .for was ruled out because for is a JS keyword?

Other than those things, all of those options sound fine to me, so happy to go with your preference (I guess (C)?)

@domenic
Copy link
Member Author

domenic commented Sep 6, 2018

My suggestion for lists is to set an empty string for the content attribute unless (at setting time) every single element has a usable ID (i.e. exists and is correctly scoped). Sound ok?

Sounds good!

Also, I thought .for was ruled out because for is a JS keyword?

Nope. That was a restriction back in the 90s when the DOM API was first created, but hasn't been a problem for many years now.

Other than those things, all of those options sound fine to me, so happy to go with your preference (I guess (C)?)

Yeah, I like (C), pending others' feedback. I can try to help with the one-offs it entails. It seems like the core is speccing the (2a) framework for new stuff. We'll lose our guinea pig of for="" in the HTML spec, and instead need to use new stuff (ARIA stuff, I assume) to prove out the general framework. But that seems fine.

@alice
Copy link
Contributor

alice commented Sep 7, 2018

To @jessebeach's earlier point - I think we just have to live with working around legacy tools' behaviour. Steve Faulkner had some thoughts on this topic.

I'd be interested to hear from @mcking65 which, if any, tools scrape content (surely they break badly with Shadow DOM as well, in that case?)

ChromeVox classic/legacy was one example of a DOM-based tool, but it's now deprecated and the new version of ChromeVox on Chrome OS operates via the accessibility tree.

I seem to recall Dragon Naturally Speaking was slow to adopt ARIA, indicating they are also scraping the DOM - but they did eventually add support.

@jnurthen
Copy link
Member

@alice as far as I'm aware Dragon still doesn't use the accessibility tree - it uses a browser extension to scrape the DOM.

@alice
Copy link
Contributor

alice commented Jan 3, 2019

@jnurthen Right, that was my hypothesis. I'm of the opinion, based on Steve's reasoning, that we shouldn't block features based on that fact. What do you think?

@alice
Copy link
Contributor

alice commented Jan 3, 2019

I updated the PR to just (for now) deal with reflection of Elements and sequences of Elements, in general.
It's still a bit of a work in progress - I think I still need some way of referring to the Element/Elements set via the setting steps; in particular, to disallow setting the value if it would break encapsulation.

@domenic, @annevk and/or @rniwa, would you care to take a look at what is there so far? https://github.com/whatwg/html/pull/3917/files

@domenic
Copy link
Member Author

domenic commented Jan 7, 2019

Had an offline discussion with @alice spurred by the latest review in #3917 (review).

To recap, in #3917 she's speccing 2(a) from her post at #3515 (comment). This involves only a single reflecting Element-valued property, call it el.attrElement; we're setting aside any interaction with a DOMString-valued property for now.

We discovered something a bit unexpected about that, which I want to document the reasoning for here, either for discussion or just for the record.

The question is, what happens in these scenarios?

// (1)
el.setAttribute("attr", "some-id");
el.attrElement = $(".no-id");
// (2)
el.attrElement = $(".no-id");
el.setAttribute("attr", "some-id");

The claim is that we expect "last one wins" behavior:

    • The attr="" content attribute gets reset to the empty string (or deleted?)
    • The el.attrElement getter returns what it was set to, i.e. the $(".no-id") element
    • The attr="" content attribute gets set to "some-id"
    • The el.attrElement getter returns the element whose ID is "some-id"

The alternatives to last-one-wins are either:

  • Content attribute always wins; attrElement setter sets the ID if possible, or deletes otherwise
    • This could break shadow tree situations, and leads to situations where if the ID of the element gets mutated, attrElement will stop returning what it was set to.
  • Content attribute always wins; attrElement setter does not manipulate the content attribute
    • This makes the attrElement setter useless until the author remembers to manually clear the content attribute
  • Internal slot always wins
    • This makes the content attribute useless unless you remember to clear the internal slot by setting attrElement = null first.

However, because we don't have a clear precedence order, last-one-wins means we need setting the attribute to affect the internal slot. We can't implement it with a simple getter/setter algorithms for el.attrElement that stand independently; we need to also have side effects when changing the attributes through APIs like setAttribute().

So, we need to say that any element with this reflection setup has attribute change steps that resets the internal slot value. This is the formalization of @alice's current text in #3917, which says "If the content attribute is set directly (such as via Element.setAttribute()), the value for the IDL attribute is reset to null."

This is a bit weird! But I hope with the above I've recorded why it's the best way to implement 2(a) semantics.

@annevk
Copy link
Member

annevk commented Nov 11, 2022

This was fixed by #7934.

@annevk annevk closed this as completed Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Affects accessibility addition/proposal New features or enhancements impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation topic: shadow Relates to shadow trees (as defined in DOM)
Development

No branches or pull requests

12 participants