fix(ui): route transient dropdown menus through escMenuStack to stop listener leaks (#4684)

The app's ad-hoc dropdown/context menus each wire their own document-level
outside-click listener, but that listener only removes itself on an *outside*
click. Every other dismissal path -- clicking a menu item (which calls
el.remove() directly), a Cancel button, Escape, or the "close the
previously-open menu" reopen sweep -- tears the node down without
unregistering the listener, orphaning it on `document`. The stranded listener
then lingers and can break the next menu interaction: the recurring "the
button stops working until I refresh the page" class of bug (e.g. delete an
email, then the kebab/more button is dead on the other rows).

Route all 16 of these menus through the existing escMenuStack helper
(bindMenuDismiss / dismissOrRemove), exactly as documentLibrary.js
_showLibDropdown, cookbookRunning.js, and research/panel.js already do: a
single idempotent close() owns the teardown and is released on every dismissal
path, reopen sweeps use dismissOrRemove() instead of a bare .remove(), and
Escape flows through the central LIFO esc-stack arbiter. Net -49 lines.

Menus migrated: cookbook _showDepMenu; document export menu and
_openDocAiReplyChoice; emailInbox _showEmailMenu; emailLibrary
_showReaderMoreMenu / _showCardMenu / _showBulkActionsMenu; gallery
_showGalleryBulkMenu; notes _pickCustomDate / _openNoteCornerMenu; settings
(3 unified-integrations dropdowns); skills _openSkillMenu; tasks
_showTaskDropdown; compare _toggleExportMenu.

Per-menu semantics preserved (anchor-as-inside tests, the tasks 250ms
ghost-click guard, emailLibrary's reader-more-active anchor class and the
bulk-Cancel select-mode reset, settings' reused-vs-recreated lifecycles).

Six menus with custom lifecycles (notes _openReminderMenu, sessions
long-press, document markdown-toolbar, emojiPicker, compare model selector)
are intentionally left for a follow-up -- each needs individual review.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mocchibird
2026-06-22 20:40:56 +02:00
committed by GitHub
parent b899095f18
commit 4c82e4a172
10 changed files with 106 additions and 155 deletions
+16 -9
View File
@@ -8,6 +8,7 @@ import { clearDockSide } from './modalSnap.js';
import { sortModelIds } from './modelSort.js';
import { providerLogo } from './providers.js';
import { isAltGrEvent } from './platform.js';
import { bindMenuDismiss } from './escMenuStack.js';
let initialized = false;
let modalEl = null;
@@ -3838,7 +3839,10 @@ async function initUnifiedIntegrations() {
if (lbl) lbl.textContent = text;
if (ico) ico.innerHTML = _apiIconFor(k);
};
const _close = () => { menu.style.display = 'none'; };
// Menu is reused (hidden, not recreated). close() hides it and tears down
// its outside-click listener + Escape-stack entry; bindMenuDismiss is
// re-registered fresh on each open (see _open).
let _close = () => { menu.style.display = 'none'; };
const _open = () => {
menu.style.display = 'block';
const tRect = trig.getBoundingClientRect();
@@ -3847,8 +3851,7 @@ async function initUnifiedIntegrations() {
const above = tRect.top;
if (mRect.height > below && above > below) { menu.style.top = 'auto'; menu.style.bottom = 'calc(100% + 2px)'; }
else { menu.style.top = 'calc(100% + 2px)'; menu.style.bottom = 'auto'; }
const onDoc = (ev) => { if (!menu.contains(ev.target) && ev.target !== trig) { _close(); document.removeEventListener('click', onDoc, true); } };
setTimeout(() => document.addEventListener('click', onDoc, true), 0);
_close = bindMenuDismiss(menu, () => { menu.style.display = 'none'; }, (ev) => !menu.contains(ev.target) && ev.target !== trig);
};
trig.addEventListener('click', (e) => { e.stopPropagation(); menu.style.display === 'block' ? _close() : _open(); });
menu.querySelectorAll('.ufapi-option').forEach(btn => {
@@ -4584,7 +4587,10 @@ async function initUnifiedIntegrations() {
if (labelEl) labelEl.textContent = lbl;
if (iconEl) iconEl.innerHTML = PROV_LOGO[k] || _customLogo;
};
const _closeMenu = () => { menu.style.display = 'none'; };
// Menu is reused (hidden, not recreated). _closeMenu hides it and tears
// down its outside-click listener + Escape-stack entry; bindMenuDismiss is
// re-registered fresh on each open (see _openMenu).
let _closeMenu = () => { menu.style.display = 'none'; };
const _openMenu = () => {
menu.style.display = 'block';
// Drop-up when there's not enough room below the trigger.
@@ -4597,8 +4603,7 @@ async function initUnifiedIntegrations() {
} else {
menu.style.top = 'calc(100% + 2px)'; menu.style.bottom = 'auto';
}
const onDoc = (ev) => { if (!menu.contains(ev.target) && ev.target !== trigger) { _closeMenu(); document.removeEventListener('click', onDoc, true); } };
setTimeout(() => document.addEventListener('click', onDoc, true), 0);
_closeMenu = bindMenuDismiss(menu, () => { menu.style.display = 'none'; }, (ev) => !menu.contains(ev.target) && ev.target !== trigger);
};
trigger.addEventListener('click', (e) => { e.stopPropagation(); menu.style.display === 'block' ? _closeMenu() : _openMenu(); });
menu.querySelectorAll('.ufp-option').forEach(btn => {
@@ -5650,8 +5655,11 @@ async function initUnifiedIntegrations() {
addBtn.parentElement.style.position = 'relative';
addBtn.parentElement.classList.add('uf-add-anchor');
}
// Menu is created per open and removed on close. _closeMenu routes through
// the bindMenuDismiss close() bound when the menu opens, so the outside-click
// listener + Escape-stack entry are torn down alongside the node removal.
let _menuEl = null;
const _closeMenu = () => { if (_menuEl) { _menuEl.remove(); _menuEl = null; } };
let _closeMenu = () => {};
addBtn.addEventListener('click', (e) => {
e.stopPropagation();
if (_menuEl) { _closeMenu(); return; }
@@ -5683,8 +5691,7 @@ async function initUnifiedIntegrations() {
showForm(k, 'new');
});
});
const onDoc = (ev) => { if (!menu.contains(ev.target) && ev.target !== addBtn) { _closeMenu(); document.removeEventListener('click', onDoc, true); } };
setTimeout(() => document.addEventListener('click', onDoc, true), 0);
_closeMenu = bindMenuDismiss(menu, () => { menu.remove(); _menuEl = null; }, (ev) => !menu.contains(ev.target) && ev.target !== addBtn);
});
}