Fix document event listener leak in onDriverClick (#452)

src/events.ts

@@ -64,6 +64,10 @@ function onKeyup(e: KeyboardEvent) {
   }
 }
 
+const DRIVER_CLICK_EVENTS = ["pointerdown", "mousedown", "pointerup", "mouseup", "click"] as const;
+
+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 +83,9 @@ export function onDriverClick(
   listener: (pointer: MouseEvent | PointerEvent) => void,
   shouldPreventDefault?: (target: HTMLElement) => boolean
 ) {
-  const listenerWrapper = (e: MouseEvent | PointerEvent, listener?: (pointer: MouseEvent | PointerEvent) => void) => {
+  destroyDriverClick(element);
+
+  const handler = (e: MouseEvent | PointerEvent) => {
     const target = e.target as HTMLElement;
     if (!element.contains(target)) {
       return;
@@ -91,26 +97,32 @@ export function onDriverClick(
       e.stopImmediatePropagation();
     }
 
-    listener?.(e);
+    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);
+  for (const type of DRIVER_CLICK_EVENTS) {
+    document.addEventListener(type, handler, useCapture);
+  }
 
-  // Actual click handler
-  document.addEventListener(
-    "click",
-    e => {
-      listenerWrapper(e, listener);
-    },
-    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 +134,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,154 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { createDriver, navButton, nextFrame, SAMPLE_STEPS, useDriverHarness } from "./utils";
+
+useDriverHarness();
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+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;
+}
+
+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);
+  });
+});