WICG / webcomponents

Web Components specifications
Other
4.36k stars 370 forks source link

In Shadow DOM v1, tabIndex=-1 makes all elements inside the shadow tree to be no tab-focusable #774

Open hugoholgersson opened 5 years ago

hugoholgersson commented 5 years ago

An inconsistency in Shadow DOM v1 has started to confuse web developers:

From crbug.com/906729:

... it seems that there is a spec bug now. tabindex -1 behaves differently on a non-Custom Element (like a \<div>) than it does on a Custom Element.

Custom Element: tabIndex -1 removes "tab focusability" for everything inside the custom element. A [scrollable] \<div>: tabIndex -1 removes "tab focusability" for the [scrollable] \<div> itself but not its content.

annevk commented 5 years ago

Is this with a shadow tree or not?

freshp86 commented 5 years ago

Is this with a shadow tree or not?

With a shadow tree. Also note that this behavior changed from Shadow DOM v0 to v1. See minimal examples at https://bugs.chromium.org/p/chromium/issues/detail?id=896624#c5 (the examples use Polymer, but they can be made without Polymer as well if it helps).

annevk commented 5 years ago

In that case this is really about delegatesFocus and the various focus related issues already on file: https://github.com/whatwg/html/issues/2013.

hugoholgersson commented 5 years ago

I guess I don't understand how delegatesFocus is supposed to work and why the introduction of delegatesFocus changed tabIndex's semantics. I read #399 changed tabIndex's semantics because we wanted a feature "make everything of this custom element/shadow tree non-tab-focsable"? But isn't that what delegatesFocus=false does for us?

My intuition:

The shadow host could then be a \<div> where Shadow DOM content overflows (a scrollable div). Still, an author could decide to eject the \<div> from the tab order but leave its [shadow] content in.

annevk commented 5 years ago

Your intuition matches mine, but I haven't looked at focus in a while. What goes amiss?

hugoholgersson commented 5 years ago

Because of #399, an author can not decide to eject the host \<div> from the tab order but leave its [shadow] content in.

annevk commented 5 years ago

That issue doesn't seem to consider delegatesFocus though (and delegatesFocus isn't defined in detail). It seems to me it discusses the default (when delegatesFocus is false).

freshp86 commented 5 years ago

/cc @hayatoito

@annevk According to the comment at https://bugs.chromium.org/p/chromium/issues/detail?id=896624#c15, and the tests at https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/shadow-dom/focus-navigation-with-delegatesFocus.html?l=106-138, delegatesFocus has no effect on whether the children of a shadow root are in the tab order. Notice that the assertions are the same for both test cases.

hayatoito commented 5 years ago

@freshp86, yeah, I think that is intentionally done in v1.

if the shadow host has -1 tab index, any node in a shadow root is skipped, as far as I can read.

caridy commented 5 years ago

if the shadow host has -1 tab index, any node in a shadow root is skipped, as far as I can read.

I want to stress that the semantics described by @hayatoito is what we believe is the right model. This should not be about delegatesFocus, this is about tabindex. It is also not about -1 or 0, it is about the possibility of using any other value. If you make this about delegatesFocus, it will become really easy for anyone to create a component without delegatesFocus flag for its shadow, and any arbitrary tabindex value that will disrupt the page where this component is used. E.g.:

<template>
     <input tabindex="1" />
</template>

That should never disrupt the page, no matter what, it is bound to whatever tabindex value its host has, independently of the delegatesFocus, otherwise authors can bypass what the consumer of the component is attempting to define. This matches the semantics of the native components.

freshp86 commented 5 years ago

@caridy: Thanks for this explanation, it does make sense. There is still the following problem though:

The combination of the described tabindex behavior along with the "Keyboad accessible scroll containers" as implemented here (which I assume also follows some other spec?) make it impossible to have a CustomElement where

Is that not considered a valid use case? Are authors forced to always wrap their contents with a div and add tabindex -1 to achieve that?

@hugoholgersson: Are the scrolling containers changes based on a spec or is it just matching Firefox's behavior but not explicitly specced? Perhaps treating shadowRoots differently in the "keyboard accessible scroll containers" implementation is the right approach here? Such that they are not added in the tab order silently?

it will become really easy for anyone to create a component without delegatesFocus flag for its shadow

Just a thought about this: Perhaps providing a way for consumers of a component to modify this behavior would alleviate this problem? Even if an author created a "bad" component, consumers could "fix" that behavior on their end, for example <my-element delegates-focus>...</my-element>?

