Fix document event listener leak in onDriverClick (#452)

onDriverClick attached five capture-phase listeners to document and
never removed them. Since renderPopover rebuilds the popover every step,
the listeners accumulated on document for the lifetime of the page.

Track each element's handler in a WeakMap and remove it on teardown
(destroyOverlay, destroyPopover, and before re-rendering the popover).
Also remove the keydown focus-trap listener in destroyEvents, which was
added in initEvents but never cleaned up.

Author: nilbuild

src/events.ts

@@ -64,6 +64,17 @@ function onKeyup(e: KeyboardEvent) {
   }
 }
 
+// The pointer events we intercept to make sure no external library ever hears
+// about a click before driver.js does. `click` carries the actual handler; the
+// rest are only suppressed.
+const DRIVER_CLICK_EVENTS = ["pointerdown", "mousedown", "pointerup", "mouseup", "click"] as const;
+
+// Associates each driver element with the single document-level handler that
+// was registered on its behalf, so it can be removed when the element is torn
+// down. A WeakMap keyed by the element avoids leaking handlers onto `document`
+// (the popover is rebuilt every step) and needs no id bookkeeping on the DOM.
+const driverClickHandlers = new WeakMap<Element, (e: MouseEvent | PointerEvent) => void>();
+
 /**
  * Attaches click handler to the elements created by driver.js. It makes
  * sure to give the listener the first chance to handle the event, and
@@ -79,7 +90,11 @@ export function onDriverClick(
   listener: (pointer: MouseEvent | PointerEvent) => void,
   shouldPreventDefault?: (target: HTMLElement) => boolean
 ) {
-  const listenerWrapper = (e: MouseEvent | PointerEvent, listener?: (pointer: MouseEvent | PointerEvent) => void) => {
+  // Defensive: if this element somehow already has a handler attached, remove
+  // it first so we never register duplicates.
+  destroyDriverClick(element);
+
+  const handler = (e: MouseEvent | PointerEvent) => {
     const target = e.target as HTMLElement;
     if (!element.contains(target)) {
       return;
@@ -91,26 +106,34 @@ export function onDriverClick(
       e.stopImmediatePropagation();
     }
 
-    listener?.(e);
+    // Only the actual click should invoke the user's listener; the other
+    // events exist purely to suppress interaction beneath the overlay/popover.
+    if (e.type === "click") {
+      listener?.(e);
+    }
   };
 
   // We want to be the absolute first one to hear about the event
   const useCapture = true;
 
-  // Events to disable
-  document.addEventListener("pointerdown", listenerWrapper, useCapture);
-  document.addEventListener("mousedown", listenerWrapper, useCapture);
-  document.addEventListener("pointerup", listenerWrapper, useCapture);
-  document.addEventListener("mouseup", listenerWrapper, useCapture);
-
-  // Actual click handler
-  document.addEventListener(
-    "click",
-    e => {
-      listenerWrapper(e, listener);
-    },
-    useCapture
-  );
+  for (const type of DRIVER_CLICK_EVENTS) {
+    document.addEventListener(type, handler, useCapture);
+  }
+
+  driverClickHandlers.set(element, handler);
+}
+
+export function destroyDriverClick(element: Element) {
+  const handler = driverClickHandlers.get(element);
+  if (!handler) {
+    return;
+  }
+
+  for (const type of DRIVER_CLICK_EVENTS) {
+    document.removeEventListener(type, handler, true);
+  }
+
+  driverClickHandlers.delete(element);
 }
 
 export function initEvents() {
@@ -122,6 +145,7 @@ export function initEvents() {
 
 export function destroyEvents() {
   window.removeEventListener("keyup", onKeyup);
+  window.removeEventListener("keydown", trapFocus);
   window.removeEventListener("resize", requireRefresh);
   window.removeEventListener("scroll", requireRefresh);
 }

src/overlay.ts

@@ -1,5 +1,5 @@
 import { easeInOutQuad } from "./utils";
-import { onDriverClick } from "./events";
+import { destroyDriverClick, onDriverClick } from "./events";
 import { emit } from "./emitter";
 import { getConfig } from "./config";
 import { getState, setState } from "./state";
@@ -173,6 +173,7 @@ function generateStageSvgPathString(stage: StageDefinition) {
 export function destroyOverlay() {
   const overlaySvg = getState("__overlaySvg");
   if (overlaySvg) {
+    destroyDriverClick(overlaySvg);
     overlaySvg.remove();
   }
 }

src/popover.ts

@@ -1,7 +1,7 @@
 import { Config, DriverHook, getConfig, getCurrentDriver } from "./config";
 import { Driver, DriveStep } from "./driver";
 import { emit } from "./emitter";
-import { onDriverClick } from "./events";
+import { destroyDriverClick, onDriverClick } from "./events";
 import { repositionPopover } from "./position";
 import { getState, setState, State } from "./state";
 import { bringInView, getFocusableElements } from "./utils";
@@ -63,6 +63,7 @@ export function hidePopover() {
 export function renderPopover(element: Element, step: DriveStep) {
   let popover = getState("popover");
   if (popover) {
+    destroyDriverClick(popover.wrapper);
     document.body.removeChild(popover.wrapper);
   }
 
@@ -331,5 +332,6 @@ export function destroyPopover() {
     return;
   }
 
+  destroyDriverClick(popover.wrapper);
   popover.wrapper.parentElement?.removeChild(popover.wrapper);
 }

tests/events.test.ts

@@ -0,0 +1,158 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { createDriver, navButton, nextFrame, SAMPLE_STEPS, useDriverHarness } from "./utils";
+
+useDriverHarness();
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+// The document-level listeners driver.js attaches to intercept clicks.
+const DRIVER_EVENT_TYPES = ["click", "pointerdown", "mousedown", "pointerup", "mouseup"];
+
+type ListenerRecord = {
+  type: string;
+  listener: EventListenerOrEventListenerObject;
+  capture: boolean;
+};
+
+function normalizeCapture(options?: boolean | AddEventListenerOptions | EventListenerOptions): boolean {
+  if (typeof options === "boolean") {
+    return options;
+  }
+
+  return !!options?.capture;
+}
+
+// Wraps document.add/removeEventListener so we can assert that every driver
+// listener that gets attached is also detached, i.e. nothing leaks onto the
+// document across steps and teardown.
+function trackDocumentListeners() {
+  const added: ListenerRecord[] = [];
+  const removed: ListenerRecord[] = [];
+
+  const originalAdd = document.addEventListener.bind(document);
+  const originalRemove = document.removeEventListener.bind(document);
+
+  vi.spyOn(document, "addEventListener").mockImplementation((type, listener, options) => {
+    if (DRIVER_EVENT_TYPES.includes(type)) {
+      added.push({
+        type,
+        listener: listener as EventListenerOrEventListenerObject,
+        capture: normalizeCapture(options),
+      });
+    }
+
+    return originalAdd(type, listener as EventListener, options);
+  });
+
+  vi.spyOn(document, "removeEventListener").mockImplementation((type, listener, options) => {
+    if (DRIVER_EVENT_TYPES.includes(type)) {
+      removed.push({
+        type,
+        listener: listener as EventListenerOrEventListenerObject,
+        capture: normalizeCapture(options),
+      });
+    }
+
+    return originalRemove(type, listener as EventListener, options);
+  });
+
+  function liveCount(): number {
+    const outstanding = [...removed];
+    let live = 0;
+
+    for (const record of added) {
+      const idx = outstanding.findIndex(
+        candidate =>
+          candidate.type === record.type &&
+          candidate.listener === record.listener &&
+          candidate.capture === record.capture
+      );
+
+      if (idx === -1) {
+        live++;
+      } else {
+        outstanding.splice(idx, 1);
+      }
+    }
+
+    return live;
+  }
+
+  return { liveCount };
+}
+
+describe("document listener cleanup", () => {
+  it("attaches document click listeners while the tour is active", () => {
+    const tracker = trackDocumentListeners();
+    const d = createDriver({ animate: false, steps: SAMPLE_STEPS });
+    d.drive();
+
+    expect(tracker.liveCount()).toBeGreaterThan(0);
+  });
+
+  it("removes every document click listener once the tour is destroyed", () => {
+    const tracker = trackDocumentListeners();
+    const d = createDriver({ animate: false, steps: SAMPLE_STEPS });
+    d.drive();
+    d.destroy();
+
+    expect(tracker.liveCount()).toBe(0);
+  });
+
+  it("does not accumulate document listeners as the tour moves between steps", () => {
+    const tracker = trackDocumentListeners();
+    const d = createDriver({ animate: false, steps: SAMPLE_STEPS });
+    d.drive();
+
+    const afterFirstStep = tracker.liveCount();
+
+    navButton("next")?.click();
+    navButton("next")?.click();
+
+    expect(tracker.liveCount()).toBe(afterFirstStep);
+  });
+
+  it("removes the keydown focus-trap listener when destroyed", () => {
+    const removeSpy = vi.spyOn(window, "removeEventListener");
+    const d = createDriver({ animate: false, steps: SAMPLE_STEPS });
+    d.drive();
+    d.destroy();
+
+    expect(removeSpy.mock.calls.some(([type]) => type === "keydown")).toBe(true);
+  });
+});
+
+describe("overlay pointer suppression", () => {
+  it("prevents default on pointer events over the overlay to block page interaction", async () => {
+    const d = createDriver({ animate: false, steps: SAMPLE_STEPS });
+    d.drive();
+    await nextFrame();
+
+    const path = document.querySelector(".driver-overlay path");
+    expect(path).not.toBeNull();
+
+    for (const type of ["pointerdown", "mousedown", "pointerup", "mouseup"]) {
+      const event = new MouseEvent(type, { bubbles: true, cancelable: true });
+      path!.dispatchEvent(event);
+      expect(event.defaultPrevented).toBe(true);
+    }
+  });
+
+  it("still routes overlay clicks to the click handler", async () => {
+    const onNextClick = vi.fn();
+    const d = createDriver({
+      animate: false,
+      overlayClickBehavior: "nextStep",
+      steps: SAMPLE_STEPS,
+      onNextClick,
+    });
+    d.drive();
+    await nextFrame();
+
+    document.querySelector(".driver-overlay path")?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+
+    expect(onNextClick).toHaveBeenCalledTimes(1);
+  });
+});