fix: account for clientX and clientY in outsidePress (#1089)

.changeset/tough-carrots-grin.md

@@ -0,0 +1,5 @@
+---
+"bits-ui": patch
+---
+
+fix: Dialog/Popover clientX/Y detection to prevent password managers and other injected elements from closing the dialog when pressed

packages/bits-ui/src/lib/bits/utilities/dismissible-layer/use-dismissable-layer.svelte.ts

@@ -18,6 +18,7 @@ import { debounce } from "$lib/internal/debounce.js";
 import { noop } from "$lib/internal/noop.js";
 import { getOwnerDocument, isOrContainsTarget } from "$lib/internal/elements.js";
 import { isElement } from "$lib/internal/is.js";
+import { isClickTrulyOutside } from "$lib/internal/dom.js";
 
 globalThis.bitsDismissableLayers ??= new Map<
 	DismissibleLayerState,
@@ -269,7 +270,9 @@ function isValidEvent(e: PointerEvent, node: HTMLElement): boolean {
 	if (!isElement(target)) return false;
 	const ownerDocument = getOwnerDocument(target);
 	const isValid =
-		ownerDocument.documentElement.contains(target) && !isOrContainsTarget(node, target);
+		ownerDocument.documentElement.contains(target) &&
+		!isOrContainsTarget(node, target) &&
+		isClickTrulyOutside(e, node);
 	return isValid;
 }
 

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

@@ -7,3 +7,17 @@ export function getFirstNonCommentChild(element: HTMLElement | null) {
 	}
 	return null;
 }
+
+/**
+ * Determines if the click event truly occurred outside the content node.
+ * This was added to handle password managers and other elements that may be injected
+ * into the DOM but visually appear inside the content.
+ */
+export function isClickTrulyOutside(event: PointerEvent, contentNode: HTMLElement): boolean {
+	const { clientX, clientY } = event;
+	const rect = contentNode.getBoundingClientRect();
+
+	return (
+		clientX < rect.left || clientX > rect.right || clientY < rect.top || clientY > rect.bottom
+	);
+}

packages/tests/src/tests/combobox/combobox.test.ts

@@ -1,6 +1,6 @@
 import { render, waitFor } from "@testing-library/svelte";
 import { axe } from "jest-axe";
-import { describe, it } from "vitest";
+import { describe, it, vi } from "vitest";
 import { type Component, tick } from "svelte";
 import { type AnyFn, getTestKbd, setupUserEvents, sleep } from "../utils.js";
 import ComboboxTest from "./combobox-test.svelte";
@@ -32,6 +32,8 @@ const testItems: Item[] = [
 	},
 ];
 
+const contentRect = { left: 100, right: 200, top: 100, bottom: 200 };
+
 function setupSingle(
 	props: Partial<ComboboxSingleTestProps | ComboboxForceMountTestProps> = {},
 	items: Item[] = testItems,
@@ -216,8 +218,10 @@ describe("combobox - single", () => {
 	it("should close on outside click", async () => {
 		const { user, getContent, outside } = await openSingle();
 		await sleep(100);
+		vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(
+			contentRect as DOMRect
+		);
 		await user.click(outside);
-		await sleep(100);
 		expect(getContent()).toBeNull();
 	});
 

packages/tests/src/tests/dialog/dialog.test.ts

@@ -6,7 +6,7 @@ import {
 	waitFor,
 } from "@testing-library/svelte/svelte5";
 import { axe } from "jest-axe";
-import { describe, it } from "vitest";
+import { describe, it, vi } from "vitest";
 import { tick } from "svelte";
 import { getTestKbd, setupUserEvents, sleep } from "../utils.js";
 import DialogTest, { type DialogTestProps } from "./dialog-test.svelte";
@@ -50,6 +50,8 @@ async function open(props: DialogTestProps = {}) {
 	return { getByTestId, queryByTestId, user };
 }
 
+const contentRect = { left: 100, right: 200, top: 100, bottom: 200 };
+
 describe("dialog", () => {
 	it("should have no accessibility violations", async () => {
 		const { container } = render(DialogTest);
@@ -99,6 +101,9 @@ describe("dialog", () => {
 		await sleep(100);
 
 		const overlay = getByTestId("overlay");
+		vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(
+			contentRect as DOMRect
+		);
 		await user.click(overlay);
 		await sleep(25);
 

packages/tests/src/tests/popover/popover.test.ts

@@ -1,7 +1,7 @@
 import { render, waitFor } from "@testing-library/svelte/svelte5";
 import { axe } from "jest-axe";
 import { describe, it, vi } from "vitest";
-import type { Component } from "svelte";
+import { type Component } from "svelte";
 import { getTestKbd, setupUserEvents } from "../utils.js";
 import PopoverTest, { type PopoverTestProps } from "./popover-test.svelte";
 import PopoverForceMountTest, {
@@ -73,16 +73,32 @@ describe("popover", () => {
 		expect(getContent()).toBeNull();
 	});
 
-	it("should close on outside click by default", async () => {
+	it("should close on outside click", async () => {
 		const mockFn = vi.fn();
-		const { user, getByTestId } = await open({
-			contentProps: {
-				onInteractOutside: mockFn,
-			},
+		const { getByTestId, user } = await open({
+			contentProps: { onInteractOutside: mockFn },
 		});
+
 		const outside = getByTestId("outside");
+
+		const contentRect = { left: 100, right: 200, top: 100, bottom: 200 };
+		vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(
+			contentRect as DOMRect
+		);
 		await user.click(outside);
-		expect(mockFn).toHaveBeenCalledTimes(1);
+
+		expect(mockFn).toHaveBeenCalledOnce();
+	});
+
+	it("should not close when clicking within bounds", async () => {
+		const mockFn = vi.fn();
+		const { user, content } = await open({
+			contentProps: { onInteractOutside: mockFn },
+		});
+
+		await user.click(content);
+
+		expect(mockFn).not.toHaveBeenCalled();
 	});
 
 	it("should close when the close button is clicked", async () => {

packages/tests/src/tests/select/select.test.ts

@@ -36,6 +36,8 @@ const testItems: Item[] = [
 	},
 ];
 
+const contentRect = { left: 100, right: 200, top: 100, bottom: 200 };
+
 function setupSingle(
 	props: Partial<SelectSingleTestProps | SelectForceMountTestProps> = {},
 	items: Item[] = testItems,
@@ -223,6 +225,11 @@ describe("select - single", () => {
 	it("should close on outside click", async () => {
 		const { user, getContent, outside } = await openSingle();
 		await sleep(100);
+
+		vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(
+			contentRect as DOMRect
+		);
+
 		await user.click(outside);
 		await sleep(100);
 		expect(getContent()).toBeNull();
@@ -623,8 +630,10 @@ describe("select - multiple", () => {
 	it("should close on outside click", async () => {
 		const { user, getContent, outside } = await openMultiple();
 		await sleep(100);
+		vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue(
+			contentRect as DOMRect
+		);
 		await user.click(outside);
-		await sleep(100);
 		expect(getContent()).toBeNull();
 	});