hugoholgersson commented 5 years ago

@freshp86 The current HTML spec does not require tab-focusable scrollers but I hope WebKit and Edge pick it up so we can spec it (it has clear accessibility benefits).

annevk commented 5 years ago

@caridy there's a difference between enabling elements inside a shadow tree being "tab-focusable" and letting that tree override the tab order of the light tree, no?

caridy commented 5 years ago

@freshp86

Just a thought about this: Perhaps providing a way for consumers of a component to modify this behavior would alleviate this problem? Even if an author created a "bad" component, consumers could "fix" that behavior on their end, for example ...?

I don't think we want this, it is basically inversion of control on something that is an internal implementation detail.

@annevk

@caridy there's a difference between enabling elements inside a shadow tree being "tab-focusable" and letting that tree override the tab order of the light tree, no?

That's a very interesting question. I haven't play much with tab-focusable elements in light dom, maybe @davidturissini and @SiTaggart has some opinions here, but my intuition here is:

Side note: in our platform, this is not a problem because we intentionally only allow tabindex=0 and -1 in all authored web components, and only high-privilege code (usually app level code) can bend that rules (e.g.: panels and such that might require some specific behavior).

SiTaggart commented 5 years ago

To me, tabindex only affects the node, not the subtree.

I may wish to have a programmatically focusable custom element host, with focusable children. I would achieve that by placing tabindex="-1" on the host, so it's not in my natural tab order, but is available to me to place the users focus on if I choose to, in the user flow / interaction.

It shouldn't also remove all subtree items from being focusable too, even if they are in that hosts shadow. That should be left up to delegateFocus IMO

JoelEinbinder commented 5 years ago

This issue is titled about custom elements, but seems to actually be about Shadow DOM. I have a lot of standard divs that are shadow hosts. The divs have lots of input elements in their shadow tree. Sometimes I want to assign programmatic focus to the shadow host itself, in order to control where the next tab/shift-tab will send the user and allow mouse focus. So I set manually tabIndex=-1 on the shadow host div. In v0 that worked great, the same as standard DOM. In v1, all of the input elements inside the shadow host are removed from the tab order.

document.body.appendChild(document.createElement('input'));
document.body.appendChild(document.createElement('input'));
const shadowHost = document.createElement('div');
shadowHost.textContent = 'shadow';
document.body.appendChild(shadowHost);
shadowHost.tabIndex = -1;
const shadowRoot = shadowHost.attachShadow({mode: 'open'});
shadowRoot.appendChild(document.createElement('slot'));
shadowRoot.appendChild(document.createElement('input'));

I wanted to make my div more focusable, and it ends up removing tab order from all of its children?!

hugoholgersson commented 5 years ago

I think I understand how @TakayoshiKochi wanted to circumvent this problem:

@JoelEinbinder, what if you set delegatesFocus: true instead of tabindex=-1?

That should both make your shadow host div appear tab- and click-focusable; the div will immediately forward focus to its shadow tree's tab order's first element*. Read his explanation about delegatesFocus for the details.

* The shadow tree's tab order's last element, when Shift+TAB focus the shadow host div.

jhausler1266 commented 5 years ago

It seems wrong that it would be impossible to make a custom element programmatically focusable and not in the user tab flow, without removing the entire shadow tree from tab flow.

If I explicitly set delegateFocus on the component, I’d expect the tabIndex value to propagate down the shadow tree because I explicitly told it to, but not implicitly delegateFocus down the shadow tree.

rniwa commented 5 years ago

Setting tabindex=-1 on a shadow host has the effect of disabling the focus of the component. We have builtin elements like input and video elements, and if tabindex=-1 is set on either element, then its content including buttons and media controls aren't focusable. That is, the intention of setting tabindex=-1 is to make that particular reusable element not focusable including its implementation details.

Using tabindex=-1 instead of delegatesFocus for focus delegation is a wrong thing to do, and we should really add delegatesFocus back to the HTML specification.

JoelEinbinder commented 5 years ago

That is, the intention of setting tabindex=-1 is to make that particular reusable element not focusable including its implementation details.

That is not true. It doesn't make the element not focusable, it removes it from the tab order. Clicking still moves focus, and assistive technology can still focus the element.

tabIndex=-1 is a weird quirk of the web. But making a shadow root shouldn't transfer me to a new world where focus works completely differently.

miki10194 commented 3 years ago

Finally is it any solution for it?