From 2d09335cfbf1cc13eaa1453d73e3edb03fe7aef6 Mon Sep 17 00:00:00 2001 From: efiten Date: Sat, 17 Oct 2026 21:30:57 +0200 Subject: [PATCH] fix(packets): keep the node filter pick on a slow click or late search (#2243) (#2244) Fixes #2243. ## Problem The packets node filter selected a suggestion on `click`, while the input's `blur` handler hides the dropdown 200 ms after focus leaves. A press held longer than that, or a search response that rebuilt the list between mousedown and mouseup, lost the pick: the typed text stayed and no `node=` filter was applied. ## Change - `public/packets.js`: suggestions select on `mousedown` with `preventDefault`, so the input keeps focus and the pick happens before any rebuild. `live.js` already does this for its node filter. - `public/packets.js`: a sequence counter drops search responses for an older query, bumped on every search and on a pick, so a late response can't replace or reopen the list. - `tests/e2e/test-packets-node-filter-e2e.js` (new), registered in `scripts/non-unit-tests.json` and on shard 1 in `deploy.yml`. ## Tests The new E2E has three cases: a normal click, a 400 ms press, and a search response released mid-click. | Code | Normal click | 400 ms press | Response mid-click | |---|---|---|---| | master | pass | fail | fail | | this PR without the sequence guard | pass | pass | fail (list reopens) | | this PR | pass | pass | pass | Run locally against the freshened e2e fixture. `sh test-all.sh` passes (locally `test-preflight-xss-gate.js` needs a real `python3` on PATH and `PYTHONUTF8=1`, unrelated to this change). ## Not done - Full `test-e2e-playwright.js` not run locally. - Could not reproduce on the reporter's instance with the browser tool, whose clicks are shorter than 200 ms. The 400 ms Playwright press reproduces it on the fixture. - `live.js` and `packets.js` still carry two separate node-autocomplete implementations. Merging them is out of scope here. Co-authored-by: Claude Opus 5.5 --- .github/workflows/deploy.yml | 1 + public/packets.js | 11 +- scripts/non-unit-tests.json | 1 + tests/e2e/test-packets-node-filter-e2e.js | 128 ++++++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/test-packets-node-filter-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 1d9cfec1..92857398 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -696,6 +696,7 @@ jobs: suite 2 test-marker-outline-weight.js CHROMIUM_REQUIRE=1 BASE_URL=$B suite 2 test-node-reach-e2e.js CHROMIUM_REQUIRE=1 BASE_URL=$B suite 3 test-packets-compact-columns-e2e.js CHROMIUM_REQUIRE=1 BASE_URL=$B + suite 1 test-packets-node-filter-e2e.js BASE_URL=$B suite 3 test-packets-scope-column.js CHROMIUM_REQUIRE=1 BASE_URL=$B suite 3 test-path-inspector-e2e.js CHROMIUM_REQUIRE=1 BASE_URL=$B suite 3 test-pr-1490-live-map-gpu-animations-e2e.js CHROMIUM_REQUIRE=1 BASE_URL=$B diff --git a/public/packets.js b/public/packets.js index a059e7c6..c6bca41c 100644 --- a/public/packets.js +++ b/public/packets.js @@ -2374,8 +2374,12 @@ const fNodeDrop = document.getElementById('fNodeDropdown'); fNode.value = filters.nodeName || ''; let nodeActiveIdx = -1; + // Bumped per search and on pick, so a slower response for an older query + // can't replace the list or reopen it after a selection. + let nodeSearchSeq = 0; fNode.addEventListener('input', debounce(async (e) => { const q = e.target.value.trim(); + const seq = ++nodeSearchSeq; nodeActiveIdx = -1; fNode.setAttribute('aria-activedescendant', ''); if (!q) { @@ -2387,6 +2391,7 @@ try { const resp = await fetch('/api/nodes/search?q=' + encodeURIComponent(q)); const data = await resp.json(); + if (seq !== nodeSearchSeq) return; const nodes = data.nodes || []; if (nodes.length === 0) { fNodeDrop.classList.add('hidden'); fNode.setAttribute('aria-expanded', 'false'); return; } fNodeDrop.innerHTML = nodes.map((n, i) => @@ -2395,7 +2400,10 @@ fNodeDrop.classList.remove('hidden'); fNode.setAttribute('aria-expanded', 'true'); fNodeDrop.querySelectorAll('.node-filter-option').forEach(opt => { - opt.addEventListener('click', () => { + // mousedown, not click: keeps focus in the input so the blur timer + // can't hide the list during a slow click. + opt.addEventListener('mousedown', (ev) => { + ev.preventDefault(); selectNodeOption(opt); }); }); @@ -2403,6 +2411,7 @@ }, 250)); function selectNodeOption(opt) { + nodeSearchSeq++; filters.node = opt.dataset.key; filters.nodeName = opt.dataset.name; fNode.value = opt.dataset.name; diff --git a/scripts/non-unit-tests.json b/scripts/non-unit-tests.json index 42948996..3709a422 100644 --- a/scripts/non-unit-tests.json +++ b/scripts/non-unit-tests.json @@ -108,6 +108,7 @@ "tests/e2e/test-nodes-export-e2e.js", "tests/e2e/test-observer-iata-1188-e2e.js", "tests/e2e/test-packets-compact-columns-e2e.js", + "tests/e2e/test-packets-node-filter-e2e.js", "tests/e2e/test-packets-scope-column.js", "tests/e2e/test-path-inspector-coverage-e2e.js", "tests/e2e/test-rx-coverage-mobile-nav-e2e.js", diff --git a/tests/e2e/test-packets-node-filter-e2e.js b/tests/e2e/test-packets-node-filter-e2e.js new file mode 100644 index 00000000..b5c2f8d1 --- /dev/null +++ b/tests/e2e/test-packets-node-filter-e2e.js @@ -0,0 +1,128 @@ +/** + * Packets page node filter: picking an autocomplete suggestion must apply the + * filter even when the click is slow or the list is rebuilt mid-click. + * + * The input's blur handler hides the dropdown 200 ms after focus leaves, and a + * search response can replace the list while the button is down. A selection + * that waited for `click` was lost in both cases: the input kept the typed + * text and no node= filter was applied. + * + * Usage: + * BASE_URL=http://localhost:13581 node tests/e2e/test-packets-node-filter-e2e.js + */ +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +let pass = 0, fail = 0; + +function assert(cond, msg) { if (!cond) throw new Error(msg || 'assert'); } + +async function test(name, fn) { + try { + await fn(); + console.log(` ✅ ${name}`); + pass++; + } catch (e) { + console.log(` ❌ ${name}: ${e.message}`); + fail++; + } +} + +async function openPackets(ctx) { + const page = await ctx.newPage(); + page.setDefaultTimeout(10000); + await page.goto(BASE + '/#/packets', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#fNode'); + return page; +} + +async function typeAndWaitForSuggestion(page, q) { + await page.fill('#fNode', ''); + await page.type('#fNode', q); + await page.waitForSelector('#fNodeDropdown:not(.hidden) .node-filter-option'); + return page.locator('#fNodeDropdown .node-filter-option').first(); +} + +async function pressAndHold(page, locator, holdMs, duringHold) { + const box = await locator.boundingBox(); + assert(box, 'suggestion must be visible'); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); + await page.mouse.down(); + if (duringHold) await duringHold(); + await page.waitForTimeout(holdMs); + await page.mouse.up(); +} + +async function assertNodeApplied(page, name, key) { + await page.waitForFunction(k => location.hash.includes('node=' + k), key, { timeout: 3000 }) + .catch(() => { throw new Error(`URL has no node=${key.slice(0, 8)}…, got ${page.url()}`); }); + const value = await page.inputValue('#fNode'); + assert(value === name, `input must show the picked name "${name}", got "${value}"`); +} + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + const ctx = await browser.newContext(); + console.log(`Packets node filter E2E against ${BASE}`); + + const res = await ctx.request.get(BASE + '/api/nodes?limit=50'); + assert(res.ok(), 'fixture node request must succeed'); + const named = ((await res.json()).nodes || []).find(n => n.name && n.name.length >= 3 && n.public_key); + assert(named, 'fixture must contain a named node'); + const query = named.name.slice(0, 3); + + await test('a normal click on a suggestion applies the node filter', async () => { + const page = await openPackets(ctx); + const opt = await typeAndWaitForSuggestion(page, query); + const name = await opt.getAttribute('data-name'); + const key = await opt.getAttribute('data-key'); + await opt.click(); + await assertNodeApplied(page, name, key); + await page.close(); + }); + + await test('a click held longer than the blur delay still applies the node filter', async () => { + const page = await openPackets(ctx); + const opt = await typeAndWaitForSuggestion(page, query); + const name = await opt.getAttribute('data-name'); + const key = await opt.getAttribute('data-key'); + await pressAndHold(page, opt, 400); + await assertNodeApplied(page, name, key); + await page.close(); + }); + + await test('a search response that rebuilds the list mid-click does not drop the pick', async () => { + const page = await openPackets(ctx); + const opt = await typeAndWaitForSuggestion(page, query); + const name = await opt.getAttribute('data-name'); + const key = await opt.getAttribute('data-key'); + let release; + const held = new Promise(r => { release = r; }); + let seen = false; + await page.route(url => url.pathname === '/api/nodes/search', async route => { + seen = true; + await held; + await route.continue(); + }); + // A keystroke whose search is still in flight when the user clicks. + await page.type('#fNode', named.name.charAt(3) || ' '); + await page.waitForTimeout(400); + assert(seen, 'the follow-up search request must be in flight'); + await pressAndHold(page, opt, 150, async () => { + release(); + await page.waitForTimeout(200); + }); + await assertNodeApplied(page, name, key); + const reopened = await page.$('#fNodeDropdown:not(.hidden)'); + assert(!reopened, 'the late response for the older query must not reopen the list after the pick'); + await page.close(); + }); + + await browser.close(); + console.log(`\n${pass}/${pass + fail} passed${fail ? `, ${fail} failed` : ''}`); + process.exit(fail ? 1 : 0); +})().catch(e => { console.error(e); process.exit(2); });