fix(desktop): prevent context menu listener leaks
This commit is contained in:
1 parent
285196894e
commit
a0410c5834
4 files changed
+267
-22
No files matched your search
@@ -1,7 +1,8 @@
|
||||
<script setup lang="ts">
|
||||
import { ref, watch, onBeforeUnmount, nextTick, type Component } from "vue";
|
||||
import { ref, watch, onBeforeUnmount, onMounted, nextTick, type Component } from "vue";
|
||||
import { ChevronRight } from "@lucide/vue";
|
||||
import { shortcutDisplayKeys } from "@/lib/editor/shortcutDisplay";
|
||||
import { registerGlobalContextMenu, type ContextMenuRegistration } from "@/components/ui/customContextMenuRegistry";
|
||||
|
||||
export interface ContextMenuItem {
|
||||
label: string;
|
||||
@@ -31,24 +32,6 @@ defineSlots<{
|
||||
default(props: { onContextMenu: (event: MouseEvent) => void }): any;
|
||||
}>();
|
||||
|
||||
// ---- module-level singleton state ----
|
||||
const openMenus = new Set<() => void>();
|
||||
let globalSetup = false;
|
||||
|
||||
function ensureGlobalListeners() {
|
||||
if (globalSetup) return;
|
||||
globalSetup = true;
|
||||
const closeAll = () => {
|
||||
for (const c of openMenus) c();
|
||||
openMenus.clear();
|
||||
};
|
||||
document.addEventListener("contextmenu", closeAll, true);
|
||||
document.addEventListener("scroll", closeAll, true);
|
||||
window.addEventListener("resize", closeAll);
|
||||
}
|
||||
ensureGlobalListeners();
|
||||
// -------------------------------------
|
||||
|
||||
const show = ref(false);
|
||||
const x = ref(0);
|
||||
const y = ref(0);
|
||||
@@ -62,6 +45,7 @@ const subX = ref(0);
|
||||
const subY = ref(0);
|
||||
let subCloseTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let subAnchorRect: { left: number; right: number; top: number; bottom: number } | null = null;
|
||||
let contextMenuRegistration: ContextMenuRegistration | null = null;
|
||||
|
||||
function close() {
|
||||
activeSubIndex.value = null;
|
||||
@@ -99,14 +83,13 @@ function onResize() {
|
||||
}
|
||||
|
||||
watch(show, (val) => {
|
||||
contextMenuRegistration?.setOpen(val);
|
||||
if (val) {
|
||||
openMenus.add(close);
|
||||
document.addEventListener("pointerdown", onPointerDownOutside, true);
|
||||
document.addEventListener("keydown", onKeydown);
|
||||
document.addEventListener("scroll", onScroll, true);
|
||||
window.addEventListener("resize", onResize);
|
||||
} else {
|
||||
openMenus.delete(close);
|
||||
document.removeEventListener("pointerdown", onPointerDownOutside, true);
|
||||
document.removeEventListener("keydown", onKeydown);
|
||||
document.removeEventListener("scroll", onScroll, true);
|
||||
@@ -114,6 +97,10 @@ watch(show, (val) => {
|
||||
}
|
||||
});
|
||||
|
||||
onMounted(() => {
|
||||
contextMenuRegistration = registerGlobalContextMenu(close);
|
||||
});
|
||||
|
||||
function handleItemClick(item: ContextMenuItem) {
|
||||
if (item.disabled) return;
|
||||
if (item.children?.length) return; // submenu trigger — do nothing on click
|
||||
@@ -258,7 +245,8 @@ function shortcutKeys(shortcut?: string): string[] {
|
||||
}
|
||||
|
||||
onBeforeUnmount(() => {
|
||||
openMenus.delete(close);
|
||||
contextMenuRegistration?.dispose();
|
||||
contextMenuRegistration = null;
|
||||
document.removeEventListener("pointerdown", onPointerDownOutside, true);
|
||||
document.removeEventListener("keydown", onKeydown);
|
||||
document.removeEventListener("scroll", onScroll, true);
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { createApp, defineComponent, h, nextTick } from "vue";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import CustomContextMenu from "@/components/ui/CustomContextMenu.vue";
|
||||
|
||||
function callsFor(spy: ReturnType<typeof vi.spyOn>, eventName: string) {
|
||||
return spy.mock.calls.filter(([name]) => name === eventName);
|
||||
}
|
||||
|
||||
function removalsForListener(spy: ReturnType<typeof vi.spyOn>, eventName: string, listener: unknown) {
|
||||
return spy.mock.calls.filter(([name, candidate]) => name === eventName && candidate === listener);
|
||||
}
|
||||
|
||||
const mountedContainers: HTMLElement[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const container of mountedContainers.splice(0)) container.remove();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("CustomContextMenu lifecycle", () => {
|
||||
it("uses one shared listener set across repeated bulk mount cycles", async () => {
|
||||
const documentAdd = vi.spyOn(document, "addEventListener");
|
||||
const documentRemove = vi.spyOn(document, "removeEventListener");
|
||||
const windowAdd = vi.spyOn(window, "addEventListener");
|
||||
const windowRemove = vi.spyOn(window, "removeEventListener");
|
||||
|
||||
for (let cycle = 1; cycle <= 3; cycle += 1) {
|
||||
const root = defineComponent({
|
||||
setup() {
|
||||
return () =>
|
||||
Array.from({ length: 200 }, (_, index) =>
|
||||
h(
|
||||
CustomContextMenu,
|
||||
{ items: [{ label: `Action ${index}` }] },
|
||||
{
|
||||
default: ({ onContextMenu }: { onContextMenu: (event: MouseEvent) => void }) => h("div", { onContextmenu: onContextMenu }, `Target ${index}`),
|
||||
},
|
||||
),
|
||||
);
|
||||
},
|
||||
});
|
||||
const container = document.createElement("div");
|
||||
mountedContainers.push(container);
|
||||
document.body.append(container);
|
||||
const app = createApp(root);
|
||||
|
||||
app.mount(container);
|
||||
await nextTick();
|
||||
expect(callsFor(documentAdd, "contextmenu")).toHaveLength(cycle);
|
||||
expect(callsFor(documentAdd, "scroll")).toHaveLength(cycle);
|
||||
expect(callsFor(windowAdd, "resize")).toHaveLength(cycle);
|
||||
const contextMenuListener = callsFor(documentAdd, "contextmenu")[cycle - 1][1];
|
||||
const scrollListener = callsFor(documentAdd, "scroll")[cycle - 1][1];
|
||||
const resizeListener = callsFor(windowAdd, "resize")[cycle - 1][1];
|
||||
const contextMenuRemovals = removalsForListener(documentRemove, "contextmenu", contextMenuListener).length;
|
||||
const scrollRemovals = removalsForListener(documentRemove, "scroll", scrollListener).length;
|
||||
const resizeRemovals = removalsForListener(windowRemove, "resize", resizeListener).length;
|
||||
|
||||
app.unmount();
|
||||
await nextTick();
|
||||
expect(removalsForListener(documentRemove, "contextmenu", contextMenuListener)).toHaveLength(contextMenuRemovals + 1);
|
||||
expect(removalsForListener(documentRemove, "scroll", scrollListener)).toHaveLength(scrollRemovals + 1);
|
||||
expect(removalsForListener(windowRemove, "resize", resizeListener)).toHaveLength(resizeRemovals + 1);
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves open and global close behavior", async () => {
|
||||
const root = defineComponent({
|
||||
setup() {
|
||||
return () =>
|
||||
h(
|
||||
CustomContextMenu,
|
||||
{ items: [{ label: "Inspect" }] },
|
||||
{
|
||||
default: ({ onContextMenu }: { onContextMenu: (event: MouseEvent) => void }) => h("div", { id: "context-target", onContextmenu: onContextMenu }, "Target"),
|
||||
},
|
||||
);
|
||||
},
|
||||
});
|
||||
const container = document.createElement("div");
|
||||
mountedContainers.push(container);
|
||||
document.body.append(container);
|
||||
const app = createApp(root);
|
||||
app.mount(container);
|
||||
|
||||
const target = container.querySelector("#context-target");
|
||||
target?.dispatchEvent(new MouseEvent("contextmenu", { bubbles: true, clientX: 10, clientY: 20 }));
|
||||
await nextTick();
|
||||
expect(document.body.textContent).toContain("Inspect");
|
||||
|
||||
window.dispatchEvent(new Event("resize"));
|
||||
await nextTick();
|
||||
expect(document.body.textContent).not.toContain("Inspect");
|
||||
|
||||
app.unmount();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,89 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createContextMenuRegistry } from "@/components/ui/customContextMenuRegistry";
|
||||
|
||||
function callsFor(spy: ReturnType<typeof vi.spyOn>, eventName: string) {
|
||||
return spy.mock.calls.filter(([name]) => name === eventName);
|
||||
}
|
||||
|
||||
describe("customContextMenuRegistry", () => {
|
||||
it("shares one global listener set across all registered hosts", () => {
|
||||
const documentTarget = new EventTarget();
|
||||
const windowTarget = new EventTarget();
|
||||
const documentAdd = vi.spyOn(documentTarget, "addEventListener");
|
||||
const windowAdd = vi.spyOn(windowTarget, "addEventListener");
|
||||
const registry = createContextMenuRegistry(documentTarget, windowTarget);
|
||||
const closeFirst = vi.fn();
|
||||
const closeSecond = vi.fn();
|
||||
const registrations = Array.from({ length: 200 }, (_, index) => registry.register(index === 0 ? closeFirst : index === 1 ? closeSecond : vi.fn()));
|
||||
|
||||
expect(callsFor(documentAdd, "contextmenu")).toHaveLength(1);
|
||||
expect(callsFor(documentAdd, "scroll")).toHaveLength(1);
|
||||
expect(callsFor(windowAdd, "resize")).toHaveLength(1);
|
||||
|
||||
registrations[0].setOpen(true);
|
||||
registrations[1].setOpen(true);
|
||||
documentTarget.dispatchEvent(new Event("contextmenu"));
|
||||
expect(closeFirst).toHaveBeenCalledOnce();
|
||||
expect(closeSecond).toHaveBeenCalledOnce();
|
||||
|
||||
documentTarget.dispatchEvent(new Event("scroll"));
|
||||
windowTarget.dispatchEvent(new Event("resize"));
|
||||
expect(closeFirst).toHaveBeenCalledOnce();
|
||||
expect(closeSecond).toHaveBeenCalledOnce();
|
||||
|
||||
registrations.forEach((registration) => registration.dispose());
|
||||
});
|
||||
|
||||
it("removes disposed callbacks and detaches listeners after the final host", () => {
|
||||
const documentTarget = new EventTarget();
|
||||
const windowTarget = new EventTarget();
|
||||
const documentRemove = vi.spyOn(documentTarget, "removeEventListener");
|
||||
const windowRemove = vi.spyOn(windowTarget, "removeEventListener");
|
||||
const registry = createContextMenuRegistry(documentTarget, windowTarget);
|
||||
const closeDisposed = vi.fn();
|
||||
const closeRemaining = vi.fn();
|
||||
const disposed = registry.register(closeDisposed);
|
||||
const remaining = registry.register(closeRemaining);
|
||||
|
||||
disposed.setOpen(true);
|
||||
remaining.setOpen(true);
|
||||
disposed.dispose();
|
||||
disposed.dispose();
|
||||
documentTarget.dispatchEvent(new Event("scroll"));
|
||||
|
||||
expect(closeDisposed).not.toHaveBeenCalled();
|
||||
expect(closeRemaining).toHaveBeenCalledOnce();
|
||||
expect(callsFor(documentRemove, "contextmenu")).toHaveLength(0);
|
||||
|
||||
remaining.dispose();
|
||||
expect(callsFor(documentRemove, "contextmenu")).toHaveLength(1);
|
||||
expect(callsFor(documentRemove, "scroll")).toHaveLength(1);
|
||||
expect(callsFor(windowRemove, "resize")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("reattaches a clean listener set after complete teardown", () => {
|
||||
const documentTarget = new EventTarget();
|
||||
const windowTarget = new EventTarget();
|
||||
const documentAdd = vi.spyOn(documentTarget, "addEventListener");
|
||||
const windowAdd = vi.spyOn(windowTarget, "addEventListener");
|
||||
const registry = createContextMenuRegistry(documentTarget, windowTarget);
|
||||
const staleClose = vi.fn();
|
||||
const first = registry.register(staleClose);
|
||||
|
||||
first.setOpen(true);
|
||||
first.dispose();
|
||||
|
||||
const activeClose = vi.fn();
|
||||
const second = registry.register(activeClose);
|
||||
second.setOpen(true);
|
||||
windowTarget.dispatchEvent(new Event("resize"));
|
||||
|
||||
expect(staleClose).not.toHaveBeenCalled();
|
||||
expect(activeClose).toHaveBeenCalledOnce();
|
||||
expect(callsFor(documentAdd, "contextmenu")).toHaveLength(2);
|
||||
expect(callsFor(documentAdd, "scroll")).toHaveLength(2);
|
||||
expect(callsFor(windowAdd, "resize")).toHaveLength(2);
|
||||
|
||||
second.dispose();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,69 @@
|
||||
export type ContextMenuClose = () => void;
|
||||
|
||||
export interface ContextMenuRegistration {
|
||||
setOpen(open: boolean): void;
|
||||
dispose(): void;
|
||||
}
|
||||
|
||||
export interface ContextMenuRegistry {
|
||||
register(close: ContextMenuClose): ContextMenuRegistration;
|
||||
}
|
||||
|
||||
export function createContextMenuRegistry(documentTarget: EventTarget, windowTarget: EventTarget): ContextMenuRegistry {
|
||||
const openMenus = new Set<ContextMenuClose>();
|
||||
let hostCount = 0;
|
||||
let listenersAttached = false;
|
||||
|
||||
function closeAll() {
|
||||
const closers = [...openMenus];
|
||||
openMenus.clear();
|
||||
for (const close of closers) close();
|
||||
}
|
||||
|
||||
function attachListeners() {
|
||||
if (listenersAttached) return;
|
||||
documentTarget.addEventListener("contextmenu", closeAll, true);
|
||||
documentTarget.addEventListener("scroll", closeAll, true);
|
||||
windowTarget.addEventListener("resize", closeAll);
|
||||
listenersAttached = true;
|
||||
}
|
||||
|
||||
function detachListeners() {
|
||||
if (!listenersAttached) return;
|
||||
documentTarget.removeEventListener("contextmenu", closeAll, true);
|
||||
documentTarget.removeEventListener("scroll", closeAll, true);
|
||||
windowTarget.removeEventListener("resize", closeAll);
|
||||
listenersAttached = false;
|
||||
openMenus.clear();
|
||||
}
|
||||
|
||||
return {
|
||||
register(close) {
|
||||
hostCount += 1;
|
||||
attachListeners();
|
||||
let disposed = false;
|
||||
|
||||
return {
|
||||
setOpen(open) {
|
||||
if (disposed) return;
|
||||
if (open) openMenus.add(close);
|
||||
else openMenus.delete(close);
|
||||
},
|
||||
dispose() {
|
||||
if (disposed) return;
|
||||
disposed = true;
|
||||
openMenus.delete(close);
|
||||
hostCount -= 1;
|
||||
if (hostCount === 0) detachListeners();
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
let globalContextMenuRegistry: ContextMenuRegistry | undefined;
|
||||
|
||||
export function registerGlobalContextMenu(close: ContextMenuClose): ContextMenuRegistration {
|
||||
globalContextMenuRegistry ??= createContextMenuRegistry(document, window);
|
||||
return globalContextMenuRegistry.register(close);
|
||||
}
|
||||
Reference in new issue
Block a user