From 1d20c29f28c167dc175d91339ceccd527b8d47bf Mon Sep 17 00:00:00 2001 From: Jonathan Herlin Date: Wed, 2 Sep 2026 14:51:30 +0200 Subject: [PATCH 1/2] fix: use replaceState for traces/roles redirects to preserve back-button history (#1883) fix: use replaceState for traces/roles redirects to preserve back-button history The #/traces/ and #/roles backward-compat redirects used location.hash = ..., which pushes a new history entry instead of replacing the current one. This trapped users navigating back from a trace view: the intermediate #/traces/ entry would immediately re-redirect forward again on hashchange, so back button never reached the packets view they came from. (cherry picked from commit a46a6d55eb7912e1f917af35c33f6de28429e09e) Co-Authored-By: Claude Opus 5 --- public/app.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/public/app.js b/public/app.js index 52bab2ce8..0674d5d1e 100644 --- a/public/app.js +++ b/public/app.js @@ -1170,16 +1170,20 @@ function navigate() { closeNav(); // Backward-compat redirect: #/traces/ → #/tools/trace/ (issue #944). + // Uses replaceState (not location.hash =) so this redirect doesn't add its + // own history entry, otherwise the back button gets stuck bouncing off it. if (location.hash.startsWith('#/traces/')) { - location.hash = location.hash.replace('#/traces/', '#/tools/trace/'); + history.replaceState(null, '', location.hash.replace('#/traces/', '#/tools/trace/')); + navigate(); return; } // Backward-compat redirect: #/roles → #/analytics?tab=roles (issue #1085). // The Roles page was folded into the Analytics tab strip; old links and - // bookmarks must keep working. + // bookmarks must keep working. Uses replaceState for the same reason as above. if (location.hash === '#/roles' || location.hash.startsWith('#/roles?') || location.hash.startsWith('#/roles/')) { - location.hash = '#/analytics?tab=roles'; + history.replaceState(null, '', '#/analytics?tab=roles'); + navigate(); return; } From a6980c74cd8483525b2c43ac85b9d5328f9bbc60 Mon Sep 17 00:00:00 2001 From: Openclaw Date: Wed, 16 Sep 2026 12:18:01 +0200 Subject: [PATCH 2/2] fix(router): replace legacy redirects without history loops (#1883) --- public/app.js | 10 +- test-all.sh | 1 + test-issue-1883-redirect-history.js | 149 ++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 test-issue-1883-redirect-history.js diff --git a/public/app.js b/public/app.js index 46ebd261c..6686b5f18 100644 --- a/public/app.js +++ b/public/app.js @@ -1176,16 +1176,20 @@ function navigate() { closeNav(); // Backward-compat redirect: #/traces/ → #/tools/trace/ (issue #944). + // Uses replaceState (not location.hash =) so this redirect doesn't add its + // own history entry, otherwise the back button gets stuck bouncing off it. if (location.hash.startsWith('#/traces/')) { - location.hash = location.hash.replace('#/traces/', '#/tools/trace/'); + history.replaceState(null, '', location.hash.replace('#/traces/', '#/tools/trace/')); + navigate(); return; } // Backward-compat redirect: #/roles → #/analytics?tab=roles (issue #1085). // The Roles page was folded into the Analytics tab strip; old links and - // bookmarks must keep working. + // bookmarks must keep working. Uses replaceState for the same reason as above. if (location.hash === '#/roles' || location.hash.startsWith('#/roles?') || location.hash.startsWith('#/roles/')) { - location.hash = '#/analytics?tab=roles'; + history.replaceState(null, '', '#/analytics?tab=roles'); + navigate(); return; } diff --git a/test-all.sh b/test-all.sh index b89a1db68..ac14bd10e 100755 --- a/test-all.sh +++ b/test-all.sh @@ -51,6 +51,7 @@ node test-issue-1648-m2-emoji-scan.js node test-issue-1648-m3-emoji-scan.js node test-issue-1648-m6-final-sweep.js node test-issue-1648-m6-lint-self.js +node test-issue-1883-redirect-history.js node test-issue-1890-og-url.js node test-traces.js node test-live-multibyte-filter.js diff --git a/test-issue-1883-redirect-history.js b/test-issue-1883-redirect-history.js new file mode 100644 index 000000000..3d6ea2609 --- /dev/null +++ b/test-issue-1883-redirect-history.js @@ -0,0 +1,149 @@ +/* Regression tests for legacy-route history replacement (#1883). */ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); + +const appPath = process.env.APP_JS || path.join(__dirname, 'public', 'app.js'); + +function makeHistorySandbox() { + const entries = ['#/home']; + let index = 0; + let currentHash = entries[index]; + + const location = {}; + Object.defineProperty(location, 'hash', { + get() { return currentHash; }, + set(hash) { + if (hash === currentHash) return; + entries.splice(index + 1); + entries.push(hash); + index++; + currentHash = hash; + }, + }); + + const history = { + get length() { return entries.length; }, + replaceState(_state, _title, hash) { + entries[index] = hash; + currentHash = hash; + }, + back() { + if (index === 0) return; + index--; + currentHash = entries[index]; + }, + }; + + const classList = { add() {}, remove() {}, toggle() {} }; + const window = { addEventListener() {}, dispatchEvent() {} }; + const document = { + readyState: 'complete', + body: { classList }, + createElement: () => ({ id: '', textContent: '', innerHTML: '' }), + head: { appendChild() {} }, + getElementById: () => null, + addEventListener() {}, + querySelectorAll: () => [], + querySelector: () => null, + }; + window.document = document; + window.location = location; + window.history = history; + + const sandbox = { + window, + document, + location, + history, + console, + Date, + Infinity, + Math, + Array, + Object, + String, + Number, + JSON, + RegExp, + Error, + TypeError, + parseInt, + parseFloat, + isNaN, + isFinite, + encodeURIComponent, + decodeURIComponent, + setTimeout() {}, + clearTimeout() {}, + setInterval() {}, + clearInterval() {}, + fetch: () => Promise.resolve({ json: () => Promise.resolve({}) }), + performance: { now: () => Date.now() }, + localStorage: { getItem: () => null, setItem() {}, removeItem() {} }, + CustomEvent: class CustomEvent {}, + Map, + Promise, + URLSearchParams, + addEventListener() {}, + dispatchEvent() {}, + requestAnimationFrame() {}, + }; + + vm.createContext(sandbox); + vm.runInContext(fs.readFileSync(appPath, 'utf8'), sandbox, { filename: appPath }); + return sandbox; +} + +function isLegacyRedirect(hash) { + return hash.startsWith('#/traces/') || hash === '#/roles' || + hash.startsWith('#/roles?') || hash.startsWith('#/roles/'); +} + +function assertRedirectReplacesHistory(legacyHash, expectedHash) { + const sandbox = makeHistorySandbox(); + const productionNavigate = sandbox.navigate; + let recursiveNavigateCalls = 0; + + // Keep the test focused on the redirect branch: the production redirect + // deliberately re-enters navigate(), while rendering the target route is + // covered by the browser suite. + sandbox.navigate = () => { recursiveNavigateCalls++; }; + + sandbox.location.hash = legacyHash; + const historyLengthBeforeRedirect = sandbox.history.length; + productionNavigate(); + + assert.strictEqual(sandbox.location.hash, expectedHash, + `${legacyHash} should redirect to ${expectedHash}`); + assert.strictEqual(sandbox.history.length, historyLengthBeforeRedirect, + `${legacyHash} must replace its history entry instead of adding one`); + assert.strictEqual(recursiveNavigateCalls, 1, + `${legacyHash} should render the replacement route immediately`); + + sandbox.history.back(); + // Browsers dispatch hashchange after Back. Re-run the production router only + // when Back exposed the legacy entry; the old location.hash implementation + // redirects forward again here and reproduces the loop. + if (isLegacyRedirect(sandbox.location.hash)) productionNavigate(); + + assert.strictEqual(sandbox.location.hash, '#/home', + `Back from ${expectedHash} should return to #/home without a redirect loop`); +} + +const cases = [ + ['#/traces/a1b2c3d4', '#/tools/trace/a1b2c3d4'], + ['#/roles', '#/analytics?tab=roles'], + ['#/roles?from=bookmark', '#/analytics?tab=roles'], + ['#/roles/legacy', '#/analytics?tab=roles'], +]; + +for (const [legacyHash, expectedHash] of cases) { + assertRedirectReplacesHistory(legacyHash, expectedHash); + console.log(` ✅ ${legacyHash} replaces history and Back returns to #/home`); +} + +console.log(`\nredirect history: ${cases.length} passed, 0 failed`);