fix: nested focus scope infinite loops (#1132)

.changeset/moody-bugs-rescue.md

@@ -0,0 +1,5 @@
+---
+"bits-ui": patch
+---
+
+fix: infinite loops caused by rapid open/close of nested focus scopes

packages/bits-ui/src/lib/bits/utilities/focus-scope/focus-scope-stack.svelte.ts

@@ -1,45 +1,51 @@
 import { box } from "svelte-toolbelt";
 import { useId } from "$lib/internal/use-id.js";
 
-export type FocusScopeAPI = {
+export interface FocusScopeAPI {
 	id: string;
 	paused: boolean;
 	pause: () => void;
 	resume: () => void;
-};
-
+	isHandlingFocus: boolean;
+}
 const focusStack = box<FocusScopeAPI[]>([]);
 
 export function createFocusScopeStack() {
-	const stack = focusStack;
-
 	return {
 		add(focusScope: FocusScopeAPI) {
-			// pause the currently active focus scope (top of the stack)
-			const activeFocusScope = stack.current[0];
-			if (focusScope.id !== activeFocusScope?.id) {
-				activeFocusScope?.pause();
+			const activeFocusScope = focusStack.current[0];
+			if (activeFocusScope && focusScope.id !== activeFocusScope.id) {
+				activeFocusScope.pause();
 			}
 
-			// remove in case it already exists because it'll be added to the top
-			stack.current = removeFromFocusScopeArray(stack.current, focusScope);
-			stack.current.unshift(focusScope);
+			focusStack.current = removeFromFocusScopeArray(focusStack.current, focusScope);
+			focusStack.current.unshift(focusScope);
 		},
 		remove(focusScope: FocusScopeAPI) {
-			stack.current = removeFromFocusScopeArray(stack.current, focusScope);
-			stack.current[0]?.resume();
+			focusStack.current = removeFromFocusScopeArray(focusStack.current, focusScope);
+			focusStack.current[0]?.resume();
+		},
+		get current() {
+			return focusStack.current;
 		},
 	};
 }
 
 export function createFocusScopeAPI(): FocusScopeAPI {
 	let paused = $state(false);
+	let isHandlingFocus = $state(false);
 
 	return {
 		id: useId(),
 		get paused() {
 			return paused;
 		},
+		get isHandlingFocus() {
+			return isHandlingFocus;
+		},
+		set isHandlingFocus(value: boolean) {
+			isHandlingFocus = value;
+		},
 		pause() {
 			paused = true;
 		},

packages/bits-ui/src/lib/bits/utilities/focus-scope/use-focus-scope.svelte.ts

@@ -83,71 +83,60 @@ export function useFocusScope({
 	const focusScope = createFocusScopeAPI();
 	const ref = box<HTMLElement | null>(null);
 	const ctx = FocusScopeContext.getOr({ ignoreCloseAutoFocus: false });
+	let lastFocusedElement: HTMLElement | null = null;
 
 	useRefById({
 		id,
 		ref,
 		deps: () => enabled.current,
 	});
 
-	let lastFocusedElement: HTMLElement | null = null;
+	function manageFocus(event: FocusEvent) {
+		if (focusScope.paused || !ref.current || focusScope.isHandlingFocus) return;
+		focusScope.isHandlingFocus = true;
 
-	watch([() => ref.current, () => enabled.current], ([container, enabled]) => {
-		if (!container || !enabled) return;
-		const handleFocusIn = (event: FocusEvent) => {
-			if (focusScope.paused || !container) return;
+		try {
 			const target = event.target;
 			if (!isHTMLElement(target)) return;
-			if (container.contains(target)) {
-				lastFocusedElement = target;
-			} else {
-				if (ctx.ignoreCloseAutoFocus) return;
-				focus(lastFocusedElement, { select: true });
-			}
-		};
 
-		const handleFocusOut = (event: FocusEvent) => {
-			if (focusScope.paused || !container || ctx.ignoreCloseAutoFocus) {
-				return;
-			}
-			const relatedTarget = event.relatedTarget;
-			if (!isHTMLElement(relatedTarget)) return;
-			// A `focusout` event with a `null` `relatedTarget` will happen in at least two cases:
-			//
-			// 1. When the user switches app/tabs/windows/the browser itself loses focus.
-			// 2. In Google Chrome, when the focused element is removed from the DOM.
-			//
-			// We let the browser do its thing here because:
-			//
-			// 1. The browser already keeps a memory of what's focused for when the
-			// page gets refocused.
-			// 2. In Google Chrome, if we try to focus the deleted focused element it throws
-			// the CPU to 100%, so we avoid doing anything for this reason here too.
-			if (relatedTarget === null) return;
-
-			// If the focus has moved to an actual legitimate element (`relatedTarget !== null`)
-			// that is outside the container, we move focus to the last valid focused element inside.
-			if (!container.contains(relatedTarget)) {
-				focus(lastFocusedElement, { select: true });
+			const isWithinActiveScope = ref.current.contains(target);
+
+			if (event.type === "focusin") {
+				if (isWithinActiveScope) {
+					lastFocusedElement = target;
+				} else {
+					if (ctx.ignoreCloseAutoFocus) return;
+					focus(lastFocusedElement, { select: true });
+				}
+			} else if (event.type === "focusout") {
+				if (!isWithinActiveScope && !ctx.ignoreCloseAutoFocus) {
+					focus(lastFocusedElement, { select: true });
+				}
 			}
-		};
+		} finally {
+			focusScope.isHandlingFocus = false;
+		}
+	}
 
-		// When the focused element gets removed from the DOM, browsers move focus
-		// back to the document.body. In this case, we move focus to the container
-		// to keep focus trapped correctly.
-		// instead of leaning on document.activeElement, we use lastFocusedElement to check
-		// if the element still exists inside the container,
-		// if not then we focus to the container
-		const handleMutations = (_: MutationRecord[]) => {
-			const lastFocusedElementExists = container?.contains(lastFocusedElement);
-			if (!lastFocusedElementExists) {
-				focus(container);
-			}
-		};
+	// When the focused element gets removed from the DOM, browsers move focus
+	// back to the document.body. In this case, we move focus to the container
+	// to keep focus trapped correctly.
+	// instead of leaning on document.activeElement, we use lastFocusedElement to check
+	// if the element still exists inside the container,
+	// if not then we focus to the container
+	function handleMutations(_: MutationRecord[]) {
+		const lastFocusedElementExists = ref.current?.contains(lastFocusedElement);
+		if (!lastFocusedElementExists && ref.current) {
+			focus(ref.current);
+		}
+	}
+
+	watch([() => ref.current, () => enabled.current], ([container, enabled]) => {
+		if (!container || !enabled) return;
 
 		const removeEvents = executeCallbacks(
-			on(document, "focusin", handleFocusIn),
-			on(document, "focusout", handleFocusOut)
+			on(document, "focusin", manageFocus),
+			on(document, "focusout", manageFocus)
 		);
 		const mutationObserver = new MutationObserver(handleMutations);
 		mutationObserver.observe(container, { childList: true, subtree: true });

packages/bits-ui/src/lib/internal/focus.ts

@@ -35,6 +35,7 @@ export function focusWithoutScroll(element: HTMLElement) {
  */
 export function focus(element?: FocusableTarget | null, { select = false } = {}) {
 	if (!(element && element.focus)) return;
+	if (document.activeElement === element) return;
 	const previouslyFocusedElement = document.activeElement;
 	// prevent scroll on focus
 	element.focus({ preventScroll: true });

packages/tests/src/tests/context-menu/context-menu.test.ts

@@ -1,4 +1,4 @@
-import { cleanup, render, screen, waitFor } from "@testing-library/svelte/svelte5";
+import { render, screen, waitFor } from "@testing-library/svelte/svelte5";
 import { axe } from "jest-axe";
 import { describe, it } from "vitest";
 import { getTestKbd, setupUserEvents } from "../utils.js";
@@ -110,7 +110,6 @@ describe("context menu", () => {
 	});
 
 	it("should manage focus correctly when opened with pointer", async () => {
-		cleanup();
 		const { queryByTestId, user } = await open();
 		const item = queryByTestId("item");
 		expect(item).not.toHaveFocus();