From 1e05d7fb54f9b3e972a82825bfdbd9eec35fb3fb Mon Sep 17 00:00:00 2001 From: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com> Date: Sat, 13 Jun 2026 01:40:12 -0300 Subject: [PATCH] fix(player): clean up controls on destroy --- packages/player/src/controls.test.ts | 78 +++++++++++++++++++ packages/player/src/controls.ts | 13 +++- .../player/src/hyperframes-player.test.ts | 13 ++++ 3 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 packages/player/src/controls.test.ts diff --git a/packages/player/src/controls.test.ts b/packages/player/src/controls.test.ts new file mode 100644 index 000000000..afa75a7c1 --- /dev/null +++ b/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(); + 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(".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(".hfp-controls"); + expect(controls).not.toBeNull(); + + api.destroy(); + + expect(host.querySelector(".hfp-controls")).toBeNull(); + expect(controls!.isConnected).toBe(false); + + host.remove(); + }); +}); diff --git a/packages/player/src/controls.ts b/packages/player/src/controls.ts index 7351cf0bd..9e2a410a4 100644 --- a/packages/player/src/controls.ts +++ b/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(); }, }; } diff --git a/packages/player/src/hyperframes-player.test.ts b/packages/player/src/hyperframes-player.test.ts index 043bb1007..4a0b1ef72 100644 --- a/packages/player/src/hyperframes-player.test.ts +++ b/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);