Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: remove stale aria-hidden from dialog portal ancestors when focusing on open, addressing focus trap breakage when closing stacked non-nested sibling dialogs (https://github.com/microsoft/fluentui/issues/35985)",
"packageName": "@fluentui/react-dialog",
"email": "Hotell@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,200 @@ describe('Dialog', () => {
cy.get('#second-dialog').should('not.exist');
cy.get('#first-dialog').should('not.exist');
});

describe('stacked non-nested dialogs (sibling)', () => {
/**
* Regression test for https://github.com/microsoft/fluentui/issues/35985
*
* Two sibling Dialogs (NOT nested inside each other's JSX tree).
* Dialog 1 is opened via a page-level trigger.
* Dialog 2 is opened via a button inside Dialog 1, but is a sibling in the React tree.
* When Dialog 2 closes, focus must return to the button inside Dialog 1 — NOT to the page trigger.
*
* Bug: Dialog 2's Modalizer leaves stale aria-hidden="true" on Dialog 1's portal mount node,
* blocking browser focus restoration back into Dialog 1.
*/
it('should restore focus to underlying dialog when top stacked dialog closes', () => {
const StackedDialogsTest = () => {
const [dialog1Open, setDialog1Open] = React.useState(false);
const [dialog2Open, setDialog2Open] = React.useState(false);

return (
<>
<Button id={dialogTriggerOpenId} onClick={() => setDialog1Open(true)}>
Open Dialog 1
</Button>

{/* Dialog 1 — opened from the page-level trigger */}
<Dialog open={dialog1Open} onOpenChange={(_, data) => setDialog1Open(data.open)}>
<DialogSurface id="dialog-1-surface">
<DialogBody>
<DialogTitle>Dialog 1</DialogTitle>
<DialogContent>Dialog 1 content</DialogContent>
<DialogActions>
<Button id="open-dialog-2-btn" onClick={() => setDialog2Open(true)}>
Open Dialog 2
</Button>
<Button id={dialogTriggerCloseId} onClick={() => setDialog1Open(false)}>
Close Dialog 1
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>

{/* Dialog 2 — sibling in the React tree, NOT nested inside Dialog 1 */}
<Dialog modalType="alert" open={dialog2Open} onOpenChange={(_, data) => setDialog2Open(data.open)}>
<DialogSurface id="dialog-2-surface">
<DialogBody>
<DialogTitle>Dialog 2 (alert)</DialogTitle>
<DialogContent>Dialog 2 content</DialogContent>
<DialogActions>
<Button id="close-dialog-2-btn" onClick={() => setDialog2Open(false)}>
Close Dialog 2
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
</>
);
};

mount(<StackedDialogsTest />);

// Open Dialog 1
cy.get(dialogTriggerOpenSelector).realClick();
cy.get('#dialog-1-surface').should('exist');

// Open Dialog 2 from inside Dialog 1
cy.get('#open-dialog-2-btn').realClick();
cy.get('#dialog-2-surface').should('exist');

// Close Dialog 2
cy.get('#close-dialog-2-btn').realClick();
cy.get('#dialog-2-surface').should('not.exist');

// Dialog 1 should still be open and the trigger button for Dialog 2 should have focus
cy.get('#dialog-1-surface').should('exist');
cy.get('#open-dialog-2-btn').should('be.focused');
});

it('should not have stale aria-hidden on dialog 1 portal ancestors after dialog 2 closes', () => {
const StackedDialogsTest = () => {
const [dialog1Open, setDialog1Open] = React.useState(false);
const [dialog2Open, setDialog2Open] = React.useState(false);

return (
<>
<Button id={dialogTriggerOpenId} onClick={() => setDialog1Open(true)}>
Open Dialog 1
</Button>
<Dialog open={dialog1Open} onOpenChange={(_, data) => setDialog1Open(data.open)}>
<DialogSurface id="dialog-1-surface">
<DialogBody>
<DialogTitle>Dialog 1</DialogTitle>
<DialogActions>
<Button id="open-dialog-2-btn" onClick={() => setDialog2Open(true)}>
Open Dialog 2
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
<Dialog modalType="alert" open={dialog2Open} onOpenChange={(_, data) => setDialog2Open(data.open)}>
<DialogSurface id="dialog-2-surface">
<DialogBody>
<DialogTitle>Dialog 2</DialogTitle>
<DialogActions>
<Button id="close-dialog-2-btn" onClick={() => setDialog2Open(false)}>
Close
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
</>
);
};

mount(<StackedDialogsTest />);

cy.get(dialogTriggerOpenSelector).realClick();
cy.get('#open-dialog-2-btn').realClick();
cy.get('#close-dialog-2-btn').realClick();

// After Dialog 2 closes, no ancestor of Dialog 1's surface (up to body)
// should carry a stale aria-hidden="true" (the backdrop div is intentionally aria-hidden but is not an ancestor)
cy.get('#dialog-1-surface').then($el => {
let el = $el[0].parentElement;
while (el && el !== document.body) {
expect(el.getAttribute('aria-hidden'), `ancestor <${el.tagName}> should not be aria-hidden`).to.not.equal(
'true',
);
el = el.parentElement;
}
});
});

it('should maintain focus trap in dialog 1 after stacked dialog 2 is dismissed', () => {
const StackedDialogsTest = () => {
const [dialog1Open, setDialog1Open] = React.useState(false);
const [dialog2Open, setDialog2Open] = React.useState(false);

return (
<>
<Button id={dialogTriggerOpenId} onClick={() => setDialog1Open(true)}>
Open Dialog 1
</Button>
<Dialog open={dialog1Open} onOpenChange={(_, data) => setDialog1Open(data.open)}>
<DialogSurface id="dialog-1-surface">
<DialogBody>
<DialogTitle>Dialog 1</DialogTitle>
<DialogActions>
<Button id="open-dialog-2-btn" onClick={() => setDialog2Open(true)}>
Open Dialog 2
</Button>
<Button id={dialogTriggerCloseId} onClick={() => setDialog1Open(false)}>
Close Dialog 1
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
<Dialog modalType="modal" open={dialog2Open} onOpenChange={(_, data) => setDialog2Open(data.open)}>
<DialogSurface id="dialog-2-surface">
<DialogBody>
<DialogTitle>Dialog 2</DialogTitle>
<DialogActions>
<Button id="close-dialog-2-btn" onClick={() => setDialog2Open(false)}>
Close
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
</>
);
};

mount(<StackedDialogsTest />);

cy.get(dialogTriggerOpenSelector).realClick();
cy.get('#open-dialog-2-btn').realClick();
cy.get('#dialog-2-surface').should('exist');

// Close Dialog 2 via its close button
cy.get('#close-dialog-2-btn').realClick();
cy.get('#dialog-2-surface').should('not.exist');
cy.get('#dialog-1-surface').should('exist');

// Tab should cycle inside Dialog 1 (focus trap re-engaged)
cy.get('#open-dialog-2-btn').should('be.focused').realPress('Tab');
cy.get(dialogTriggerCloseSelector).should('be.focused').realPress('Tab');
cy.get('#open-dialog-2-btn').should('be.focused');
});
});
});

