fix(studio): one Cmd+Z pressed inside the preview takes one undo step, not three (#4824)

* fix(studio): one Cmd+Z pressed inside the preview takes one undo step, not three

* fix(studio): one undo per key press, tracked, with the preview's extra history listener removed

packages/studio/src/hooks/appHotkeysDispatch.ts

@@ -14,13 +14,7 @@ import { STUDIO_PLAIN_KEYS } from "../player/components/studioShortcuts";
 // following useTimelineDeleteOps's precedent. Pure functions, no hooks — the
 // hook still owns the actual keydown listeners and calls into these.
 
-/** Exported so useAppHotkeys's own history-only preview listener can reuse
- *  the same undo/redo key arbitration without duplicating it. */
-export function handleUndoRedoKey(
-  event: KeyboardEvent,
-  onUndo: () => void,
-  onRedo: () => void,
-): boolean {
+function handleUndoRedoKey(event: KeyboardEvent, onUndo: () => void, onRedo: () => void): boolean {
   const key = event.key.toLowerCase();
   if (key === "z" && !event.shiftKey) {
     event.preventDefault();

packages/studio/src/hooks/useAppHotkeys.previewForwarding.test.tsx

@@ -7,10 +7,15 @@ import type { DomEditSelection } from "../components/editor/domEditing";
 import { usePlayerStore } from "../player/store/playerStore";
 import { useAppHotkeys } from "./useAppHotkeys";
 import { mountReactHarness } from "./domSelectionTestHarness";
+import { trackStudioEvent } from "../utils/studioTelemetry";
+
+vi.mock("../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
 
 (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
 
 const domDelete = vi.fn(async () => undefined);
+const historyUndo = vi.fn(async () => ({ ok: false }));
+const historyRedo = vi.fn(async () => ({ ok: false }));
 let root: Root | null = null;
 let sync: ((iframe: HTMLIFrameElement | null) => void) | null = null;
 
@@ -35,8 +40,8 @@ function Harness() {
     domEditSelectionRef: selectionRef,
     clearDomSelectionRef: useRef<() => void>(() => undefined),
     editHistory: {
-      undo: vi.fn(async () => ({ ok: false })),
-      redo: vi.fn(async () => ({ ok: false })),
+      undo: historyUndo,
+      redo: historyRedo,
       state: { undo: [], redo: [] },
     },
     readOptionalProjectFile: vi.fn(async () => ""),
@@ -62,6 +67,9 @@ afterEach(() => {
   document.body.innerHTML = "";
   usePlayerStore.getState().reset();
   domDelete.mockClear();
+  historyUndo.mockClear();
+  historyRedo.mockClear();
+  vi.mocked(trackStudioEvent).mockClear();
 });
 
 describe("preview iframe hotkey forwarding", () => {
@@ -87,4 +95,61 @@ describe("preview iframe hotkey forwarding", () => {
 
     expect(domDelete).toHaveBeenCalledTimes(1);
   });
+
+  function mountWithPreview(): Window & typeof globalThis {
+    root = mountReactHarness(<Harness />);
+    const iframe = document.createElement("iframe");
+    document.body.append(iframe);
+    act(() => sync?.(iframe));
+    const inner = iframe.contentWindow as (Window & typeof globalThis) | null;
+    if (!inner) throw new Error("expected an iframe window");
+    return inner;
+  }
+
+  async function press(inner: Window & typeof globalThis, init: KeyboardEventInit) {
+    await act(async () => {
+      inner.document.body.dispatchEvent(
+        new inner.KeyboardEvent("keydown", { ...init, bubbles: true }),
+      );
+    });
+  }
+
+  it("takes one tracked undo step per Cmd+Z pressed inside the preview", async () => {
+    await press(mountWithPreview(), { key: "z", ctrlKey: true });
+
+    expect(historyUndo).toHaveBeenCalledTimes(1);
+    expect(trackStudioEvent).toHaveBeenCalledTimes(1);
+    expect(trackStudioEvent).toHaveBeenCalledWith("keyboard_shortcut", { action: "undo" });
+  });
+
+  it.each([
+    ["Shift+Cmd+Z", { key: "Z", metaKey: true, shiftKey: true }],
+    ["Ctrl+Y", { key: "y", ctrlKey: true }],
+  ])("takes one tracked redo step per %s pressed inside the preview", async (_, init) => {
+    await press(mountWithPreview(), init);
+
+    expect(historyRedo).toHaveBeenCalledTimes(1);
+    expect(trackStudioEvent).toHaveBeenCalledTimes(1);
+    expect(trackStudioEvent).toHaveBeenCalledWith("keyboard_shortcut", { action: "redo" });
+  });
+
+  it("stops handling preview keys once the app unmounts", async () => {
+    root = mountReactHarness(<Harness />);
+    const iframe = document.createElement("iframe");
+    document.body.append(iframe);
+    const inner = iframe.contentWindow as (Window & typeof globalThis) | null;
+    if (!inner) throw new Error("expected an iframe window");
+    const add = vi.spyOn(inner, "addEventListener");
+    const remove = vi.spyOn(inner, "removeEventListener");
+    act(() => sync?.(iframe));
+    act(() => root?.unmount());
+    root = null;
+
+    await press(inner, { key: "z", ctrlKey: true });
+
+    expect(historyUndo).not.toHaveBeenCalled();
+    // happy-dom removes a listener whatever its capture flag; a browser does not, so pin the flag.
+    expect(add).toHaveBeenCalled();
+    expect(remove.mock.calls).toEqual(add.mock.calls);
+  });
 });

packages/studio/src/hooks/useAppHotkeys.ts

@@ -8,13 +8,7 @@ import {
   applyCaptionModelToIframe,
   isCaptionPreviewVisible,
 } from "../captions/components/CaptionOverlayUtils";
-import { shouldIgnoreHistoryShortcut } from "../utils/studioHelpers";
-import {
-  type HotkeyCallbacks,
-  dispatchModifierKey,
-  dispatchPlainKey,
-  handleUndoRedoKey,
-} from "./appHotkeysDispatch";
+import { type HotkeyCallbacks, dispatchModifierKey, dispatchPlainKey } from "./appHotkeysDispatch";
 import {
   useEditHistoryActions,
   type EditHistoryHandle,
@@ -36,9 +30,14 @@ function safeAddListener(t: EventTarget | null, type: string, h: EventListener,
     /* cross-origin */
   }
 }
-function safeRemoveListener(t: EventTarget | null, type: string, h: EventListener) {
+function safeRemoveListener(
+  t: EventTarget | null,
+  type: string,
+  h: EventListener,
+  capture = false,
+) {
   try {
-    t?.removeEventListener(type, h);
+    t?.removeEventListener(type, h, capture);
   } catch {
     /* cross-origin */
   }
@@ -232,52 +231,23 @@ export function useAppHotkeys({
 
   // ── Preview iframe forwarding ──
 
-  const handleHistoryHotkey = useCallback((event: KeyboardEvent) => {
-    if (!(event.metaKey || event.ctrlKey) || shouldIgnoreHistoryShortcut(event.target)) return;
-    handleUndoRedoKey(
-      event,
-      () => void cbRef.current.handleUndo(),
-      () => void cbRef.current.handleRedo(),
-    );
-  }, []);
-
   /**
-   * Give the preview iframe the app's hotkeys, because a keypress lands in
-   * whichever document has focus and clicking the canvas puts focus in there.
-   *
-   * Must run on every iframe LOAD, not once when the element mounts: a reload
-   * keeps the same element (so no ref callback) and the same WindowProxy (so an
-   * identity check sees no change) while replacing the inner window that holds
-   * the listeners. Attaching once left Delete dead in the canvas after the first
-   * reload — press it with a selection and nothing happened, no toast, nothing
-   * to explain it — while undo/redo kept working because they re-attached here.
+   * Give the preview iframe the app's hotkeys: clicking the canvas puts focus in there.
+   * Runs on every iframe LOAD: a reload keeps the element and WindowProxy but replaces the
+   * inner window holding the listeners, which once left Delete dead after the first reload.
    */
   const syncPreviewHotkeys = useCallback(
     (iframe: HTMLIFrameElement | null) => {
       previewHistoryCleanupRef.current?.();
       previewHistoryCleanupRef.current = null;
       const win = iframeContentWindow(iframe);
-      let doc: Document | null = null;
-      try {
-        doc = iframe?.contentDocument ?? null;
-      } catch {
-        doc = null;
-      }
-      if (!win && !doc) return;
-      const handler = handleHistoryHotkey as EventListener;
+      if (!win) return;
       const appHandler = handleAppKeyDown as EventListener;
-      safeAddListener(win, "keydown", handler, true);
-      // Window only: the history pair also listens on the document, and a
-      // capture listener on both would run the app handler twice per press.
+      // Window only: a capture listener on the document too would run it twice per press.
       safeAddListener(win, "keydown", appHandler, true);
-      doc?.addEventListener("keydown", handleHistoryHotkey, true);
-      previewHistoryCleanupRef.current = () => {
-        safeRemoveListener(win, "keydown", handler);
-        safeRemoveListener(win, "keydown", appHandler);
-        doc?.removeEventListener("keydown", handleHistoryHotkey, true);
-      };
+      previewHistoryCleanupRef.current = () => safeRemoveListener(win, "keydown", appHandler, true);
     },
-    [handleAppKeyDown, handleHistoryHotkey],
+    [handleAppKeyDown],
   );
 
   useEffect(