From be868cff100f78e38dcc2e5e695515b0eb4f9102 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:20:02 +0000 Subject: [PATCH 01/15] test(server): reproduce the silent missing-node lookup failure (#208) A failed lookupMissingNode (schema drift on inactive_nodes) answers the bare 404 without a trace in the log. Pin that it is logged exactly once for repeated requests, carries the error but not the requested key, and that a request cancelled by its client is not logged. Co-Authored-By: Claude Opus 5.5 --- cmd/server/issue199_node_not_found_test.go | 56 ++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/cmd/server/issue199_node_not_found_test.go b/cmd/server/issue199_node_not_found_test.go index 086dbdbaf..3460fc8e7 100644 --- a/cmd/server/issue199_node_not_found_test.go +++ b/cmd/server/issue199_node_not_found_test.go @@ -1,7 +1,10 @@ package main import ( + "bytes" + "context" "encoding/json" + "log" "net/http/httptest" "strings" "testing" @@ -153,3 +156,56 @@ func TestNodeDetail404HiddenIdentityStaysBare(t *testing.T) { }) } } + +// #208 item 1: a failed lookup still answers the bare 404, but is logged -- +// once at first, then at most once per missingNodeLogEvery -- so a broken +// lookup (schema drift on inactive_nodes) does not pass for a plain miss. +func TestNodeDetail404LookupErrorIsLoggedOnce(t *testing.T) { + srv, router := setupTestServer(t) + // Schema drift: an inactive_nodes without the columns the lookup reads. + if _, err := srv.db.conn.Exec(`CREATE TABLE inactive_nodes (public_key TEXT PRIMARY KEY, name TEXT)`); err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + prev := log.Writer() + log.SetOutput(&buf) + defer log.SetOutput(prev) + + for i := 0; i < 3; i++ { + code, body := issue199Get(t, router, issue199Unknown) + if code != 404 || len(body) != 1 || body["error"] == nil { + t.Fatalf("request %d: status=%d body=%v, want the bare 404", i, code, body) + } + } + out := buf.String() + if n := strings.Count(out, "missing-node lookup failed"); n != 1 { + t.Fatalf("logged %d lookup failures for 3 requests, want 1; log:\n%s", n, out) + } + if !strings.Contains(out, "no such column") { + t.Errorf("log line does not carry the error: %s", out) + } + if strings.Contains(out, issue199Unknown) { + t.Errorf("log line carries the requested key: %s", out) + } +} + +// A request cancelled by its client is not a broken lookup: nothing logged. +func TestNodeDetail404CancelledLookupIsNotLogged(t *testing.T) { + srv, _ := setupTestServer(t) + var buf bytes.Buffer + prev := log.Writer() + log.SetOutput(&buf) + defer log.SetOutput(prev) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + req := httptest.NewRequest("GET", "/api/nodes/"+issue199Unknown, nil).WithContext(ctx) + w := httptest.NewRecorder() + srv.writeNodeNotFound(w, req, issue199Unknown) + if w.Code != 404 { + t.Fatalf("status=%d, want 404", w.Code) + } + if strings.Contains(buf.String(), "missing-node lookup failed") { + t.Errorf("cancelled request logged as a lookup failure: %s", buf.String()) + } +} From d1cfe9d168fc9d909be21f1fe9c68d25c3770746 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:20:33 +0000 Subject: [PATCH 02/15] fix(server): log failed missing-node lookups, rate-limited (#208) writeNodeNotFound still answers the bare 404 when lookupMissingNode fails, but now logs the error: the first failure at once, later ones at most every 10 minutes with the number suppressed in between (the log-first-then-interval shape of schema_wait.go). A request its client cancelled is not logged, and the requested key stays out of the line. Read-only: no new query, no new write. Co-Authored-By: Claude Opus 5.5 --- cmd/server/issue199_node_not_found_test.go | 27 ++++++++++++++++ cmd/server/node_not_found.go | 37 ++++++++++++++++++++++ cmd/server/routes.go | 3 ++ 3 files changed, 67 insertions(+) diff --git a/cmd/server/issue199_node_not_found_test.go b/cmd/server/issue199_node_not_found_test.go index 3460fc8e7..b3c83329b 100644 --- a/cmd/server/issue199_node_not_found_test.go +++ b/cmd/server/issue199_node_not_found_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/gorilla/mux" ) @@ -209,3 +210,29 @@ func TestNodeDetail404CancelledLookupIsNotLogged(t *testing.T) { t.Errorf("cancelled request logged as a lookup failure: %s", buf.String()) } } + +// The throttle logs the first failure, suppresses the rest inside the +// interval and reports how many it suppressed with the next logged one. +func TestMissingNodeLookupLogThrottle(t *testing.T) { + var l missingNodeLookupLog + t0 := time.Date(2026, 10, 4, 12, 0, 0, 0, time.UTC) + steps := []struct { + at time.Duration + log bool + wantSuppressed int + }{ + {0, true, 0}, + {time.Second, false, 0}, + {missingNodeLogEvery - time.Second, false, 0}, + {missingNodeLogEvery, true, 2}, + {missingNodeLogEvery + time.Minute, false, 0}, + {3 * missingNodeLogEvery, true, 1}, + {5 * missingNodeLogEvery, true, 0}, + } + for i, s := range steps { + logIt, suppressed := l.note(t0.Add(s.at)) + if logIt != s.log || suppressed != s.wantSuppressed { + t.Errorf("step %d (+%v): note()=(%v, %d), want (%v, %d)", i, s.at, logIt, suppressed, s.log, s.wantSuppressed) + } + } +} diff --git a/cmd/server/node_not_found.go b/cmd/server/node_not_found.go index 22b3657c9..e9ba5a17e 100644 --- a/cmd/server/node_not_found.go +++ b/cmd/server/node_not_found.go @@ -8,6 +8,8 @@ import ( "log" "net/http" "strings" + "sync" + "time" ) // nodeNotFoundResponse is the 404 body of GET /api/nodes/{pubkey} (#199). @@ -93,12 +95,47 @@ func (s *Server) lookupMissingNode(ctx context.Context, pubkey string) (nodeNotF return resp, nil } +// missingNodeLogEvery bounds the log of failed missing-node lookups (#208): +// the first failure is logged at once, later ones at most once per interval. +const missingNodeLogEvery = 10 * time.Minute + +// missingNodeLookupLog throttles that log. The zero value is ready. +type missingNodeLookupLog struct { + mu sync.Mutex + last time.Time + suppressed int +} + +// note reports whether a failure at now is logged and, if so, how many +// failures were suppressed since the previous logged one. +func (l *missingNodeLookupLog) note(now time.Time) (bool, int) { + l.mu.Lock() + defer l.mu.Unlock() + if !l.last.IsZero() && now.Sub(l.last) < missingNodeLogEvery { + l.suppressed++ + return false, 0 + } + n := l.suppressed + l.last, l.suppressed = now, 0 + return true, n +} + // writeNodeNotFound answers a node-detail miss. A failed lookup falls back to // the bare 404: it only enriches the error and must not turn it into a 500. +// The failure is logged, throttled, so a broken lookup (e.g. schema drift on +// inactive_nodes) does not pass for a plain miss (#208). A request its client +// cancelled is not a broken lookup and is not logged. The requested key is +// left out of the line: it is client input and adds nothing to the error. func (s *Server) writeNodeNotFound(w http.ResponseWriter, r *http.Request, pubkey string) { resp, err := s.lookupMissingNode(r.Context(), pubkey) if err != nil { resp = nodeNotFoundResponse{Error: "Not found"} + if r.Context().Err() == nil { + if logIt, suppressed := s.missingNodeLog.note(time.Now()); logIt { + log.Printf("[routes] missing-node lookup failed, answering a bare 404: %v (%d more suppressed since the last report; next report in %v at the earliest)", + err, suppressed, missingNodeLogEvery) + } + } } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusNotFound) diff --git a/cmd/server/routes.go b/cmd/server/routes.go index e2ce37ea0..f349355f4 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -58,6 +58,9 @@ type Server struct { // miss moves through handleStats. Nil in production. statsHook func(stage string) + // Throttles the log of failed GET /api/nodes/{pubkey} 404 lookups (#208). + missingNodeLog missingNodeLookupLog + // Shared channel proposals; built lazily from cfg/db (tests may preset). proposals *channelProposalService proposalsOnce sync.Once From 2b1b2916ff34d36e026386ce3414a77955c2696b Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:21:38 +0000 Subject: [PATCH 03/15] test(nodes): assert the missing-node card's "Last advert" row (#208) The last advert date also appears in the explanation sentence, so a card without the "Last advert" row (mutant M8) passed both the unit test and the E2E. Both now read the card's
/
rows and assert the "Last advert" row shows the inactive row's last_seen. Co-Authored-By: Claude Opus 5.5 --- test-issue-199-inactive-observer-e2e.js | 13 +++++++++++++ test-issue-199-missing-node.js | 15 +++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/test-issue-199-inactive-observer-e2e.js b/test-issue-199-inactive-observer-e2e.js index b606154a7..2428ea30d 100644 --- a/test-issue-199-inactive-observer-e2e.js +++ b/test-issue-199-inactive-observer-e2e.js @@ -17,6 +17,7 @@ const { chromium } = require('playwright'); const BASE = process.env.BASE_URL || 'http://localhost:13581'; const INACTIVE_OBS = '0D3B3F382173A49EB0E3BB01AB2EA9B28601D9039E4BD52C55B91A8000CC092D'; const OBS_ONLY = '424419FDF9DD9D206A5A917979E56E843DE78E890359C70B9F59B7E2CF2CE392'; +let inactiveLastSeen = null; // inactive_nodes.last_seen, from the precondition step let passed = 0, failed = 0; async function step(name, fn) { @@ -37,6 +38,12 @@ async function settledNodePage(page) { title: title ? title.textContent.trim() : '', text: body ? body.textContent : '', hrefs: body ? Array.from(body.querySelectorAll('a[href]')).map(a => a.getAttribute('href')) : [], + // The card's
/
rows, label → text (#208 item 2). + rows: body ? Array.from(body.querySelectorAll('dt')).reduce((o, dt) => { + const dd = dt.nextElementSibling; + o[dt.textContent.trim()] = dd && dd.tagName === 'DD' ? dd.textContent.trim() : null; + return o; + }, {}) : {}, }; }); } @@ -61,6 +68,7 @@ async function settledNodePage(page) { assert(body.inactive_node && body.inactive_node.name === 'Inactive Observer E2E', 'inactive_node missing (seed-199 applied?): ' + JSON.stringify(body)); assert(body.observer && body.observer.id === INACTIVE_OBS, 'observer missing: ' + JSON.stringify(body)); + inactiveLastSeen = body.inactive_node.last_seen; }); await step('observer detail → "View node detail" explains the inactive node', async () => { @@ -75,6 +83,11 @@ async function settledNodePage(page) { assert(r.text.indexOf('Inactive Observer E2E') !== -1, 'inactive name missing'); assert(/repeater/i.test(r.text), 'inactive role missing'); assert(r.title.indexOf('Inactive Observer E2E') !== -1, 'title should name the device: ' + r.title); + // #208 item 2: the row itself, with the inactive row's last advert date. + const lastAdvert = r.rows['Last advert']; + assert(lastAdvert, '"Last advert" row missing: ' + JSON.stringify(r.rows)); + const want = await page.evaluate((iso) => formatAbsoluteTimestamp(iso), inactiveLastSeen); + assert(lastAdvert === want, '"Last advert" row shows ' + JSON.stringify(lastAdvert) + ', want ' + JSON.stringify(want)); assert(r.hrefs.indexOf('#/observers/' + encodeURIComponent(INACTIVE_OBS)) !== -1, 'observer link missing: ' + r.hrefs); assert(r.hrefs.indexOf('#/nodes') !== -1, 'Back to Nodes link missing'); }); diff --git a/test-issue-199-missing-node.js b/test-issue-199-missing-node.js index 412e57b6a..a3d6ad765 100644 --- a/test-issue-199-missing-node.js +++ b/test-issue-199-missing-node.js @@ -139,6 +139,21 @@ const NOT_FOUND = { assert.ok(!/Node not found/.test(v.html), 'must not dead-end on "Node not found"'); }); + // #208 item 2: the same date is also in the explanation sentence, so the + // row itself is asserted, as a
/
pair, not just the date text. + await test('inactive node: the "Last advert" row carries the last advert date', async () => { + const v = view(PK, NOT_FOUND); + const pairs = {}; + const re = /
([^<]*)<\/dt>]*>([\s\S]*?)<\/dd>/g; + let m; + while ((m = re.exec(v.html))) pairs[m[1]] = m[2]; + assert.ok('Last advert' in pairs, 'no "Last advert" row; rows: ' + Object.keys(pairs).join(', ')); + assert.ok(pairs['Last advert'].indexOf(ctx.formatAbsoluteTimestamp('2026-09-24T15:35:00Z')) !== -1, + '"Last advert" row does not show the inactive last_seen: ' + pairs['Last advert']); + assert.ok(pairs['Last upload as observer'].indexOf(ctx.formatAbsoluteTimestamp('2026-10-04T04:23:00Z')) !== -1, + '"Last upload as observer" row does not show the observer last_seen'); + }); + await test('inactive observer: links to its observer page and back to Nodes', async () => { const v = view(PK, NOT_FOUND); assert.ok(v.html.indexOf('href="#/observers/' + encodeURIComponent(OBS_ID) + '"') !== -1, 'observer link missing'); From fa639baf5f350460cef3e37327cefbbd412fa881 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:21:54 +0000 Subject: [PATCH 04/15] test(nodes): the inactive card must not claim the device is inactive (#208) An observer can be uploading right now while its node row is still in inactive_nodes (retired before #203, or while the observer was quiet), so "this device is inactive" can contradict the "Last upload as observer" row next to it. Pin the wording to the record instead. Co-Authored-By: Claude Opus 5.5 --- test-issue-199-inactive-observer-e2e.js | 7 ++++--- test-issue-199-missing-node.js | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/test-issue-199-inactive-observer-e2e.js b/test-issue-199-inactive-observer-e2e.js index 2428ea30d..c1893349e 100644 --- a/test-issue-199-inactive-observer-e2e.js +++ b/test-issue-199-inactive-observer-e2e.js @@ -4,8 +4,8 @@ * * Needs test-fixtures/seed-199-inactive-observer.sql applied to the fixture: * - observer 0D3B3F38... has its node row only in inactive_nodes, so the - * node page explains "No advert heard since ; this device is - * inactive" with the inactive row's name and role; + * node page explains "No advert heard since ; this node is listed + * as inactive" with the inactive row's name and role; * - observer 424419FD... has no node record at all, so the node page says * so and links back to the observer. * @@ -79,7 +79,8 @@ async function settledNodePage(page) { await page.waitForFunction(() => location.hash.indexOf('#/nodes/') === 0); const r = await settledNodePage(page); assert(r.text.indexOf('Node not found') === -1, 'dead-ended on "Node not found": ' + r.text.slice(0, 200)); - assert(/No advert heard since .+; this device is inactive/.test(r.text), 'missing inactive explanation: ' + r.text.slice(0, 300)); + assert(/No advert heard since .+; this node is listed as inactive\./.test(r.text), 'missing inactive explanation: ' + r.text.slice(0, 300)); + assert(r.text.indexOf('this device is inactive') === -1, 'card claims the device is inactive (#208)'); assert(r.text.indexOf('Inactive Observer E2E') !== -1, 'inactive name missing'); assert(/repeater/i.test(r.text), 'inactive role missing'); assert(r.title.indexOf('Inactive Observer E2E') !== -1, 'title should name the device: ' + r.title); diff --git a/test-issue-199-missing-node.js b/test-issue-199-missing-node.js index a3d6ad765..127dc1e9a 100644 --- a/test-issue-199-missing-node.js +++ b/test-issue-199-missing-node.js @@ -131,7 +131,10 @@ const NOT_FOUND = { const v = view(PK, NOT_FOUND); assert.ok(v, 'expected a view for an inactive node'); assert.ok(/No advert heard since/.test(v.html), 'missing "No advert heard since"'); - assert.ok(/this device is inactive/.test(v.html), 'missing "this device is inactive"'); + // #208 item 3: the card states the record, not the device: an observer + // can be uploading right now while its node row is still inactive. + assert.ok(/No advert heard since .+; this node is listed as inactive\./.test(v.html), 'missing "this node is listed as inactive"'); + assert.ok(!/this device is inactive/.test(v.html), 'must not claim the device is inactive'); assert.ok(v.html.indexOf(ctx.formatAbsoluteTimestamp('2026-09-24T15:35:00Z')) !== -1, 'last advert date not shown'); assert.ok(v.html.indexOf('Quiet Repeater') !== -1, 'inactive name missing'); assert.ok(/repeater/i.test(v.html.replace('Quiet Repeater', '')), 'role missing'); @@ -165,7 +168,7 @@ const NOT_FOUND = { assert.ok(v, 'expected a view for an observer-only device'); assert.ok(/no node record/i.test(v.html), 'missing "no node record" explanation'); assert.ok(/no advert from it has been heard/i.test(v.html), 'explanation must say why: no advert heard'); - assert.ok(!/this device is inactive/.test(v.html), 'observer-only must not claim an inactive row'); + assert.ok(!/inactive/.test(v.html), 'observer-only must not claim an inactive row'); assert.ok(v.html.indexOf('Quiet Observer') !== -1, 'observer name missing'); assert.ok(v.html.indexOf('href="#/observers/' + encodeURIComponent(OBS_ID) + '"') !== -1, 'observer link missing'); }); From ee7a9032ab24d1d87017e1dc237a422ce6ba73c8 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:21:54 +0000 Subject: [PATCH 05/15] fix(nodes): word the inactive card as a listing, not a device state (#208) "No advert heard since ; this device is inactive" becomes "...; this node is listed as inactive", which stays true while the observer of the same key is uploading. The observer's current status is already on the card as the "Last upload as observer" row. Co-Authored-By: Claude Opus 5.5 --- public/nodes.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/public/nodes.js b/public/nodes.js index 2e1aa4c4e..516220da8 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -713,7 +713,9 @@ let headline, explanation; if (inactive) { headline = 'Inactive node'; - explanation = 'No advert heard since ' + when(inactive.last_seen) + '; this device is inactive. ' + + // The record, not the device (#208): an observer can be uploading now + // while its node row is still in inactive_nodes. + explanation = 'No advert heard since ' + when(inactive.last_seen) + '; this node is listed as inactive. ' + 'Nodes without an advert inside the retention window are moved off the node list until they advertise again.'; rows.push(['Name', escapeHtml(inactive.name || '—')], ['Role', escapeHtml(inactive.role || '—')], ['Last advert', when(inactive.last_seen)]); } else { From 0c0cadf0ec76059637c17776ccfc85fd87cae8ee Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:22:31 +0000 Subject: [PATCH 06/15] test(packets): Escape with focus inside the View Path modal closes it (#208) No test focused an element inside the modal, so dropping the overlay.contains(el) check in focusInLayerAbove (mutant M4) survived: the position:fixed .modal-overlay is then taken for a layer drawn over the modal and Escape on its close button does nothing. Add a unit case and an E2E step for the close and copy-link buttons. Co-Authored-By: Claude Opus 5.5 --- test-issue-180-packets-url-modal-e2e.js | 16 ++++++++++++- test-packet-path-map.js | 30 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/test-issue-180-packets-url-modal-e2e.js b/test-issue-180-packets-url-modal-e2e.js index 280f6365c..5dadbec3f 100644 --- a/test-issue-180-packets-url-modal-e2e.js +++ b/test-issue-180-packets-url-modal-e2e.js @@ -11,7 +11,8 @@ * * - Item 4 (1400 px): with the View Path modal open, Escape in a layer opened * over it (global search via Ctrl+K, the nav More menu) closes that layer, - * not the modal; the next Escape closes the modal. + * not the modal; the next Escape closes the modal. Escape with focus on + * the modal's own close or copy-link button closes it (#208). * * - Item 5 (800 and 1400 px): Back from a packet whose View Path modal is * open closes the modal, and Forward onto that history entry (which still @@ -154,6 +155,19 @@ function detailPaneOpen(page) { await secondEscapeClosesModal(); }); + // #208 item 4: the modal's own controls are not a layer above it. + for (const id of ['packetPathClose', 'packetPathCopyLink']) { + await step(`desktop (1400): Escape with focus inside the View Path modal (#${id}) closes it`, async () => { + await openModal(); + await page.focus('#' + id); + await waitFor(page, (i) => document.activeElement && document.activeElement.id === i, 'focus did not move into the modal', id); + await page.keyboard.press('Escape'); + await page.waitForSelector('#packetPathModal', { state: 'detached', timeout: 5000 }) + .catch(() => { throw new Error('Escape with focus inside the modal did not close it'); }); + await waitFor(page, () => !/viewPath=/.test(location.hash), 'viewPath still in URL after the modal closed'); + }); + } + await step('desktop (1400): Escape in the nav More menu over the View Path modal closes the menu, not the modal', async () => { await openModal(); await page.click('#navMoreBtn'); diff --git a/test-packet-path-map.js b/test-packet-path-map.js index bf2dd515a..7b1b694d8 100644 --- a/test-packet-path-map.js +++ b/test-packet-path-map.js @@ -1408,6 +1408,36 @@ function makeSandbox(apiImpl) { await escapeCase('Escape with focus in the sticky top nav (no floating layer) closes the modal (#180)', [{}, { pos: 'sticky', cls: 'top-nav' }], 'target', true); + // #208 item 4: focus inside the modal itself (its close or copy-link + // button) is not a layer above it. The .modal-overlay is position:fixed and + // topmost at the button, so without the overlay.contains() check the walk + // would take the modal for a layer drawn over it and leave Escape alone. + await (async () => { + const name = 'Escape with focus inside the modal (its close button) closes the modal (#208)'; + try { + const ctx = makeSandbox(() => Promise.reject(new Error('boom'))); + ctx.location.hash = '#/packets/deadbeef?obs=1&viewPath=1'; + await ctx.window.PacketPathMap.open('deadbeef'); + const overlay = ctx.document.getElementById('packetPathModal'); + const btn = ctx.document.getElementById('packetPathClose'); + assert.ok(overlay && btn && overlay.contains(btn), 'close button not inside the modal'); + overlay._pos = 'fixed'; + overlay.parentElement = ctx.document.body; + overlay.matches = () => false; + btn.parentElement = overlay; + btn.getBoundingClientRect = () => ({ left: 10, top: 10, width: 20, height: 20 }); + ctx.__topAt = btn; + const key = ctx.__docLog.find(r => r.type === 'keydown'); + let stopped = 0; + key.fn({ key: 'Escape', target: btn, stopPropagation() { stopped++; } }); + assert.ok(!ctx.document.getElementById('packetPathModal'), 'modal still open'); + assert.strictEqual(stopped, 1, 'stopPropagation calls: ' + stopped); + assert.strictEqual(ctx.location.hash, '#/packets/deadbeef?obs=1'); + passed++; + console.log(' ✅ ' + name); + } catch (e) { failed++; console.log(' ❌ ' + name + ': ' + e.message); } + })(); + // #180: the modal closes on a route change, and Back/Forward onto the // #/packets/?…&viewPath=1 entry it was closed away from does not // reopen it. A new link to the same URL (an entry without that state) From e322ea82106fa56f677ed99e63e82de3df4a3d76 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:22:47 +0000 Subject: [PATCH 07/15] test(analytics): leaving Hash Issues must drop bytes= and section= (#208) TAB_URL_PARAMS is "the hash keys each tab owns", but Hash Issues' bytes= and section= are not in it, so they ride along to the next tab: #/analytics?tab=topology&bytes=2§ion=hashMatrixSection. Co-Authored-By: Claude Opus 5.5 --- test-analytics-subtab-deeplinks-205.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test-analytics-subtab-deeplinks-205.js b/test-analytics-subtab-deeplinks-205.js index 15cd65fc4..1db9482b5 100644 --- a/test-analytics-subtab-deeplinks-205.js +++ b/test-analytics-subtab-deeplinks-205.js @@ -475,6 +475,17 @@ function pageEnv(opts) { assert.strictEqual(env.hash(), '#/analytics?tab=scopes'); }); + // #208 item 7: Hash Issues owns bytes= and section= (the byte-size + // selector and its section links) like Scopes owns sub=/swin=. + await test('switching from Hash Issues to another tab drops bytes= and section=, keeps window=', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=collisions&bytes=2§ion=hashMatrixSection&window=24h'); + assert.strictEqual(env.initError(), null, 'init() threw'); + assert.strictEqual(env.params().bytes, '2', 'precondition: bytes= kept on Hash Issues'); + await env.clickTab('topology'); + assert.strictEqual(env.hash(), '#/analytics?tab=topology&window=24h'); + }); + console.log('\n=== #205: Wardriving window (wdwin=) ==='); for (const w of ['1h', '24h', '7d']) { From 249351f10bf683e15c9919b27c8f1a3c34e735a0 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:22:48 +0000 Subject: [PATCH 08/15] fix(analytics): Hash Issues owns bytes= and section= in TAB_URL_PARAMS (#208) A tab switch away from Hash Issues now drops its byte-size selector (bytes=) and section link (section=) keys, like the other tabs' keys. Hash Issues itself is unchanged: it reads and writes both as before. Co-Authored-By: Claude Opus 5.5 --- public/analytics.js | 1 + 1 file changed, 1 insertion(+) diff --git a/public/analytics.js b/public/analytics.js index fce9f4bc2..273d5d34f 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -353,6 +353,7 @@ // another tab is selected. var TAB_URL_PARAMS = { 'rf-health': ['range', 'observer', 'from', 'to'], + collisions: ['bytes', 'section'], scopes: [SCOPES_SUBTAB.param, SCOPES_WINDOW.param], wardriving: [WARDRIVING_WINDOW.param], }; From 8eb4398a1703a454627b18f97de15bd0e6307780 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:23:38 +0000 Subject: [PATCH 09/15] test(analytics): deep-link the Hash Stats multi-byte adopters filter (#208) The #206 audit missed the All / Confirmed / Suspected / Unknown filter of the Multi-Byte Hash Adopters card. Pin the #194/#205 pattern for it as mbf=: a URL value selects and filters, an unknown or hostile value is All and is dropped from the URL, All keeps the URL as it is, nothing goes to sessionStorage, and leaving the tab drops the key. Co-Authored-By: Claude Opus 5.5 --- test-analytics-subtab-deeplinks-205.js | 69 ++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test-analytics-subtab-deeplinks-205.js b/test-analytics-subtab-deeplinks-205.js index 1db9482b5..bd5c0433a 100644 --- a/test-analytics-subtab-deeplinks-205.js +++ b/test-analytics-subtab-deeplinks-205.js @@ -228,6 +228,7 @@ function pageEnv(opts) { }, activeScopesWindows: () => activeOf('data-win'), activeWardrivingWindows: () => activeOf('data-wdwin'), + activeMbFilters: () => activeOf('data-mb-filter'), clickSubtab: async (key) => { const bar = content().querySelector('#scopesSubtabs'); assert.ok(bar, 'no #scopesSubtabs'); @@ -254,6 +255,7 @@ function pageEnv(opts) { globalWindow: () => el('analyticsTimeWindow').value, content: () => content().innerHTML, resolveViewParam: ctx._analyticsResolveViewParam, + renderMultiByteAdopters: ctx._analyticsRenderMultiByteAdopters, }; } @@ -541,6 +543,73 @@ function pageEnv(opts) { assert.strictEqual(env.hash(), '#/analytics?tab=wardriving'); }); + // #208 item 6: the Hash Stats multi-byte adopters filter (All / Confirmed + // / Suspected / Unknown) is deep-linked as mbf=. URL only: it had no + // stored state before, so a plain visit still opens on All. + console.log('\n=== #208: Hash Stats multi-byte adopters filter (mbf=) ==='); + + const ADOPTERS = REAL.hashData.multiByteNodes.map((n) => n.name); + await test('precondition: the fixture has confirmed multi-byte adopters only', async () => { + assert.ok(ADOPTERS.length > 0, 'no multiByteNodes in the fixture'); + assert.ok(REAL.hashData.multiByteCapability.every((c) => c.status === 'confirmed'), 'fixture statuses changed'); + }); + + for (const f of ['all', 'confirmed', 'suspected', 'unknown']) { + await test('#/analytics?tab=hashsizes&mbf=' + f + ' selects ' + f + ' and filters the table', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=hashsizes&mbf=' + f); + assert.strictEqual(env.initError(), null, 'init() threw'); + assert.deepStrictEqual(env.activeMbFilters(), [f], 'active filter button'); + const shown = f === 'all' || f === 'confirmed'; + for (const name of ADOPTERS) assert.strictEqual(env.content().indexOf('' + name + '') >= 0, shown, name + (shown ? ' missing' : ' shown')); + assert.strictEqual(env.content().indexOf('No adopters match this filter.') >= 0, !shown, 'empty-filter message'); + assert.strictEqual(env.hash(), f === 'all' ? '#/analytics?tab=hashsizes' : '#/analytics?tab=hashsizes&mbf=' + f, 'URL not canonical'); + }); + } + + for (const value of ['', 'x"]', 'x"],[data-mb-filter="unknown', '__proto__', 'constructor', 'Confirmed', ' confirmed', '']) { + await test('?mbf=' + JSON.stringify(value) + ' falls back to All, no exception', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=hashsizes&mbf=' + encodeURIComponent(value)); + assert.strictEqual(env.initError(), null, 'init() threw'); + assert.deepStrictEqual(env.activeMbFilters(), ['all']); + assert.strictEqual(env.hash(), '#/analytics?tab=hashsizes'); + assert.ok(env.content().indexOf('onerror') < 0, 'URL value reached the markup'); + }); + } + + await test('#/analytics?tab=hashsizes is left as it is and opens on All', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=hashsizes'); + assert.deepStrictEqual(env.activeMbFilters(), ['all']); + assert.deepStrictEqual(env.hashLog.filter((h) => h !== '#/analytics?tab=hashsizes'), [], 'URL rewritten'); + }); + + await test('mbf= is URL only: nothing stored, a later plain visit opens on All', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=hashsizes&mbf=confirmed'); + assert.deepStrictEqual(Object.keys(env.session), [], 'sessionStorage written: ' + JSON.stringify(env.session)); + env.destroy(); + await env.mount('#/analytics?tab=hashsizes'); + assert.deepStrictEqual(env.activeMbFilters(), ['all']); + }); + + await test('switching from Hash Stats to another tab drops mbf=', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=hashsizes&mbf=confirmed&window=24h'); + await env.clickTab('topology'); + assert.strictEqual(env.hash(), '#/analytics?tab=topology&window=24h'); + }); + + await test('renderMultiByteAdopters(nodes, caps, filter) marks the filter active; an unknown one is All', async () => { + const env = pageEnv(); + const nodes = REAL.hashData.multiByteNodes, caps = REAL.hashData.multiByteCapability; + const active = (html) => (html.match(/' + - '' + - '' + - '' + + '' + + '' + + '' + + '' + '' + '' + - '
' + buildTableContent(rows, 'all') + '
' + + '
' + buildTableContent(rows, initialFilter) + '
' + ''; // Use setTimeout for event delegation on the stable section container setTimeout(function() { var section = document.getElementById('mbAdoptersSection'); if (!section) return; - var currentFilter = 'all'; + var currentFilter = initialFilter; section.addEventListener('click', function handler(e) { var btn = e.target.closest('[data-mb-filter]'); @@ -1653,6 +1664,7 @@ // Replace only the table content, not the whole section var wrap = section.querySelector('#mbAdoptersTableWrap'); if (wrap) wrap.innerHTML = buildTableContent(rows, currentFilter); + setViewParam(HASHSTATS_MB_FILTER, currentFilter); return; } var th = e.target.closest('[data-sort]'); diff --git a/test-analytics-subtab-deeplinks-205.js b/test-analytics-subtab-deeplinks-205.js index bd5c0433a..3138cb7f1 100644 --- a/test-analytics-subtab-deeplinks-205.js +++ b/test-analytics-subtab-deeplinks-205.js @@ -548,10 +548,13 @@ function pageEnv(opts) { // stored state before, so a plain visit still opens on All. console.log('\n=== #208: Hash Stats multi-byte adopters filter (mbf=) ==='); - const ADOPTERS = REAL.hashData.multiByteNodes.map((n) => n.name); - await test('precondition: the fixture has confirmed multi-byte adopters only', async () => { + // Each adopter's status as the card derives it: its capability row by + // pubkey, else unknown. + const capStatus = {}; + REAL.hashData.multiByteCapability.forEach((c) => { capStatus[c.pubkey] = c.status; }); + const ADOPTERS = REAL.hashData.multiByteNodes.map((n) => ({ name: n.name, status: capStatus[n.pubkey] || 'unknown' })); + await test('precondition: the fixture has multi-byte adopters', async () => { assert.ok(ADOPTERS.length > 0, 'no multiByteNodes in the fixture'); - assert.ok(REAL.hashData.multiByteCapability.every((c) => c.status === 'confirmed'), 'fixture statuses changed'); }); for (const f of ['all', 'confirmed', 'suspected', 'unknown']) { @@ -560,9 +563,12 @@ function pageEnv(opts) { await env.mount('#/analytics?tab=hashsizes&mbf=' + f); assert.strictEqual(env.initError(), null, 'init() threw'); assert.deepStrictEqual(env.activeMbFilters(), [f], 'active filter button'); - const shown = f === 'all' || f === 'confirmed'; - for (const name of ADOPTERS) assert.strictEqual(env.content().indexOf('' + name + '') >= 0, shown, name + (shown ? ' missing' : ' shown')); - assert.strictEqual(env.content().indexOf('No adopters match this filter.') >= 0, !shown, 'empty-filter message'); + for (const a of ADOPTERS) { + const shown = f === 'all' || a.status === f; + assert.strictEqual(env.content().indexOf('' + a.name + '') >= 0, shown, a.name + ' (' + a.status + ')' + (shown ? ' missing' : ' shown')); + } + const none = !ADOPTERS.some((a) => f === 'all' || a.status === f); + assert.strictEqual(env.content().indexOf('No adopters match this filter.') >= 0, none, 'empty-filter message'); assert.strictEqual(env.hash(), f === 'all' ? '#/analytics?tab=hashsizes' : '#/analytics?tab=hashsizes&mbf=' + f, 'URL not canonical'); }); } From b992cfafc357c53f3137fa237c4e9d9abb8afe69 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:26:31 +0000 Subject: [PATCH 11/15] test(analytics): E2E for the Hash Stats mbf= deep link (#208) In Chromium against the fixture server: a cold load of ?tab=hashsizes&mbf=confirmed selects Confirmed; a click writes mbf=, a reload keeps it, All drops it and nothing is stored; a tab switch drops it; a hostile value is All without a page error. The click path is not reachable from the vm unit test (the card wires its handler in a setTimeout), so this is what kills a click that does not write the URL. Co-Authored-By: Claude Opus 5.5 --- ...ssue-205-analytics-subtab-deeplinks-e2e.js | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/test-issue-205-analytics-subtab-deeplinks-e2e.js b/test-issue-205-analytics-subtab-deeplinks-e2e.js index 03f14634a..6749bed4e 100644 --- a/test-issue-205-analytics-subtab-deeplinks-e2e.js +++ b/test-issue-205-analytics-subtab-deeplinks-e2e.js @@ -9,7 +9,8 @@ * - a URL value wins over the sessionStorage one; * - a hostile ?sub= falls back to Overview without a page error; * - the default view keeps the URL it had before (#/analytics?tab=scopes), - * and switching to another tab drops the keys. + * and switching to another tab drops the keys; + * - Hash Stats' multi-byte adopters filter is deep-linked as ?mbf= (#208). * * Usage: BASE_URL=http://localhost:13581 node test-issue-205-analytics-subtab-deeplinks-e2e.js */ @@ -192,6 +193,61 @@ async function coldLoad(page, path) { assert(await hash(page) === '#/analytics?tab=wardriving&wdwin=1h', 'after 1h: ' + await hash(page)); }); + // #208 item 6: Hash Stats' multi-byte adopters filter as mbf=. + const mbActive = (p) => p.evaluate(() => Array.from(document.querySelectorAll('#mbCapFilters [data-mb-filter].active')).map((b) => b.dataset.mbFilter)); + async function expectMb(p, f) { + try { + await p.waitForFunction((want) => { + const a = Array.from(document.querySelectorAll('#mbCapFilters [data-mb-filter].active')); + return a.length === 1 && a[0].dataset.mbFilter === want; + }, f, { timeout: 15000 }); + } catch (_) { + throw new Error('filter ' + f + ' not active; active ' + JSON.stringify(await mbActive(p)) + ', hash ' + await hash(p)); + } + } + // The card wires its click handler 100 ms after it renders. + async function clickMb(p, f) { + await p.waitForTimeout(300); + await p.click('#mbCapFilters [data-mb-filter="' + f + '"]'); + await expectMb(p, f); + } + + await step('cold load #/analytics?tab=hashsizes&mbf=confirmed selects Confirmed, URL unchanged', async () => { + await coldLoad(page, '#/analytics?tab=hashsizes&mbf=confirmed'); + await expectMb(page, 'confirmed'); + assert(await page.locator('#mbAdoptersTable tbody tr').count() > 0, 'no confirmed adopters in the table (fixture?)'); + assert(await hash(page) === '#/analytics?tab=hashsizes&mbf=confirmed', 'hash ' + await hash(page)); + }); + + await step('clicking a filter writes mbf=, reload keeps it, All drops it', async () => { + await clickMb(page, 'unknown'); + await page.waitForFunction(() => location.hash === '#/analytics?tab=hashsizes&mbf=unknown', null, { timeout: 5000 }) + .catch(async () => { throw new Error('after Unknown: ' + await hash(page)); }); + await page.reload({ waitUntil: 'load' }); + await page.waitForFunction(() => window.__themeRefreshed, null, { timeout: 8000 }).catch(() => {}); + await expectMb(page, 'unknown'); + await clickMb(page, 'all'); + await page.waitForFunction(() => location.hash === '#/analytics?tab=hashsizes', null, { timeout: 5000 }) + .catch(async () => { throw new Error('after All: ' + await hash(page)); }); + assert(await page.evaluate(() => Object.keys(sessionStorage).filter((k) => /mb/i.test(k)).length === 0), 'filter stored in sessionStorage'); + }); + + await step('switching from Hash Stats to another tab drops mbf=', async () => { + await clickMb(page, 'confirmed'); + await page.waitForFunction(() => location.hash === '#/analytics?tab=hashsizes&mbf=confirmed', null, { timeout: 5000 }); + await page.click('#analyticsTabs [data-tab="topology"]'); + await page.waitForFunction(() => location.hash === '#/analytics?tab=topology', null, { timeout: 5000 }) + .catch(async () => { throw new Error('after the tab switch: ' + await hash(page)); }); + }); + + await step('a hostile ?mbf= falls back to All without a page error', async () => { + const before = pageErrors.length; + await coldLoad(page, '#/analytics?tab=hashsizes&mbf=' + encodeURIComponent('x"],[data-mb-filter="unknown')); + await expectMb(page, 'all'); + assert(await hash(page) === '#/analytics?tab=hashsizes', 'hash not canonical: ' + await hash(page)); + assert(pageErrors.length === before, 'page errors: ' + pageErrors.slice(before).join(' | ')); + }); + await browser.close(); console.log('\n' + passed + ' passed, ' + failed + ' failed'); From d7b5e19db41adf14d6ee02d515e367913acb3e00 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:27:08 +0000 Subject: [PATCH 12/15] test(analytics): Back/Forward to a default-view entry shows its own view (#208) A default view leaves its key out of the URL and a missing key falls back to sessionStorage, so Back to #/analytics?tab=scopes after a later entry stored sub=regions opens Regions, not that entry's Overview (and the same for Wardriving's window). The vm's fake history now keeps a state, and new entries and Back/Forward are modelled. Also pinned: a new entry without the key still opens the stored value, a tab switch inside an entry still brings it back, a garbled entry state never reaches the view, and other history.state keys are kept. Co-Authored-By: Claude Opus 5.5 --- test-analytics-subtab-deeplinks-205.js | 108 ++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/test-analytics-subtab-deeplinks-205.js b/test-analytics-subtab-deeplinks-205.js index 3138cb7f1..b2467d071 100644 --- a/test-analytics-subtab-deeplinks-205.js +++ b/test-analytics-subtab-deeplinks-205.js @@ -169,7 +169,8 @@ function pageEnv(opts) { localStorage: storage(), sessionStorage: storage(opts.session), location: { hash: '#/analytics' }, - history: { replaceState(_s, _t, url) { hashLog.push(url); ctx.location.hash = url; } }, + // state is kept as a structured clone, like the real history.state. + history: { state: null, replaceState(st, _t, url) { ctx.history.state = st == null ? null : JSON.parse(JSON.stringify(st)); hashLog.push(url); ctx.location.hash = url; } }, CustomEvent: class CustomEvent {}, Map, Set, Promise, URLSearchParams, getComputedStyle: () => ({ getPropertyValue: () => '' }), timeAgo: () => 'x ago', initTabBar() {}, makeColumnsResizable() {}, @@ -209,6 +210,21 @@ function pageEnv(opts) { await flush(); }, initError: () => initError, + // History entries (#208): the current one as { hash, state }; a new + // entry (a link, location.hash = …) has no state; Back/Forward brings + // an entry back with the state it had when it was left. + entry: () => ({ hash: ctx.location.hash, state: ctx.history.state == null ? null : JSON.parse(JSON.stringify(ctx.history.state)) }), + async visit(hash) { + page.destroy(); + ctx.history.state = null; + await this.mount(hash); + }, + async traverse(entry) { + page.destroy(); + ctx.history.state = entry.state == null ? null : JSON.parse(JSON.stringify(entry.state)); + await this.mount(entry.hash); + }, + historyState: () => ctx.history.state, destroy: () => page.destroy(), hash: () => ctx.location.hash, params: () => Object.fromEntries(new URLSearchParams(ctx.location.hash.split('?')[1] || '')), @@ -543,6 +559,96 @@ function pageEnv(opts) { assert.strictEqual(env.hash(), '#/analytics?tab=wardriving'); }); + // #208 item 5: a default view leaves its key out of the URL, and a missing + // key falls back to sessionStorage. Back/Forward to an entry whose view + // was the default must restore that default, not the value a later entry + // stored; a new entry without the key still gets the stored value. + console.log('\n=== #208: Back/Forward to an entry in its default view ==='); + + await test('Scopes: Back to a default-view entry shows the default, Forward the later view', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=scopes'); + const e1 = env.entry(); + await env.visit('#/analytics?tab=scopes&sub=regions&swin=7d'); + assert.deepStrictEqual(env.activeSubtabs(), ['regions'], 'precondition'); + const e2 = env.entry(); + await env.traverse(e1); + assert.deepStrictEqual(env.activeSubtabs(), ['overview'], 'Back: sub-tab'); + assert.deepStrictEqual(env.visiblePanels(), ['overview'], 'Back: panel'); + assert.deepStrictEqual(env.activeScopesWindows(), ['24h', '24h'], 'Back: window'); + assert.strictEqual(env.hash(), '#/analytics?tab=scopes', 'Back: URL'); + assert.strictEqual(env.session.scopes_subtab, 'overview', 'Back: stored sub-tab'); + await env.traverse(e2); + assert.deepStrictEqual(env.activeSubtabs(), ['regions'], 'Forward: sub-tab'); + assert.deepStrictEqual(env.activeScopesWindows(), ['7d', '7d'], 'Forward: window'); + }); + + await test('Scopes: an entry clicked back to its default is restored as the default', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=scopes'); + await env.clickSubtab('hygiene'); + await env.clickSubtab('overview'); + const e1 = env.entry(); + await env.visit('#/analytics?tab=scopes&sub=regions'); + await env.traverse(e1); + assert.deepStrictEqual(env.activeSubtabs(), ['overview']); + assert.strictEqual(env.hash(), '#/analytics?tab=scopes'); + }); + + await test('Scopes: a new entry without sub= still opens the stored sub-tab', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=scopes'); + await env.visit('#/analytics?tab=scopes&sub=regions'); + await env.visit('#/analytics?tab=scopes'); + assert.deepStrictEqual(env.activeSubtabs(), ['regions']); + assert.strictEqual(env.hash(), '#/analytics?tab=scopes&sub=regions'); + }); + + await test('Wardriving: Back to a default-view entry shows 24h', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=wardriving'); + const e1 = env.entry(); + await env.visit('#/analytics?tab=wardriving&wdwin=1h'); + assert.deepStrictEqual(env.activeWardrivingWindows(), ['1h'], 'precondition'); + await env.traverse(e1); + assert.deepStrictEqual(env.activeWardrivingWindows(), ['24h']); + assert.strictEqual(env.hash(), '#/analytics?tab=wardriving'); + }); + + await test('an entry from another page (Back from Nodes) keeps its own view', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=scopes'); + const e1 = env.entry(); + await env.visit('#/analytics?tab=scopes&sub=hopdepth'); + await env.visit('#/nodes'); + await env.traverse(e1); + assert.deepStrictEqual(env.activeSubtabs(), ['overview']); + }); + + await test('a tab switch inside an entry still brings back the stored view', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=scopes&sub=hopdepth'); + await env.clickTab('topology'); + await env.clickTab('scopes'); + assert.deepStrictEqual(env.activeSubtabs(), ['hopdepth']); + }); + + for (const st of [{ analyticsView: { sub: 'x"]', swin: '__proto__' } }, { analyticsView: 'hopdepth' }, { analyticsView: { sub: 7 } }, 'junk', 42]) { + await test('a foreign or garbled entry state ' + JSON.stringify(st) + ' never reaches the view, no exception', async () => { + const env = pageEnv({ session: { scopes_subtab: 'regions' } }); + await env.traverse({ hash: '#/analytics?tab=scopes', state: st }); + assert.strictEqual(env.initError(), null, 'init() threw'); + const sub = env.activeSubtabs(); + assert.ok(sub.length === 1 && ['overview', 'regions'].includes(sub[0]), 'sub-tab ' + JSON.stringify(sub)); + }); + } + + await test('other keys in history.state are kept', async () => { + const env = pageEnv(); + await env.traverse({ hash: '#/analytics?tab=scopes&sub=regions', state: { other: 'kept' } }); + assert.strictEqual(env.historyState() && env.historyState().other, 'kept', JSON.stringify(env.historyState())); + }); + // #208 item 6: the Hash Stats multi-byte adopters filter (All / Confirmed // / Suspected / Unknown) is deep-linked as mbf=. URL only: it had no // stored state before, so a plain visit still opens on All. From 0022501bfb2799393c631bec26718c0f4c27951a Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 08:30:41 +0000 Subject: [PATCH 13/15] fix(analytics): Back/Forward restores a default view, not the stored one (#208) restoreViewParams leaves a default out of the URL and fell back to sessionStorage for a missing key, so Back to #/analytics?tab=scopes after a later entry stored sub=regions opened Regions and rewrote that entry's URL. _writeViewParams now also records the resolved values in the entry's history.state (analyticsView, other state keys kept), and restoreViewParams uses that record before sessionStorage when the URL has no value. A new entry has no record, so a plain visit still opens the stored view; a tab switch writes a null state, so clicking a tab back in the same entry also still does. URLs are unchanged, and replaceState only runs when the URL or the record changes. E2E: real Back/Forward in Chromium for Scopes and Wardriving; Regions has no window buttons, and expectScopes now waits for a given window. Co-Authored-By: Claude Opus 5.5 --- public/analytics.js | 44 +++++++++++++- ...ssue-205-analytics-subtab-deeplinks-e2e.js | 58 +++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index 60974426e..8c6ce99f5 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -375,23 +375,58 @@ try { return typeof sessionStorage !== 'undefined' ? sessionStorage.getItem(key) : null; } catch (e) { return null; } } + // #208 — the view each history entry showed is recorded in its + // history.state, so Back/Forward to an entry whose view was a default (its + // key left out of the URL) restores that default, not the value a later + // entry stored. A new entry (a link, location.hash = …) has no record and + // still gets the stored value. Only specs with a storageKey are recorded: + // for the others a missing key already means the default. A tab switch + // writes a null state (_updateAnalyticsUrl), so a tab clicked back within + // the same entry also gets the stored value, as before. + var ENTRY_VIEW_KEY = 'analyticsView'; + + // The current entry's record, as { param: value } with string values only. + function _entryView() { + var out = {}; + try { + var st = typeof history !== 'undefined' ? history.state : null; + var v = st && typeof st === 'object' ? st[ENTRY_VIEW_KEY] : null; + if (!v || typeof v !== 'object') return null; + Object.keys(v).forEach(function (k) { if (typeof v[k] === 'string') out[k] = v[k]; }); + } catch (e) { return null; } + return out; + } + + // history.state with the record replaced; other keys are kept. + function _stateWithEntryView(view) { + var out = {}; + var st = typeof history !== 'undefined' ? history.state : null; + if (st && typeof st === 'object') Object.keys(st).forEach(function (k) { out[k] = st[k]; }); + out[ENTRY_VIEW_KEY] = view; + return out; + } + // Stores the values and writes them to the hash in one go. A default is // left out, so a tab in its default view keeps the URL it had before #205. // A spec without a storageKey lives in the URL only. function _writeViewParams(specs, values) { var updates = {}; + var view = _entryView() || {}; + var viewChanged = false; specs.forEach(function (spec, i) { if (spec.storageKey) { try { if (typeof sessionStorage !== 'undefined') sessionStorage.setItem(spec.storageKey, values[i]); } catch (e) { /* storage blocked */ } + if (view[spec.param] !== values[i]) { view[spec.param] = values[i]; viewChanged = true; } } updates[spec.param] = values[i] === spec.dflt ? '' : values[i]; }); if (!window.URLState) return; // replaceState can throw (Safari throttles it); the view has already // changed by then, so a failed URL sync must not break the tab (#1914). + // It only runs when the URL or the entry's record changes. try { var newHash = URLState.updateHashParams(updates, location.hash); - if (newHash !== location.hash) history.replaceState(null, '', newHash); + if (newHash !== location.hash || viewChanged) history.replaceState(_stateWithEntryView(view), '', newHash); } catch (e) { /* URL sync is best effort */ } } @@ -400,11 +435,16 @@ // Read on render: resolve every value of the tab from the same hash first, // then store them and write them back. Writing one value rebuilds the // hash, which drops an empty key ("?sub=") the next read would still see. + // Without a URL value, the entry's own record (#208) comes before the + // stored value. function restoreViewParams(specs) { var hash = typeof location !== 'undefined' ? String(location.hash || '') : ''; var params = new URLSearchParams(hash.split('?')[1] || ''); + var entry = _entryView(); var values = specs.map(function (spec) { - return resolveViewParam(params.get(spec.param), spec.storageKey ? _sessionGet(spec.storageKey) : null, spec.allowed, spec.dflt); + var fallback = null; + if (spec.storageKey) fallback = entry && Object.prototype.hasOwnProperty.call(entry, spec.param) ? entry[spec.param] : _sessionGet(spec.storageKey); + return resolveViewParam(params.get(spec.param), fallback, spec.allowed, spec.dflt); }); _writeViewParams(specs, values); return values; diff --git a/test-issue-205-analytics-subtab-deeplinks-e2e.js b/test-issue-205-analytics-subtab-deeplinks-e2e.js index 6749bed4e..d1030029f 100644 --- a/test-issue-205-analytics-subtab-deeplinks-e2e.js +++ b/test-issue-205-analytics-subtab-deeplinks-e2e.js @@ -10,6 +10,7 @@ * - a hostile ?sub= falls back to Overview without a page error; * - the default view keeps the URL it had before (#/analytics?tab=scopes), * and switching to another tab drops the keys; + * - Back/Forward to an entry in its default view shows that view (#208); * - Hash Stats' multi-byte adopters filter is deep-linked as ?mbf= (#208). * * Usage: BASE_URL=http://localhost:13581 node test-issue-205-analytics-subtab-deeplinks-e2e.js @@ -58,6 +59,13 @@ async function waitScopes(page, sub) { async function expectScopes(page, sub, win) { await waitScopes(page, sub); + // The window buttons render with the panel's data, after the sub-tab. + if (win) { + await page.waitForFunction((w) => { + const a = Array.from(document.querySelectorAll('[id^="scopes-panel-"] [data-win].active')).filter((b) => b.offsetParent !== null); + return a.length === 1 && a[0].dataset.win === w; + }, win, { timeout: 8000 }).catch(() => {}); + } const v = await scopesView(page); assert(JSON.stringify(v.active) === JSON.stringify([sub]), 'active sub-tab ' + JSON.stringify(v.active)); assert(JSON.stringify(v.visible) === JSON.stringify([sub]), 'visible panel ' + JSON.stringify(v.visible)); @@ -193,6 +201,56 @@ async function coldLoad(page, path) { assert(await hash(page) === '#/analytics?tab=wardriving&wdwin=1h', 'after 1h: ' + await hash(page)); }); + // #208 item 5: Back/Forward to an entry whose view was the default (no + // key in its URL) shows that default, not the value a later entry stored. + await step('Back to a default-view Scopes entry shows Overview, Forward the later Regions', async () => { + const ctx2 = await browser.newContext({ viewport: { width: 1400, height: 1000 } }); + await ctx2.addInitScript(() => { + window.addEventListener('theme-refresh', () => { window.__themeRefreshed = true; }, { once: true }); + }); + const p2 = await ctx2.newPage(); + p2.on('pageerror', (e) => { pageErrors.push(e.message); console.error('[pageerror]', e.message); }); + try { + await p2.goto(BASE + '/#/analytics?tab=scopes', { waitUntil: 'load' }); + await p2.waitForFunction(() => window.__themeRefreshed, null, { timeout: 8000 }).catch(() => {}); + await expectScopes(p2, 'overview', '24h'); + await p2.evaluate(() => { location.hash = '#/analytics?tab=scopes&sub=regions&swin=7d'; }); + await expectScopes(p2, 'regions'); + assert(await p2.evaluate(() => sessionStorage.getItem('scopes_subtab')) === 'regions', 'precondition: Regions stored'); + await p2.goBack(); + await p2.waitForFunction(() => location.hash.indexOf('sub=regions') < 0, null, { timeout: 5000 }).catch(() => {}); + await expectScopes(p2, 'overview', '24h'); + assert(await hash(p2) === '#/analytics?tab=scopes', 'after Back: ' + await hash(p2)); + await p2.goForward(); + await expectScopes(p2, 'regions'); // Regions has no window buttons + assert(await hash(p2) === '#/analytics?tab=scopes&sub=regions&swin=7d', 'after Forward: ' + await hash(p2)); + // A new entry without sub= still opens the stored sub-tab. + await p2.evaluate(() => { location.hash = '#/nodes'; }); + await p2.waitForFunction(() => !document.getElementById('scopesSubtabs')); + await p2.evaluate(() => { location.hash = '#/analytics?tab=scopes'; }); + await expectScopes(p2, 'regions'); + } finally { + await ctx2.close(); + } + }); + + await step('Back to a default-view Wardriving entry shows 24h', async () => { + const ctx2 = await browser.newContext({ viewport: { width: 1400, height: 1000 } }); + const p2 = await ctx2.newPage(); + try { + await p2.goto(BASE + '/#/analytics?tab=wardriving', { waitUntil: 'load' }); + await p2.waitForSelector('[data-wdwin="24h"].active'); + await p2.evaluate(() => { location.hash = '#/analytics?tab=wardriving&wdwin=1h'; }); + await p2.waitForSelector('[data-wdwin="1h"].active'); + await p2.goBack(); + await p2.waitForSelector('[data-wdwin="24h"].active', { timeout: 8000 }) + .catch(async () => { throw new Error('24h not active after Back; hash ' + await hash(p2)); }); + assert(await hash(p2) === '#/analytics?tab=wardriving', 'after Back: ' + await hash(p2)); + } finally { + await ctx2.close(); + } + }); + // #208 item 6: Hash Stats' multi-byte adopters filter as mbf=. const mbActive = (p) => p.evaluate(() => Array.from(document.querySelectorAll('#mbCapFilters [data-mb-filter].active')).map((b) => b.dataset.mbFilter)); async function expectMb(p, f) { From defbc59c4b94dfb500acbc48a617e0dd66bee5ab Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 09:56:53 +0000 Subject: [PATCH 14/15] test(analytics): a tab switch keeps Hash Issues' bytes= (#1914), drops section= (#208) CI's #1914 E2E (test-issue-1306-collisions-terminology-e2e.js) pins that Hash Issues -> Hash Stats -> Hash Issues keeps the chosen byte size: bytes= has no stored fallback, so the URL is where the tab remembers it. Dropping it on a tab switch (249351f1) broke that. Only section=, a one-shot scroll anchor, belongs to the tab's dropped keys. Co-Authored-By: Claude Opus 5.5 --- test-analytics-subtab-deeplinks-205.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test-analytics-subtab-deeplinks-205.js b/test-analytics-subtab-deeplinks-205.js index b2467d071..7b29eb67e 100644 --- a/test-analytics-subtab-deeplinks-205.js +++ b/test-analytics-subtab-deeplinks-205.js @@ -493,15 +493,24 @@ function pageEnv(opts) { assert.strictEqual(env.hash(), '#/analytics?tab=scopes'); }); - // #208 item 7: Hash Issues owns bytes= and section= (the byte-size - // selector and its section links) like Scopes owns sub=/swin=. - await test('switching from Hash Issues to another tab drops bytes= and section=, keeps window=', async () => { + // #208 item 7: Hash Issues' section= is a one-shot scroll anchor and is + // dropped when leaving the tab. bytes= is its remembered byte size: it has + // no stored fallback, and #1914 pins that a tab round-trip keeps it. + await test('switching from Hash Issues to another tab drops section=, keeps bytes= and window=', async () => { const env = pageEnv(); await env.mount('#/analytics?tab=collisions&bytes=2§ion=hashMatrixSection&window=24h'); assert.strictEqual(env.initError(), null, 'init() threw'); assert.strictEqual(env.params().bytes, '2', 'precondition: bytes= kept on Hash Issues'); await env.clickTab('topology'); - assert.strictEqual(env.hash(), '#/analytics?tab=topology&window=24h'); + assert.strictEqual(env.hash(), '#/analytics?tab=topology&bytes=2&window=24h'); + }); + + await test('a Hash Issues → Hash Stats → Hash Issues round-trip keeps bytes= (#1914)', async () => { + const env = pageEnv(); + await env.mount('#/analytics?tab=collisions&bytes=2'); + await env.clickTab('hashsizes'); + await env.clickTab('collisions'); + assert.strictEqual(env.params().bytes, '2', 'bytes= lost: ' + env.hash()); }); console.log('\n=== #205: Wardriving window (wdwin=) ==='); From 0f77c97499fe484321ab9b8a6a147109d1afc870 Mon Sep 17 00:00:00 2001 From: dborup Date: Sun, 4 Oct 2026 09:57:12 +0000 Subject: [PATCH 15/15] fix(analytics): only section= is dropped when leaving Hash Issues (#208) Narrows 249351f1: TAB_URL_PARAMS.collisions is ['section']. bytes= stays in the URL across a tab switch, as #1914 intends (no stored fallback), and the comment now says why it is not listed, which was the mismatch the #208 review flagged. Co-Authored-By: Claude Opus 5.5 --- public/analytics.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/public/analytics.js b/public/analytics.js index 8c6ce99f5..405e9c26e 100644 --- a/public/analytics.js +++ b/public/analytics.js @@ -353,10 +353,13 @@ var HASHSTATS_MB_FILTER = { param: 'mbf', allowed: ['all', 'confirmed', 'suspected', 'unknown'], dflt: 'all' }; // The hash keys each tab owns; _updateAnalyticsUrl drops them when - // another tab is selected. + // another tab is selected. Hash Issues' bytes= is deliberately not listed: + // it has no stored fallback, so it stays in the URL across a tab switch + // and a return to Hash Issues keeps the chosen byte size (#1914, #208). + // Its section= is a one-shot scroll anchor and is dropped. var TAB_URL_PARAMS = { 'rf-health': ['range', 'observer', 'from', 'to'], - collisions: ['bytes', 'section'], + collisions: ['section'], hashsizes: [HASHSTATS_MB_FILTER.param], scopes: [SCOPES_SUBTAB.param, SCOPES_WINDOW.param], wardriving: [WARDRIVING_WINDOW.param],