From 2e207fc31584235e4360b475dba8fca67911b2b6 Mon Sep 17 00:00:00 2001 From: nubs Date: Fri, 5 Jun 2026 14:25:05 +0000 Subject: [PATCH] fix(notes): track + remove the select-mode Esc keydown listener so it doesn't leak per open (#2792) --- static/js/notes.js | 19 ++++++++++++-- tests/test_notes_select_esc_listener_js.py | 30 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/test_notes_select_esc_listener_js.py diff --git a/static/js/notes.js b/static/js/notes.js index 039b31089..e64e5035c 100644 --- a/static/js/notes.js +++ b/static/js/notes.js @@ -31,6 +31,9 @@ let _reminderTimer = null; // (previously leaked one per openPanel; on multi-open sessions this // stacked dozens of identical handlers). let _notesKeydownHandler = null; +// Capture-phase "Esc cancels select mode" listener on document — tracked so it +// is removed on close instead of leaking +1 per panel open/close cycle. +let _notesSelectEscHandler = null; const REMINDER_FIRED_KEY = 'odysseus-notes-reminder-fired'; // Note IDs already shown with the entry-glow once. Re-set when the user // reschedules the reminder so the new firing glows again on next open. @@ -54,6 +57,10 @@ function _forceCloseNotesPanel() { document.removeEventListener('keydown', _notesKeydownHandler); _notesKeydownHandler = null; } + if (_notesSelectEscHandler) { + document.removeEventListener('keydown', _notesSelectEscHandler, true); + _notesSelectEscHandler = null; + } if (_reminderTimer) { clearInterval(_reminderTimer); _reminderTimer = null; @@ -1270,13 +1277,17 @@ export function openPanel() { // than a *-bulk-cancel button, so the global Esc-cancel handler in // keyboard-shortcuts.js can't reach it — handle it here. Capture phase // + stopPropagation so Esc cancels select instead of closing the panel. - document.addEventListener('keydown', (e) => { + if (_notesSelectEscHandler) { + document.removeEventListener('keydown', _notesSelectEscHandler, true); + } + _notesSelectEscHandler = (e) => { if (e.key === 'Escape' && _selectMode) { e.preventDefault(); e.stopPropagation(); _exitSelectMode(); } - }, true); + }; + document.addEventListener('keydown', _notesSelectEscHandler, true); document.getElementById('notes-select-all').addEventListener('change', (e) => { if (e.target.checked) _notes.forEach(n => _selectedIds.add(n.id)); else _selectedIds.clear(); @@ -1580,6 +1591,10 @@ export function closePanel(direction) { document.removeEventListener('keydown', _notesKeydownHandler); _notesKeydownHandler = null; } + if (_notesSelectEscHandler) { + document.removeEventListener('keydown', _notesSelectEscHandler, true); + _notesSelectEscHandler = null; + } if (_reminderTimer) { clearInterval(_reminderTimer); _reminderTimer = null; diff --git a/tests/test_notes_select_esc_listener_js.py b/tests/test_notes_select_esc_listener_js.py new file mode 100644 index 000000000..dedc612a2 --- /dev/null +++ b/tests/test_notes_select_esc_listener_js.py @@ -0,0 +1,30 @@ +"""Issue #2791 — the Notes panel's capture-phase "Esc cancels select mode" +keydown listener must be tracked and removed on close, not leaked anonymously on +every open/close cycle. + +notes.js is a browser ES module with a heavy import chain (can't be node-imported +in isolation), so — per the repo's convention for DOM-coupled guards (cf. the +document.js diff-discard and memory.js filter-guard tests) — this asserts the +tracked-handler pattern in source. +""" +from pathlib import Path + +SRC = Path("static/js/notes.js").read_text(encoding="utf-8") + + +def test_select_esc_listener_is_tracked_not_anonymous(): + assert "let _notesSelectEscHandler = null;" in SRC + # added via the tracked module-level var in capture phase + assert "document.addEventListener('keydown', _notesSelectEscHandler, true);" in SRC + + +def test_select_esc_listener_removed_with_matching_capture_flag(): + # remove-before-add in openPanel + removal in both close paths => >= 3, + # each with the `true` capture flag (a removal without it would not match). + removals = SRC.count("document.removeEventListener('keydown', _notesSelectEscHandler, true);") + assert removals >= 3, removals + + +def test_old_anonymous_capture_listener_is_gone(): + # the leak was an inline anonymous capture listener; it must no longer exist. + assert "addEventListener('keydown', (e) => {\n if (e.key === 'Escape' && _selectMode)" not in SRC