From 7709b0d97bee5e0a2e1f5c5035ef48a7464f330b Mon Sep 17 00:00:00 2001 From: librelad Date: Sun, 31 May 2026 14:48:54 +0100 Subject: [PATCH] fix(backup): dispose document listeners on unmount (sidebar stacking bug) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original report: clicking a backup sidebar tab loaded content on top of the old content. Root cause (flagged in the unmount comment as deferred): BackupPage.bindEvents() attaches document-level click/input/change listeners guarded only by the instance-level this.eventBound, and unmount() nulled window.backupPage WITHOUT removing them. Each revisit added another full set of listeners bound to a stale BackupPage, all firing on every click and mutating the live DOM (double tab-switches, double modal opens, stale-instance renders). Fix (mirrors the kernel's MountContext pattern): give BackupPage an AbortController, bind the three document listeners to its signal, add dispose() that aborts them (+ drops the task-refresh reg + clears the timer), and call it from the feature module's unmount(). Revisits now start clean — one live instance, one set of listeners. Co-Authored-By: Claude Opus 4.8 Signed-off-by: librelad --- .../components/backup/core/js/backup-page.js | 17 ++++++++++++++--- .../frontend/components/backup/index.js | 10 +++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/containers/libreportal/frontend/components/backup/core/js/backup-page.js b/containers/libreportal/frontend/components/backup/core/js/backup-page.js index c7d117f..1d54e0c 100644 --- a/containers/libreportal/frontend/components/backup/core/js/backup-page.js +++ b/containers/libreportal/frontend/components/backup/core/js/backup-page.js @@ -16,6 +16,7 @@ class BackupPage { this.taskManager = (typeof TaskManager !== 'undefined') ? new TaskManager() : null; this.eventBound = false; this._taskRefreshTimer = null; + this._ac = new AbortController(); // dispose() aborts this to remove the document listeners on unmount } async init() { @@ -294,13 +295,13 @@ class BackupPage { this.saveSection(saveBtn.dataset.backupSave); return; } - }); + }, { signal: this._ac.signal }); document.addEventListener('input', (e) => { if (e.target.id === 'backup-snapshot-filter' || e.target.id === 'backup-snapshot-repo') { this.renderSnapshots(); } - }); + }, { signal: this._ac.signal }); // Type select changes refresh the visible connection fields inline. // Retention preset changes are handled by applyRetentionPreset, which @@ -323,7 +324,17 @@ class BackupPage { if (presetSel) { this.applyRetentionPreset(presetSel); } - }); + }, { signal: this._ac.signal }); + } + + /* Release everything this page attached to document/window so navigating + away (kernel unmount) can't leave stale listeners firing on the live DOM — + the cause of the backup sidebar "content stacks on revisit" bug. Called by + the feature module's unmount(). */ + dispose() { + try { this._ac && this._ac.abort(); } catch (_) {} + try { window.taskRefresh && window.taskRefresh.unregister('backups'); } catch (_) {} + if (this._taskRefreshTimer) { clearTimeout(this._taskRefreshTimer); this._taskRefreshTimer = null; } } diff --git a/containers/libreportal/frontend/components/backup/index.js b/containers/libreportal/frontend/components/backup/index.js index 50483b0..64b6db9 100644 --- a/containers/libreportal/frontend/components/backup/index.js +++ b/containers/libreportal/frontend/components/backup/index.js @@ -46,11 +46,11 @@ LP.features.register({ }, async unmount(ctx) { - // Best-effort teardown. BackupPage self-guards stale work via - // (window.backupPage === this), so nulling the global neutralises any - // pending task-refresh repaint; we also drop its coordinator registration. - // A proper dispose() (removing the leaked document listeners) lands with - // the Phase 5 backup decomposition. + // Release the page's document listeners + task-refresh registration so a + // navigation away doesn't leave stale BackupPage listeners firing on the + // live DOM — the backup sidebar "content stacks on revisit" bug. dispose() + // aborts the click/input/change listeners and drops the coordinator reg. + try { window.backupPage && window.backupPage.dispose(); } catch (_) {} try { ctx.services.tasks.refresh && ctx.services.tasks.refresh.unregister('backups'); } catch (_) {} window.backupPage = null; },