From 0b82ca791e2fee2a171c21b34bea4dc32f164196 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot <259247574+Kpa-clawbot@users.noreply.github.com> Date: Mon, 30 Mar 2026 18:37:37 -0700 Subject: [PATCH] Fix #249 BYOP dialog stacking and close behavior - Prevent leaked document click handlers from stacking across packets page init - Ensure BYOP dialog is singleton and close removes all overlay layers - Add frontend regression tests for BYOP overlay and handler de-dup - Bump public asset cache busters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- public/index.html | 56 ++++++++++++++++++++-------------------- public/packets.js | 37 +++++++++++++++++++++----- test-frontend-helpers.js | 23 +++++++++++++++++ 3 files changed, 82 insertions(+), 34 deletions(-) diff --git a/public/index.html b/public/index.html index 36cc440d..fb43f146 100644 --- a/public/index.html +++ b/public/index.html @@ -22,9 +22,9 @@ - - - + + + @@ -81,30 +81,30 @@
- - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/public/packets.js b/public/packets.js index c6085c1f..68ce3c65 100644 --- a/public/packets.js +++ b/public/packets.js @@ -157,9 +157,29 @@ let directPacketId = null; let directPacketHash = null; let initGeneration = 0; + let _docActionHandler = null; + let _docMenuCloseHandler = null; + let _docColMenuCloseHandler = null; let directObsId = null; + function removeAllByopOverlays() { + document.querySelectorAll('.byop-overlay').forEach(function (el) { el.remove(); }); + } + + function bindDocumentHandler(kind, eventName, handler) { + const prev = kind === 'action' + ? _docActionHandler + : kind === 'menu' + ? _docMenuCloseHandler + : _docColMenuCloseHandler; + if (prev) document.removeEventListener(eventName, prev); + document.addEventListener(eventName, handler); + if (kind === 'action') _docActionHandler = handler; + else if (kind === 'menu') _docMenuCloseHandler = handler; + else _docColMenuCloseHandler = handler; + } + function renderTimestampCell(isoString) { if (typeof formatTimestampWithTooltip !== 'function' || typeof getTimestampMode !== 'function') { return escapeHtml(typeof timeAgo === 'function' ? timeAgo(isoString) : '—'); @@ -237,7 +257,7 @@ } // Event delegation for data-action buttons - document.addEventListener('click', function (e) { + bindDocumentHandler('action', 'click', function (e) { var btn = e.target.closest('[data-action]'); if (!btn) return; if (btn.dataset.action === 'pkt-refresh') loadPackets(); @@ -376,6 +396,10 @@ function destroy() { if (wsHandler) offWS(wsHandler); wsHandler = null; + if (_docActionHandler) { document.removeEventListener('click', _docActionHandler); _docActionHandler = null; } + if (_docMenuCloseHandler) { document.removeEventListener('click', _docMenuCloseHandler); _docMenuCloseHandler = null; } + if (_docColMenuCloseHandler) { document.removeEventListener('click', _docColMenuCloseHandler); _docColMenuCloseHandler = null; } + removeAllByopOverlays(); packets = []; hashIndex = new Map(); selectedId = null; filtersBuilt = false; @@ -705,7 +729,7 @@ }); // Close multi-select menus on outside click - document.addEventListener('click', (e) => { + bindDocumentHandler('menu', 'click', (e) => { const obsWrap = document.getElementById('observerFilterWrap'); const typeWrap = document.getElementById('typeFilterWrap'); if (obsWrap && !obsWrap.contains(e.target)) { const m = obsWrap.querySelector('.multi-select-menu'); if (m) m.classList.remove('open'); } @@ -820,7 +844,7 @@ e.stopPropagation(); colMenu.classList.toggle('open'); }); - document.addEventListener('click', () => colMenu.classList.remove('open')); + bindDocumentHandler('colmenu', 'click', () => colMenu.classList.remove('open')); applyColVisibility(); document.getElementById('hexHashToggle').addEventListener('click', function () { @@ -1548,9 +1572,10 @@ // BYOP modal — decode only, no DB injection function showBYOP() { + removeAllByopOverlays(); const triggerBtn = document.querySelector('[data-action="pkt-byop"]'); const overlay = document.createElement('div'); - overlay.className = 'modal-overlay'; + overlay.className = 'modal-overlay byop-overlay'; overlay.innerHTML = '
' + '

📦 Decode a Packet

' + '

Paste raw hex bytes from your radio or MQTT feed:

' @@ -1561,7 +1586,7 @@ document.body.appendChild(overlay); const modal = overlay.querySelector('.byop-modal'); - const close = () => { overlay.remove(); if (triggerBtn) triggerBtn.focus(); }; + const close = () => { removeAllByopOverlays(); if (triggerBtn) triggerBtn.focus(); }; overlay.querySelector('.byop-x').onclick = close; overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); @@ -1597,7 +1622,7 @@ async function doDecode() { const hex = textarea.value.trim().replace(/[\s\n]/g, ''); - const result = document.getElementById('byopResult'); + const result = overlay.querySelector('#byopResult'); if (!hex) { result.innerHTML = '

Enter hex data

'; return; } if (!/^[0-9a-fA-F]+$/.test(hex)) { result.innerHTML = '

Invalid hex — only 0-9 and A-F allowed

'; return; } result.innerHTML = '

Decoding...

'; diff --git a/test-frontend-helpers.js b/test-frontend-helpers.js index b8a759a0..1a94f9da 100644 --- a/test-frontend-helpers.js +++ b/test-frontend-helpers.js @@ -1318,6 +1318,29 @@ console.log('\n=== compare.js: comparePacketSets ==='); assert.ok(packetsSource.includes("classList.remove('detail-collapsed')"), 'selectPacket should remove detail-collapsed class'); }); + + test('BYOP uses dedicated overlay class and clears existing overlays before opening', () => { + assert.ok(packetsSource.includes("overlay.className = 'modal-overlay byop-overlay'"), + 'BYOP overlay should have byop-overlay class'); + assert.ok(/function showBYOP\(\)\s*\{\s*removeAllByopOverlays\(\);/m.test(packetsSource), + 'showBYOP should clear existing overlays before creating a new one'); + }); + + test('BYOP close removes all overlays in one click', () => { + assert.ok(packetsSource.includes("const close = () => { removeAllByopOverlays(); if (triggerBtn) triggerBtn.focus(); };"), + 'close handler should remove all BYOP overlays'); + }); + + test('packets page de-duplicates document click handlers', () => { + assert.ok(packetsSource.includes("bindDocumentHandler('action', 'click'"), + 'action click handler should be bound through bindDocumentHandler'); + assert.ok(packetsSource.includes("bindDocumentHandler('menu', 'click'"), + 'menu close handler should be bound through bindDocumentHandler'); + assert.ok(packetsSource.includes("bindDocumentHandler('colmenu', 'click'"), + 'column menu close handler should be bound through bindDocumentHandler'); + assert.ok(packetsSource.includes("if (prev) document.removeEventListener(eventName, prev);"), + 'bindDocumentHandler should remove previous handler before re-binding'); + }); } // ===== APP.JS: formatEngineBadge =====