fix: detach DOM widget event listeners on widget removal (#11724)

## Summary

Fixes leaked event listeners

## Changes

- **What**: 
- update all listeners to use AbortController to signal removal on
widget remove

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11724-fix-detach-DOM-widget-event-listeners-on-widget-removal-3506d73d3650811dae81c034c1098759)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <drjkl@comfy.org>

src/composables/node/useNodeAnimatedImage.test.ts

@@ -0,0 +1,70 @@
+import { describe, expect, it, onTestFinished, vi } from 'vitest'
+
+import { useNodeAnimatedImage } from '@/composables/node/useNodeAnimatedImage'
+import { createMockMediaNode } from '@/renderer/extensions/vueNodes/widgets/composables/domWidgetTestUtils'
+
+const { canvasInteractionsMock } = vi.hoisted(() => ({
+  canvasInteractionsMock: {
+    handleWheel: vi.fn(),
+    handlePointer: vi.fn(),
+    forwardEventToCanvas: vi.fn()
+  }
+}))
+
+vi.mock('@/renderer/core/canvas/useCanvasInteractions', () => ({
+  useCanvasInteractions: () => canvasInteractionsMock
+}))
+// `@/scripts/app` has a heavy import graph (pinia stores, LGraphCanvas, etc.)
+// that we cannot pull in here, so we stub only the constant we need.
+vi.mock('@/scripts/app', () => ({
+  ANIM_PREVIEW_WIDGET: '$$comfy_animation_preview'
+}))
+
+describe('useNodeAnimatedImage', () => {
+  function setup() {
+    vi.clearAllMocks()
+    const node = createMockMediaNode({ imgs: [document.createElement('img')] })
+    const { showAnimatedPreview, removeAnimatedPreview } =
+      useNodeAnimatedImage()
+    showAnimatedPreview(node)
+    const element = node.widgets[0].element
+    document.body.append(element)
+    onTestFinished(() => element.remove())
+    return { node, element, showAnimatedPreview, removeAnimatedPreview }
+  }
+
+  it('forwards non-right-click pointer events and wheel to the canvas while alive', () => {
+    const { element } = setup()
+    element.dispatchEvent(new WheelEvent('wheel'))
+    element.dispatchEvent(new PointerEvent('pointermove'))
+    element.dispatchEvent(new PointerEvent('pointerup'))
+    element.dispatchEvent(new PointerEvent('pointerdown', { button: 0 }))
+
+    expect(canvasInteractionsMock.handleWheel).toHaveBeenCalledTimes(1)
+    expect(canvasInteractionsMock.handlePointer).toHaveBeenCalledTimes(3)
+    expect(canvasInteractionsMock.forwardEventToCanvas).not.toHaveBeenCalled()
+  })
+
+  it('routes right-click pointerdown through forwardEventToCanvas, not handlePointer', () => {
+    const { element } = setup()
+    element.dispatchEvent(new PointerEvent('pointerdown', { button: 2 }))
+
+    expect(canvasInteractionsMock.forwardEventToCanvas).toHaveBeenCalledTimes(1)
+    expect(canvasInteractionsMock.handlePointer).not.toHaveBeenCalled()
+  })
+
+  it('detaches every listener when the preview is removed', () => {
+    const { node, element, removeAnimatedPreview } = setup()
+    removeAnimatedPreview(node)
+
+    element.dispatchEvent(new WheelEvent('wheel'))
+    element.dispatchEvent(new PointerEvent('pointermove'))
+    element.dispatchEvent(new PointerEvent('pointerup'))
+    element.dispatchEvent(new PointerEvent('pointerdown', { button: 0 }))
+    element.dispatchEvent(new PointerEvent('pointerdown', { button: 2 }))
+
+    expect(canvasInteractionsMock.handleWheel).not.toHaveBeenCalled()
+    expect(canvasInteractionsMock.handlePointer).not.toHaveBeenCalled()
+    expect(canvasInteractionsMock.forwardEventToCanvas).not.toHaveBeenCalled()
+  })
+})

src/composables/node/useNodeAnimatedImage.ts

@@ -1,3 +1,4 @@
+import { useChainCallback } from '@/composables/functional/useChainCallback'
 import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
 import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
 import { ANIM_PREVIEW_WIDGET } from '@/scripts/app'
@@ -39,17 +40,20 @@ export function useNodeAnimatedImage() {
       const { handleWheel, handlePointer, forwardEventToCanvas } =
         useCanvasInteractions()
       node.imgs[0].style.pointerEvents = 'none'
-      element.addEventListener('wheel', handleWheel)
-      element.addEventListener('pointermove', handlePointer)
-      element.addEventListener('pointerup', handlePointer)
+      const controller = new AbortController()
+      const { signal } = controller
+      element.addEventListener('wheel', handleWheel, { signal })
+      element.addEventListener('pointermove', handlePointer, { signal })
+      element.addEventListener('pointerup', handlePointer, { signal })
       element.addEventListener(
         'pointerdown',
-        (e) => {
-          return e.button !== 2 ? handlePointer(e) : forwardEventToCanvas(e)
-        },
-        true
+        (e) => (e.button !== 2 ? handlePointer(e) : forwardEventToCanvas(e)),
+        { capture: true, signal }
       )
 
+      widget.onRemove = useChainCallback(widget.onRemove, () => {
+        controller.abort()
+      })
       widget.serialize = false
       widget.serializeValue = () => undefined
     }

