Bug with selecting jumping to first tabbable from last if web component is involved.

[simonxabris]

Description

I found an issue where if you put a focus trap on a web component, like you have a my-modal element, and also add tabindex="0" to it to make it focusable/tabbable, when you cycle through the tabbable elements within the web component with Tab, you cannot jump back to the first element from the last.

Expected behaviour

When I hit Tab on the last element, the containing web-component, which is the first tabbable element is focused

Actual behaviour

Hitting Tab on the last element seemingly does nothing

Reproduction

More detailed steps on how you can see the issue are provided in this stackblitz

https://stackblitz.com/edit/vitejs-vite-mxdsxb?file=index.html

Suggestion

I identified that the issue comes from here:

focus-trap/index.js

Lines 423 to 425 in ed95b7c

	if (node === doc.activeElement) {
	return;
	}

The node that we want to focus is going to be the web-component itself, and document.activeElement will also be the web-component, but that's not really true, because inside its shadow-root, there's another activeElement, but that's not visible from the outside this way.

My proposed solution is that when doing this check, it should recursively check if document.activeElement has a shadow-root and if it does, check for the activeElement inside that.

If someone from the project can confirm that this is a bug and my solution seems okay, I'm happy to submit a PR. If you think another solution would work better, I'm happy to work on that as well.

[stefcameron]

@simonxabris Thanks for bringing this to my attention, and for providing a repro sandbox. Very helpful.

It's interesting that it works in reverse, but not forward direction. It gets stuck going forward, but doesn't going backward.

The other thing I notice is that when the trap is first activated, the focusable/tabbable <my-calendar> slotted element doesn't show its focus rectangle, while I would expect it to since it's technically the first tabbable element inside the trap, which has the <my-modal> element as its root. I wonder why that is. Maybe it's focused but the focus rectangle just isn't showing, or maybe it's not focused (but it should be). Yet when going backward, it clearly gets focused and gets the rectangle around it.

In your investigation (which I also appreciate!), what has happening when pressing TAB with focus on "Day 3" (after which we'd expect it to go to <my-calendar>)?

I would expect we would be in checkKeyNav() and called findNextNavNode() -- does it return a destinationNode?

I'm suspicious of what might be happening with getTabIndex(target) here:

focus-trap/index.js

Line 564 in ed95b7c

destinationNode =

[simonxabris]

When you initiate the focus trap in the reproduction, according to document.activeElement, the focused element is my-calendar and document.activeElement.shadowRoot.activeElement is null, so my-calendar is in fact the focused element. Why the focus indicator is not showing I have no idea sadly, I'd say that's a weird browser behaviour.

I inspected the line you linked and yes it does set the destinationNode there, at it sets it to my-calendar. I think everything works as it should up to the point that I linked, where the library does not check if the activeElement has a shadow-root that has an activeElement inside it.

Thanks for the quick response by the way, le me know if you need any additional info! Also as I said before, I'm down to submit a PR if we can find the right solution.

[stefcameron]

@simonxabris Thank you for explaining further. I think I have a clearer understanding of the issue:

• Because of the shadow DOMs at play here, from the outside (Focus-trap's perspective), the active element is always my-calendar while the actual active element is a child hidden inside the shadow.
• Tabbable digs into the shadows to find all the elements, so when we're going backward, we know we should be focusing Day 3 and that doesn't match my-calendar at the line you pointed out (node === doc.activeElement) so tryFocus(node) calls node.focus() and it works.
• But when going forward, the node after Day 3 is my-calendar and because of the node === doc.activeElement, tryFocus(node) thinks it's already focused and does nothing -- yet it's not actually focused in terms of the shadow DOM.

So yes, I think your suggestion of looking into the active element on line 423 to see if there's a shadow would be a good solution and I'd certainly welcome the PR you've offered to make! 😄

[simonxabris]

Yep, what you described is exactly what's happening! I'll try to submit that PR in the coming days

[stefcameron]

@simonxabris Your fix has been published in v7.5.4!