fix(player): clean up controls on destroy (#1407)

Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>

Author: miguel-heygen

packages/player/src/controls.test.ts

@@ -0,0 +1,78 @@
+import { describe, it, expect, vi } from "vitest";
+import { type ControlsCallbacks, createControls } from "./controls";
+
+function noopCallbacks(): ControlsCallbacks {
+  return {
+    onPlay: () => {},
+    onPause: () => {},
+    onSeek: () => {},
+    onSpeedChange: () => {},
+    onMuteToggle: () => {},
+    onVolumeChange: () => {},
+  };
+}
+
+describe("createControls host listeners", () => {
+  it("removes every host listener it added on destroy", () => {
+    const host = document.createElement("div");
+    document.body.appendChild(host);
+
+    const addSpy = vi.spyOn(host, "addEventListener");
+    const removeSpy = vi.spyOn(host, "removeEventListener");
+
+    const api = createControls(host, noopCallbacks());
+
+    // Capture the exact handler references registered on the host element.
+    const added = new Map<string, EventListenerOrEventListenerObject>();
+    for (const [type, handler] of addSpy.mock.calls) {
+      added.set(type, handler as EventListenerOrEventListenerObject);
+    }
+    expect(added.has("mousemove")).toBe(true);
+    expect(added.has("mouseleave")).toBe(true);
+
+    api.destroy();
+
+    // Each host listener must be torn down with the same reference; anonymous
+    // handlers (the previous bug) could never be removed, so toggling the
+    // `controls` attribute leaked a duplicate pair on every cycle.
+    for (const [type, handler] of added) {
+      expect(removeSpy).toHaveBeenCalledWith(type, handler);
+    }
+
+    host.remove();
+  });
+
+  it("stops reacting to host mousemove after destroy", () => {
+    const host = document.createElement("div");
+    document.body.appendChild(host);
+
+    const api = createControls(host, noopCallbacks());
+    const controls = host.querySelector<HTMLElement>(".hfp-controls");
+    expect(controls).not.toBeNull();
+
+    api.destroy();
+
+    // A mousemove after destroy must not revive the controls overlay.
+    controls!.classList.add("hfp-hidden");
+    host.dispatchEvent(new Event("mousemove"));
+    expect(controls!.classList.contains("hfp-hidden")).toBe(true);
+
+    host.remove();
+  });
+
+  it("removes its controls element from the host on destroy", () => {
+    const host = document.createElement("div");
+    document.body.appendChild(host);
+
+    const api = createControls(host, noopCallbacks());
+    const controls = host.querySelector<HTMLElement>(".hfp-controls");
+    expect(controls).not.toBeNull();
+
+    api.destroy();
+
+    expect(host.querySelector(".hfp-controls")).toBeNull();
+    expect(controls!.isConnected).toBe(false);
+
+    host.remove();
+  });
+});

packages/player/src/controls.ts

@@ -330,13 +330,15 @@ export function createControls(
   };
 
   const host = parent instanceof ShadowRoot ? (parent.host as HTMLElement) : parent;
-  host.addEventListener("mousemove", () => {
+  const onHostMouseMove = () => {
     controls.classList.remove("hfp-hidden");
     startHideTimer();
-  });
-  host.addEventListener("mouseleave", () => {
+  };
+  const onHostMouseLeave = () => {
     if (isPlaying) controls.classList.add("hfp-hidden");
-  });
+  };
+  host.addEventListener("mousemove", onHostMouseMove);
+  host.addEventListener("mouseleave", onHostMouseLeave);
 
   return {
     updateTime(current: number, duration: number) {
@@ -389,7 +391,10 @@ export function createControls(
       document.removeEventListener("touchmove", onVolumeTouchMove);
       document.removeEventListener("touchend", onVolumeTouchEnd);
       document.removeEventListener("click", onDocClick);
+      host.removeEventListener("mousemove", onHostMouseMove);
+      host.removeEventListener("mouseleave", onHostMouseLeave);
       if (hideTimeout) clearTimeout(hideTimeout);
+      controls.remove();
     },
   };
 }

packages/player/src/hyperframes-player.test.ts

@@ -1467,6 +1467,19 @@ describe("HyperframesPlayer volume and mute", () => {
     expect(slider.getAttribute("tabindex")).toBe("0");
   });
 
+  it("removes and recreates one controls bar when the controls attribute toggles", () => {
+    document.body.appendChild(player);
+
+    player.setAttribute("controls", "");
+    expect(player.shadowRoot!.querySelectorAll(".hfp-controls")).toHaveLength(1);
+
+    player.removeAttribute("controls");
+    expect(player.shadowRoot!.querySelectorAll(".hfp-controls")).toHaveLength(0);
+
+    player.setAttribute("controls", "");
+    expect(player.shadowRoot!.querySelectorAll(".hfp-controls")).toHaveLength(1);
+  });
+
   it("dispatches volumechange when muted toggles (HTML5 spec)", () => {
     document.body.appendChild(player);