const lorem = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,39 @@ import type { DialogSurfaceElement } from '../DialogSurface';
import type { DialogModalType } from '../Dialog';

/**
* Focus first element on content when dialog is opened,
* Removes stale `aria-hidden="true"` from ancestor nodes of `element` up to (but not including)
* `document.body`.
*
* This is a temporary mitigation for a Tabster Modalizer limitation where closing a stacked sibling
* dialog can leave a stale `aria-hidden` on the underlying dialog's portal mount node, blocking
* browser focus from entering the subtree.
*
* The fix is safe because:
* - It only runs when `open === true` (the dialog's own Modalizer is active).
* - It only touches direct ancestors of the dialog surface — not unrelated siblings.
* - Tabster's active Modalizer will immediately re-apply correct `aria-hidden` state on the next
* mutation cycle if the attribute was legitimately supposed to be there.
*
* TODO: Remove once Tabster Modalizer supports a proper activation stack.
* @see https://github.com/microsoft/fluentui/issues/35985
*
* @internal
*/
function removeStaleAriaHiddenFromAncestors(element: HTMLElement): void {
let current = element.parentElement;
while (current && current.ownerDocument && current !== current.ownerDocument.body) {
if (current.getAttribute('aria-hidden') === 'true') {
current.removeAttribute('aria-hidden');
}
current = current.parentElement;
}
}

/**
* Focus first element on content when dialog is opened.
* Also clears stale `aria-hidden` from portal ancestor nodes before focusing,
* guarding against the stacked-sibling-dialog focus trap breakage described in
* https://github.com/microsoft/fluentui/issues/35985.
*/
export function useFocusFirstElement(
open: boolean,
Expand All @@ -21,11 +53,21 @@ export function useFocusFirstElement(
if (!open) {
return;
}
const element = dialogRef.current && findFirstFocusable(dialogRef.current);

const dialogEl = dialogRef.current;
if (!dialogEl) {
return;
}

// Workaround for https://github.com/microsoft/fluentui/issues/35985:
// Strip any stale aria-hidden="true" from ancestors of the dialog surface before focusing.
removeStaleAriaHiddenFromAncestors(dialogEl);

const element = findFirstFocusable(dialogEl);
if (element) {
element.focus();
} else {
dialogRef.current?.focus(); // https://github.com/microsoft/fluentui/issues/25150
dialogEl.focus(); // https://github.com/microsoft/fluentui/issues/25150
if (process.env.NODE_ENV === 'development') {
// eslint-disable-next-line no-console
console.warn(/** #__DE-INDENT__ */ `
Expand Down
Loading