src/composables/node/useNodeImage.test.ts

@@ -0,0 +1,93 @@
+import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'
+
+import { useNodeVideo } from '@/composables/node/useNodeImage'
+import { createMockMediaNode } from '@/renderer/extensions/vueNodes/widgets/composables/domWidgetTestUtils'
+
+const { canvasInteractionsMock, nodeOutputStoreMock } = vi.hoisted(() => ({
+  canvasInteractionsMock: {
+    handleWheel: vi.fn(),
+    handlePointer: vi.fn()
+  },
+  nodeOutputStoreMock: {
+    getNodeImageUrls: vi.fn<(node: unknown) => string[] | undefined>()
+  }
+}))
+
+vi.mock('@/renderer/core/canvas/useCanvasInteractions', () => ({
+  useCanvasInteractions: () => canvasInteractionsMock
+}))
+vi.mock('@/stores/nodeOutputStore', () => ({
+  useNodeOutputStore: () => nodeOutputStoreMock
+}))
+vi.mock('@/utils/imageUtil', () => ({
+  fitDimensionsToNodeWidth: () => ({ minHeight: 256, minWidth: 256 })
+}))
+
+describe('useNodeVideo', () => {
+  afterEach(() => {
+    vi.useRealTimers()
+    vi.restoreAllMocks()
+  })
+
+  async function setup() {
+    vi.clearAllMocks()
+    vi.useFakeTimers()
+
+    nodeOutputStoreMock.getNodeImageUrls.mockReturnValue(['http://video/1.mp4'])
+    const node = createMockMediaNode({
+      size: [400, 400],
+      graph: { setDirtyCanvas: vi.fn() }
+    })
+
+    const createdVideos: HTMLVideoElement[] = []
+    const realCreateElement = document.createElement.bind(document)
+    vi.spyOn(document, 'createElement').mockImplementation(
+      (tag: string, opts?: ElementCreationOptions) => {
+        const el = realCreateElement(tag, opts)
+        if (tag === 'video') createdVideos.push(el as HTMLVideoElement)
+        return el
+      }
+    )
+
+    const { showPreview } = useNodeVideo(node)
+    showPreview()
+
+    // happy-dom does not auto-fire onloadeddata for src assignment, so we
+    // manually trigger it, then drain the resulting promise chain.
+    const video = createdVideos[0]
+    video.onloadeddata?.(new Event('loadeddata'))
+    await vi.runAllTimersAsync()
+
+    onTestFinished(() => {
+      node.widgets[0]?.onRemove?.()
+    })
+
+    return { node, video }
+  }
+
+  it('creates a video-preview widget and forwards canvas events while alive', async () => {
+    const { node, video } = await setup()
+
+    expect(node.widgets[0]?.name).toBe('video-preview')
+
+    video.dispatchEvent(new WheelEvent('wheel', { bubbles: true }))
+    video.dispatchEvent(new PointerEvent('pointermove', { bubbles: true }))
+    video.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true }))
+
+    expect(canvasInteractionsMock.handleWheel).toHaveBeenCalledTimes(1)
+    expect(canvasInteractionsMock.handlePointer).toHaveBeenCalledTimes(2)
+  })
+
+  it('detaches every listener when the widget is removed', async () => {
+    const { node, video } = await setup()
+
+    node.widgets[0]?.onRemove?.()
+
+    video.dispatchEvent(new WheelEvent('wheel', { bubbles: true }))
+    video.dispatchEvent(new PointerEvent('pointermove', { bubbles: true }))
+    video.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true }))
+
+    expect(canvasInteractionsMock.handleWheel).not.toHaveBeenCalled()
+    expect(canvasInteractionsMock.handlePointer).not.toHaveBeenCalled()
+  })
+})

src/composables/node/useNodeImage.ts

@@ -1,3 +1,4 @@
+import { useChainCallback } from '@/composables/functional/useChainCallback'
 import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
 import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
 import { useNodeOutputStore } from '@/stores/nodeOutputStore'
@@ -151,11 +152,6 @@ export const useNodeVideo = (node: LGraphNode, callback?: () => void) => {
       const video = document.createElement('video')
       Object.assign(video, VIDEO_DEFAULT_OPTIONS)
 
-      // Add event listeners for canvas interactions
-      video.addEventListener('wheel', handleWheel)
-      video.addEventListener('pointermove', handlePointer)
-      video.addEventListener('pointerdown', handlePointer)
-
       video.onloadeddata = () => {
         setMinDimensions(video)
         resolve(video)
@@ -176,6 +172,16 @@ export const useNodeVideo = (node: LGraphNode, callback?: () => void) => {
         minHeight,
         minWidth
       })
+
+      const controller = new AbortController()
+      const { signal } = controller
+      container.addEventListener('wheel', handleWheel, { signal })
+      container.addEventListener('pointermove', handlePointer, { signal })
+      container.addEventListener('pointerdown', handlePointer, { signal })
+
+      widget.onRemove = useChainCallback(widget.onRemove, () => {
+        controller.abort()
+      })
     }
   }