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

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

Open
hugoholgersson opened this issue Nov 20, 2018 · 21 comments

Comments

@hugoholgersson
Copy link

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
Copy link
Collaborator

annevk commented Nov 20, 2018

Is this with a shadow tree or not?

@freshp86
Copy link

freshp86 commented Nov 20, 2018

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
Copy link
Collaborator

annevk commented Nov 21, 2018

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

@hugoholgersson
Copy link
Author

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:

  • If I want a custom element where neither the host nor its shadow content are tab-focusable: tabIndex=-1 on the host and delegatesFocus=false on the root.

  • If I want a custom element where the host isn't tab-focusable but its shadow content is:
    tabIndex=-1 on the host and delegatesFocus=true on the root.

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
Copy link
Collaborator

annevk commented Nov 21, 2018

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

@hugoholgersson
Copy link
Author

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

@annevk
Copy link
Collaborator

annevk commented Nov 21, 2018

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
Copy link

freshp86 commented Nov 21, 2018

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

aarongable pushed a commit to chromium/chromium that referenced this issue Nov 21, 2018
The original change at r608013 caused WebUI regressions, some of which are
unclear on how to fix because of a separate issue with tabindex -1 and Custom
Elements, WICG/webcomponents#774.

Need to address WebUI affected cases, before the flag is re-enabled by defalut.

Bug: 907284
Change-Id: Ie8a41df1df663d25cf06a0ed6ae42ac80512df0c
Reviewed-on: https://chromium-review.googlesource.com/c/1347429
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Reviewed-by: Dave Tapuska <[email protected]>
Cr-Commit-Position: refs/heads/master@{#610218}
@hayatoito
Copy link
Contributor

@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
Copy link

caridy commented Nov 22, 2018

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
Copy link

freshp86 commented Nov 22, 2018

@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

  • its shadowRoot is a scrollable container and
  • the shadowRoot itself is not in the tab order and
  • its children are in the tab order.

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
Copy link
Author

@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
Copy link
Collaborator

annevk commented Nov 22, 2018

@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
Copy link

caridy commented Nov 22, 2018

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

  • the shadow tree does not dictate the order of the light tree tab order (e.g.: <x-foo><input tabindex=2 /><x-foo> might be the first focusable element from x-foo, even though x-foo might have another tab-focusable element with tabindex=1)
  • the host dictates whether or not the tab-focusable elements in the light dom are reachable (e.g.: tabindex=-1 on the host indicates that those elements, along with the shadow tree elements are not part of the tab navigation.

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
Copy link

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
Copy link

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
Copy link
Author

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.

@rniwa rniwa changed the title tabIndex -1 behaves differently on non-Custom Elements In Shadow DOM v1, tabIndex -1 makes all elements inside the shadow tree to be no tab-focusable Jan 27, 2019
@rniwa rniwa changed the title In Shadow DOM v1, tabIndex -1 makes all elements inside the shadow tree to be no tab-focusable In Shadow DOM v1, tabIndex=-1 makes all elements inside the shadow tree to be no tab-focusable Jan 27, 2019
@jhausler1266
Copy link

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
Copy link
Collaborator

rniwa commented Jun 25, 2019

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
Copy link

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
Copy link

Finally is it any solution for it?

Cirras added a commit to Cirras/eomap-js that referenced this issue May 7, 2022
When experimental web platform features are enabled, the blink
KeyboardFocusableScrollers feature is enabled.
This causes scrollable container to become keyboard-focusable.
See: https://bugs.chromium.org/p/chromium/issues/detail?id=907284

Even trickier, setting tabindex to -1 on a web component causes
everything inside of it to become untabbable.
See: WICG/webcomponents#774

So the solution in this case is to wrap up the action-group component
with a plain old div with tabIndex -1, and let that div handle the
overflow/scrolling instead.
Cirras added a commit to Cirras/eomap-js that referenced this issue May 7, 2022
When experimental web platform features are enabled, the blink
KeyboardFocusableScrollers feature is enabled.
This causes scrollable container to become keyboard-focusable.
See: https://bugs.chromium.org/p/chromium/issues/detail?id=907284

Even trickier, setting tabindex to -1 on a web component causes
everything inside of it to become untabbable.
See: WICG/webcomponents#774

So the solution in this case is to wrap up the action-group component
with a plain old div with tabIndex -1, and let that div handle the
overflow/scrolling instead.
hanwen pushed a commit to GerritCodeReview/gerrit that referenced this issue Jul 8, 2022
Previously the screenreader readout was done by removing all rows except
the selected row from the tab flow by setting tabindex = -1, but this
broke tabbing through the other non-selected rows.

This method was not breaking tab flow in the gr-file-list because the
rows are not using shadow DOMs. see
WICG/webcomponents#774

This change restores all rows to the tab flow by always setting tabindex
to 0, and maintains the screenreader readout by focusing the row when it
gets selected. Verified with VoiceOver on Mac OS.

Release-Notes: skip
Google-Bug-Id: b/235477335
Change-Id: I48ef40f18ce061e50ef71bd4df0cf311a25e683b
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

10 participants