From aabe888c809305873e933b90e9e54afd6f7e3916 Mon Sep 17 00:00:00 2001 From: Varun Date: Sat, 9 May 2026 02:38:09 +0530 Subject: [PATCH 1/2] [FocusTrap] Fix tab order when children have positive tabIndex (#36479) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set sentinelStart tabIndex to 1 instead of 0 so it takes priority over positive-tabIndex children inside the trap. Previously, tabbing past the last element with tabIndex > 0 would reach the sentinel only after all other tabIndex=0 elements — including elements outside the trap — allowing focus to escape. --- .../src/Unstable_TrapFocus/FocusTrap.test.tsx | 25 +++++++++++++++++++ .../src/Unstable_TrapFocus/FocusTrap.tsx | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx index a8b6813a7871cf..830b56e1772bea 100644 --- a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx +++ b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx @@ -401,6 +401,31 @@ describe('', () => { expect(screen.getByRole('textbox')).toHaveFocus(); }); + it('should keep focus inside when children have positive tabIndex', async () => { + render( +
+ + +
+ +
+
+
, + ); + + await act(async () => { + screen.getByTestId('positive-tab').focus(); + }); + expect(screen.getByTestId('positive-tab')).toHaveFocus(); + + // Attempting to move focus outside should be trapped + await act(async () => { + screen.getByTestId('outside-input').focus(); + }); + expect(screen.getByTestId('outside-input')).not.toHaveFocus(); + }); + it('should trap once the focus moves inside', async () => { render(
diff --git a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx index f3ee77341a8e7e..1136af3c082478 100644 --- a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx +++ b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx @@ -352,7 +352,7 @@ function FocusTrap(props: FocusTrapProps): React.JSX.Element { return (
Date: Tue, 12 May 2026 00:05:23 +0530 Subject: [PATCH 2/2] [FocusTrap] Fix Tab from positive-tabIndex elements escaping the trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sentinel approach alone is not enough when children have positive tabIndex. After the last positive-tabIndex element inside the trap, the browser's natural Tab target is the first tabIndex=0 element in document order — which can be outside the trap (e.g. elements rendered before the modal in the DOM). Fix: In loopFocus, intercept forward Tab from any positive-tabIndex element inside the trap and redirect to the next entry in getTabbable() order instead, preventing the brief focus escape. Also fix the test: the previous version called .focus() directly on outside-input, which contain() already handles regardless of the fix. The new test fires a keydown Tab event, which exercises the loopFocus path that the fix adds. --- .../src/Unstable_TrapFocus/FocusTrap.test.tsx | 12 +++++---- .../src/Unstable_TrapFocus/FocusTrap.tsx | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx index 830b56e1772bea..0c70461a58dd09 100644 --- a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx +++ b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.test.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; import { expect } from 'chai'; -import { act, createRenderer, reactMajor, screen } from '@mui/internal-test-utils'; +import { act, createRenderer, fireEvent, reactMajor, screen } from '@mui/internal-test-utils'; import FocusTrap from '@mui/material/Unstable_TrapFocus'; import Portal from '@mui/material/Portal'; import { FOCUSABLE_ATTRIBUTE } from '../utils/focusable'; @@ -401,7 +401,7 @@ describe('', () => { expect(screen.getByRole('textbox')).toHaveFocus(); }); - it('should keep focus inside when children have positive tabIndex', async () => { + it('should keep focus inside when Tab is pressed from a positive-tabIndex element', async () => { render(
@@ -419,11 +419,13 @@ describe('', () => { }); expect(screen.getByTestId('positive-tab')).toHaveFocus(); - // Attempting to move focus outside should be trapped + // Without the fix, forward Tab from a positive-tabIndex element jumps to + // the first tabIndex=0 element in document order — which is outside-input (outside the trap). + // The fix intercepts the Tab keydown and redirects to the next tabbable inside the trap. await act(async () => { - screen.getByTestId('outside-input').focus(); + fireEvent.keyDown(screen.getByTestId('positive-tab'), { key: 'Tab', bubbles: true }); }); - expect(screen.getByTestId('outside-input')).not.toHaveFocus(); + expect(screen.getByTestId('normal-tab')).toHaveFocus(); }); it('should trap once the focus moves inside', async () => { diff --git a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx index 1136af3c082478..09c3ffc4290f94 100644 --- a/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx +++ b/packages/mui-material/src/Unstable_TrapFocus/FocusTrap.tsx @@ -233,6 +233,32 @@ function FocusTrap(props: FocusTrapProps): React.JSX.Element { if (sentinelEnd.current) { sentinelEnd.current.focus(); } + return; + } + + // Forward Tab from a positive-tabIndex element inside the trap. + // After all positive-tabIndex elements, the browser's natural next focus target + // is tabIndex=0 elements in document order — which may be outside the trap. + // Intercept and redirect to the correct next element inside the trap. + if ( + !nativeEvent.shiftKey && + rootRef.current !== null && + contains(rootRef.current, activeElement) && + (activeElement as HTMLElement).tabIndex > 0 + ) { + const tabbable = getTabbable(rootRef.current); + const currentIndex = tabbable.indexOf(activeElement as HTMLElement); + if (currentIndex !== -1) { + nativeEvent.preventDefault(); + const nextEl = tabbable[currentIndex + 1]; + if (nextEl) { + nextEl.focus(); + } else { + // Last tabbable element — loop back via sentinelEnd + ignoreNextEnforceFocus.current = true; + sentinelEnd.current?.focus(); + } + } } };