mui / mui/material-ui

FocusTrap cleanup calls nodeToRestore.current.focus() unconditionally since #48368, can throw when relatedTarget lacks a .focus method (e.g. under jsdom)

Open
#49,159 4 comments 0 reactions 1 assignee View on GitHub

@ZeeshanTamboli is already working on this.

Since Sep 18, 2026.

scope: focus trap status: waiting for maintainer
Dominant language
JavaScript
Stars
99.1k
Forks
32.5k
Avg merge
2d 17h
Merged PRs (30d)
106

Description

Before #48368 ("Remove IE 11-specific focus safety check from FocusTrap cleanup"), FocusTrap's cleanup effect guarded the focus-restore call like this:

if (nodeToRestore.current && (nodeToRestore.current as HTMLElement).focus) {
  nodeToRestore.current.focus();
}

That PR simplified it to:

if (!disableRestoreFocus && nodeToRestore.current) {
  nodeToRestore.current.focus();
}

reasoning that the .focus check was IE11-specific and unnecessary in real browsers. That's true for real browsers — every genuine Element has .focus(). But nodeToRestore.current is captured from event.relatedTarget in the trap's focus handler, and in test environments running on jsdom (via @testing-library/react + Vitest/Jest), we've observed event.relatedTarget occasionally resolve to something that is truthy but does not expose a working .focus() method, producing:

TypeError: nodeToRestore.current.focus is not a function

This reproduces consistently in our test suite when rendering components that use MUI Dialog/Modal (which wrap FocusTrap) and triggering an open → focus → close/unmount cycle, specifically after bumping jsdom from 30.0.1 to 30.1.0: jsdom v30.1.0 release notes.

We dug into jsdom's source and found its event-dispatch internals (EventTarget-impl.js) were substantially reworked between those versions — most notably in jsdom/jsdom@2deaa8c, which changed how relatedTarget/target get resolved and cached during dispatch, and added a new code path for path entries with no matching listener. We were not able to build a minimal, standalone jsdom-only repro of a malformed relatedTarget outside of this specific FocusTrap usage pattern, so we can't say for certain this is a jsdom spec-compliance bug versus something else jsdom-specific about this code path.

Versions:

  • @mui/material: 9.4.0
  • jsdom: fails on 30.1.0, works on 30.0.1 (pinned as workaround)
  • @testing-library/react, Vitest (jsdom test environment)

Ask: Given the removed guard existed specifically to handle "not all elements have a focus method," would MUI consider restoring a typeof nodeToRestore.current.focus === 'function' (or similar) check in the cleanup path — independent of IE11 — as defensive hardening against non-conforming/edge-case DOM environments like jsdom in tests? Happy to help narrow down a minimal repro if useful.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.