mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-17 10:15:27 -04:00
fix(notes): track + remove the select-mode Esc keydown listener so it doesn't leak per open (#2792)
This commit is contained in:
+17
-2
@@ -31,6 +31,9 @@ let _reminderTimer = null;
|
|||||||
// (previously leaked one per openPanel; on multi-open sessions this
|
// (previously leaked one per openPanel; on multi-open sessions this
|
||||||
// stacked dozens of identical handlers).
|
// stacked dozens of identical handlers).
|
||||||
let _notesKeydownHandler = null;
|
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';
|
const REMINDER_FIRED_KEY = 'odysseus-notes-reminder-fired';
|
||||||
// Note IDs already shown with the entry-glow once. Re-set when the user
|
// 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.
|
// reschedules the reminder so the new firing glows again on next open.
|
||||||
@@ -54,6 +57,10 @@ function _forceCloseNotesPanel() {
|
|||||||
document.removeEventListener('keydown', _notesKeydownHandler);
|
document.removeEventListener('keydown', _notesKeydownHandler);
|
||||||
_notesKeydownHandler = null;
|
_notesKeydownHandler = null;
|
||||||
}
|
}
|
||||||
|
if (_notesSelectEscHandler) {
|
||||||
|
document.removeEventListener('keydown', _notesSelectEscHandler, true);
|
||||||
|
_notesSelectEscHandler = null;
|
||||||
|
}
|
||||||
if (_reminderTimer) {
|
if (_reminderTimer) {
|
||||||
clearInterval(_reminderTimer);
|
clearInterval(_reminderTimer);
|
||||||
_reminderTimer = null;
|
_reminderTimer = null;
|
||||||
@@ -1270,13 +1277,17 @@ export function openPanel() {
|
|||||||
// than a *-bulk-cancel button, so the global Esc-cancel handler in
|
// than a *-bulk-cancel button, so the global Esc-cancel handler in
|
||||||
// keyboard-shortcuts.js can't reach it — handle it here. Capture phase
|
// keyboard-shortcuts.js can't reach it — handle it here. Capture phase
|
||||||
// + stopPropagation so Esc cancels select instead of closing the panel.
|
// + 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) {
|
if (e.key === 'Escape' && _selectMode) {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
e.stopPropagation();
|
e.stopPropagation();
|
||||||
_exitSelectMode();
|
_exitSelectMode();
|
||||||
}
|
}
|
||||||
}, true);
|
};
|
||||||
|
document.addEventListener('keydown', _notesSelectEscHandler, true);
|
||||||
document.getElementById('notes-select-all').addEventListener('change', (e) => {
|
document.getElementById('notes-select-all').addEventListener('change', (e) => {
|
||||||
if (e.target.checked) _notes.forEach(n => _selectedIds.add(n.id));
|
if (e.target.checked) _notes.forEach(n => _selectedIds.add(n.id));
|
||||||
else _selectedIds.clear();
|
else _selectedIds.clear();
|
||||||
@@ -1580,6 +1591,10 @@ export function closePanel(direction) {
|
|||||||
document.removeEventListener('keydown', _notesKeydownHandler);
|
document.removeEventListener('keydown', _notesKeydownHandler);
|
||||||
_notesKeydownHandler = null;
|
_notesKeydownHandler = null;
|
||||||
}
|
}
|
||||||
|
if (_notesSelectEscHandler) {
|
||||||
|
document.removeEventListener('keydown', _notesSelectEscHandler, true);
|
||||||
|
_notesSelectEscHandler = null;
|
||||||
|
}
|
||||||
if (_reminderTimer) {
|
if (_reminderTimer) {
|
||||||
clearInterval(_reminderTimer);
|
clearInterval(_reminderTimer);
|
||||||
_reminderTimer = null;
|
_reminderTimer = null;
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in New Issue
Block a user