From 3a17f5eb147c6c44e9f2e20ddeaaf070b146664e Mon Sep 17 00:00:00 2001
From: Reorx <novoreorx@gmail.com>
Date: Mon, 17 Aug 2026 21:45:07 +0800
Subject: [PATCH] Remove the scroll listener with the capture flag it was
 registered with
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`destroy()` called `document.removeEventListener('scroll', onScroll)` while
the listener had been registered with `{ capture: true }`. removeEventListener
only matches a registration whose capture flag is equal, so the call was a
no-op and every destroyed renderer left its scroll listener on the document.

Each orphan keeps running `redraw(true)` — a forced, full highlight-layer
rebuild — on every scroll event in the page, so the cost accumulates with
every mount/destroy cycle (collapsing a panel, re-creating an annotator,
client-side route changes).

Adds a regression test that scrolls after destroy and asserts the painter is
not asked to redraw. It fails without the one-line change above.

Fixes #283

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../src/rendering/base-renderer.ts            |  4 +-
 .../test/rendering/base-renderer.test.ts      | 87 +++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 packages/text-annotator/test/rendering/base-renderer.test.ts

diff --git a/packages/text-annotator/src/rendering/base-renderer.ts b/packages/text-annotator/src/rendering/base-renderer.ts
index 07af9560..2f799082 100644
--- a/packages/text-annotator/src/rendering/base-renderer.ts
+++ b/packages/text-annotator/src/rendering/base-renderer.ts
@@ -229,7 +229,9 @@ export const createRenderer = <T extends TextAnnotatorState<TextAnnotationLike,
     unsubscribeSelection();
     unsubscribeHover();
 
-    document.removeEventListener('scroll', onScroll);
+    // Note: the capture flag must match the one used on registration,
+    // otherwise this call matches no listener and silently does nothing.
+    document.removeEventListener('scroll', onScroll, { capture: true });
 
     // onResize.clear();
     window.removeEventListener('resize', onResize);
diff --git a/packages/text-annotator/test/rendering/base-renderer.test.ts b/packages/text-annotator/test/rendering/base-renderer.test.ts
new file mode 100644
index 00000000..b17b4692
--- /dev/null
+++ b/packages/text-annotator/test/rendering/base-renderer.test.ts
@@ -0,0 +1,87 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createRenderer, type Painter } from '../../src/rendering/base-renderer';
+
+/**
+ * The renderer redraws through a 10ms debounce and a rAF, so give both a
+ * chance to run before asserting.
+ */
+const flushRedraw = () => new Promise(resolve => setTimeout(resolve, 50));
+
+const createPainter = () => ({
+  destroy: vi.fn(),
+  redraw: vi.fn(),
+  setVisible: vi.fn()
+}) satisfies Painter;
+
+const createState = () => ({
+  store: {
+    observe: vi.fn(),
+    unobserve: vi.fn(),
+    getAt: vi.fn(),
+    getIntersecting: vi.fn(() => []),
+    recalculatePositions: vi.fn()
+  },
+  selection: {
+    subscribe: vi.fn(() => vi.fn()),
+    selected: [],
+    evalSelectAction: vi.fn()
+  },
+  hover: {
+    subscribe: vi.fn(() => vi.fn()),
+    current: undefined,
+    set: vi.fn()
+  }
+});
+
+const viewport = { set: vi.fn() };
+
+describe('createRenderer', () => {
+
+  let container: HTMLElement;
+
+  beforeEach(() => {
+    // jsdom has no ResizeObserver.
+    global.ResizeObserver = class {
+      observe() {}
+      unobserve() {}
+      disconnect() {}
+    };
+
+    container = document.createElement('div');
+    document.body.appendChild(container);
+  });
+
+  afterEach(() => {
+    container.remove();
+  });
+
+  it('should redraw when the page scrolls', async () => {
+    const painter = createPainter();
+    const renderer = createRenderer(painter, container, createState() as any, viewport as any);
+
+    container.dispatchEvent(new Event('scroll'));
+    await flushRedraw();
+
+    expect(painter.redraw).toHaveBeenCalled();
+
+    renderer.destroy();
+  });
+
+  it('should stop redrawing on scroll after destroy', async () => {
+    // Regression test: the scroll listener is registered with { capture: true },
+    // so destroy() has to remove it with the same flag. Without it the listener
+    // stays on the document forever, and every destroyed renderer keeps forcing
+    // a full redraw on every scroll event in the page.
+    const painter = createPainter();
+    const renderer = createRenderer(painter, container, createState() as any, viewport as any);
+
+    renderer.destroy();
+    painter.redraw.mockClear();
+
+    container.dispatchEvent(new Event('scroll'));
+    await flushRedraw();
+
+    expect(painter.redraw).not.toHaveBeenCalled();
+  });
+
+});