A navigation defect, not a disclosure one: the screens were blank precisely because nothing had been rendered into them.
The defect
goBack() only unhid its target. A reopened popup renders the wallet list and the one view restoreView() lands on, so every other view is still the blank static template from src/popup/index.html. Stack ["main", "address"] with settings on top, close and reopen, press Back: view-address came up with an empty address line and address-balances showing  . Same for address-token, receive, confirm-tx and transaction.
The fix
The per-view dispatch and its data guards move out of restoreView() into src/popup/viewRouter.js. restoreView() now calls renderView(); nothing about which views restore, or what they check, changed.
goBack() routes a popped view through that same renderView(), via a renderer index.js registers with setBackRenderer() (replacing setRenderMain(), whose only caller was goBack()). Each view module shows itself, so the target is rendered and on screen.
A popped view whose backing state is gone — no selected token, no persisted transaction, no pending transaction, no resumable wait, no valid address — falls back to Home exactly as the restore does.
Rendering happens once per view per page load
The Back path renders only a view this page load has not rendered yet. viewRouter.js keeps a page-load-scoped set of rendered views, written by showView() — the last thing every render path runs, forward navigation and the restore alike, so a view added later registers itself rather than needing to be remembered. makeBackRenderer() declines a view already in that set, leaving goBack() to unhide it, which is what it also does for a view outside RESTORABLE_VIEWS (the restored stack is filtered against that same set (#266), so such a view can only be on the stack from this page load).
That is what keeps a second render from re-fetching and overwriting what a view holds — an unsaved edit in Settings, a transaction list already loaded.
main is the one deliberate exception. Back onto Home re-renders every time, as it did before this router existed: goBack() called the renderWalletList() registered through setRenderMain(), and home.render() already ran loadHomeTxs(). Suppressing it would leave a stale wallet list after a wallet rename or an address removal in Settings, so ALWAYS_RENDER_ON_BACK keeps it net-identical to _renderMain.
The leave handlers that wipe secrets still run exactly once per navigation — every render path ends in showView(), including the fallback — so deleteWallet.js and exportPrivkey.js keep the invariant their comments rely on. src/popup/restorableViews.js and the revealGeneration guard are untouched.
Verification
Demonstrated failing first. With the sources reverted to next and only the tests in place, make test-e2e:
not ok 15 - Back after reopening the popup renders the address screen (#268)
the address line reads "", expected "Wallet 1 — Address 1\n0xD50e9eba912e5bca884850d1F5B3ECA0c54A8591"
not ok 16 - Back after reopening the popup renders the Receive screen (#268)
Receive shows "", expected "0xD50e9eba912e5bca884850d1F5B3ECA0c54A8591"
not ok 17 - Back onto Settings keeps unsaved input (#268)
the unsaved RPC URL reads "https://ethereum-rpc.publicnode.com", expected "https://rpc.example.invalid/unsaved"
That is the reproduction verbatim. tests/backNavigation.test.js could not load at all without src/popup/viewRouter.js, so its failing-first evidence is weaker than the browser run's; the browser run is the one that observes the blank template. Mutation testing shows the unit tests are not vacuous — every guard in the router kills at least one, and the goBack() hook takes 18 with it.
With the fix, at head a9c4080 rebased onto next at c755a5e:
make test-e2e — exit 0, 40/40, including all three new cases.
New tests:
tests/backNavigation.test.js drives the real goBack() with the real router: the reproduction, then address-token, receive, confirm-tx, transaction, success-tx, error-tx, main and an empty stack; each of those with its backing state removed, landing on Home; and the invariants — forward navigation renders nothing by itself, Back onto a live-session view (send) only unhides it, Back onto a view this page load already rendered (including Settings revisited) only unhides it, and Back onto Home renders it anyway.
tests/e2e/run.js adds three cases, in the established style: a real close and reopen of the popup, then Back onto the address screen (asserting the address line and an ETH balance line) and onto Receive (asserting dataset.full, the address on screen, and that the QR canvas has been painted — the blank template carries a fully transparent canvas, so pixels are read rather than the element); plus an in-session Settings → add-token → Back that must preserve unsaved #settings-rpc input, with no reopen involved.
README.md documents the Back-path rendering in the Screen Map section. TODO.md updated in the same commit.
Closes [#268](https://git.eeqj.de/sneak/AutistMask/issues/268).
A navigation defect, not a disclosure one: the screens were blank precisely because nothing had been rendered into them.
## The defect
`goBack()` only unhid its target. A reopened popup renders the wallet list and the one view `restoreView()` lands on, so every other view is still the blank static template from `src/popup/index.html`. Stack `["main", "address"]` with `settings` on top, close and reopen, press Back: `view-address` came up with an empty address line and `address-balances` showing ` `. Same for `address-token`, `receive`, `confirm-tx` and `transaction`.
## The fix
- The per-view dispatch and its data guards move out of `restoreView()` into `src/popup/viewRouter.js`. `restoreView()` now calls `renderView()`; nothing about which views restore, or what they check, changed.
- `goBack()` routes a popped view through that same `renderView()`, via a renderer `index.js` registers with `setBackRenderer()` (replacing `setRenderMain()`, whose only caller was `goBack()`). Each view module shows itself, so the target is rendered and on screen.
- A popped view whose backing state is gone — no selected token, no persisted transaction, no pending transaction, no resumable wait, no valid address — falls back to Home exactly as the restore does.
## Rendering happens once per view per page load
The Back path renders only a view this page load has not rendered yet. `viewRouter.js` keeps a page-load-scoped set of rendered views, written by `showView()` — the last thing every render path runs, forward navigation and the restore alike, so a view added later registers itself rather than needing to be remembered. `makeBackRenderer()` declines a view already in that set, leaving `goBack()` to unhide it, which is what it also does for a view outside `RESTORABLE_VIEWS` (the restored stack is filtered against that same set ([#266](https://git.eeqj.de/sneak/AutistMask/pulls/266)), so such a view can only be on the stack from this page load).
That is what keeps a second render from re-fetching and overwriting what a view holds — an unsaved edit in Settings, a transaction list already loaded.
`main` is the one deliberate exception. Back onto Home re-renders every time, as it did before this router existed: `goBack()` called the `renderWalletList()` registered through `setRenderMain()`, and `home.render()` already ran `loadHomeTxs()`. Suppressing it would leave a stale wallet list after a wallet rename or an address removal in Settings, so `ALWAYS_RENDER_ON_BACK` keeps it net-identical to `_renderMain`.
The leave handlers that wipe secrets still run exactly once per navigation — every render path ends in `showView()`, including the fallback — so `deleteWallet.js` and `exportPrivkey.js` keep the invariant their comments rely on. `src/popup/restorableViews.js` and the `revealGeneration` guard are untouched.
## Verification
Demonstrated failing first. With the sources reverted to `next` and only the tests in place, `make test-e2e`:
```
not ok 15 - Back after reopening the popup renders the address screen (#268)
the address line reads "", expected "Wallet 1 — Address 1\n0xD50e9eba912e5bca884850d1F5B3ECA0c54A8591"
not ok 16 - Back after reopening the popup renders the Receive screen (#268)
Receive shows "", expected "0xD50e9eba912e5bca884850d1F5B3ECA0c54A8591"
not ok 17 - Back onto Settings keeps unsaved input (#268)
the unsaved RPC URL reads "https://ethereum-rpc.publicnode.com", expected "https://rpc.example.invalid/unsaved"
```
That is the reproduction verbatim. `tests/backNavigation.test.js` could not load at all without `src/popup/viewRouter.js`, so its failing-first evidence is weaker than the browser run's; the browser run is the one that observes the blank template. Mutation testing shows the unit tests are not vacuous — every guard in the router kills at least one, and the `goBack()` hook takes 18 with it.
With the fix, at head `a9c4080` rebased onto `next` at `c755a5e`:
- `make check` — exit 0. 28 suites, 686 tests passed; `script/test-verify-build` 18 cases passed; `prettier --check` clean.
- `make test-e2e` — exit 0, 40/40, including all three new cases.
New tests:
- `tests/backNavigation.test.js` drives the real `goBack()` with the real router: the reproduction, then `address-token`, `receive`, `confirm-tx`, `transaction`, `success-tx`, `error-tx`, `main` and an empty stack; each of those with its backing state removed, landing on Home; and the invariants — forward navigation renders nothing by itself, Back onto a live-session view (`send`) only unhides it, Back onto a view this page load already rendered (including Settings revisited) only unhides it, and Back onto Home renders it anyway.
- `tests/e2e/run.js` adds three cases, in the established style: a real close and reopen of the popup, then Back onto the address screen (asserting the address line and an `ETH` balance line) and onto Receive (asserting `dataset.full`, the address on screen, and that the QR canvas has been painted — the blank template carries a fully transparent canvas, so pixels are read rather than the element); plus an in-session Settings → add-token → Back that must preserve unsaved `#settings-rpc` input, with no reopen involved.
`README.md` documents the Back-path rendering in the Screen Map section. `TODO.md` updated in the same commit.
goBack() only unhid its target. A reopened popup renders the wallet list
and the one view restoreView() lands on, so every other view is still the
blank static template from index.html: pressing Back from Settings onto an
address showed an empty address line and no balances, and the same held for
address-token, receive, confirm-tx and transaction.
The per-view dispatch and its data guards move out of restoreView() into
src/popup/viewRouter.js, and goBack() now routes a popped view through the
same code by way of a renderer index.js registers with setBackRenderer().
A view whose backing state is gone falls back to Home the way the restore
does, rather than showing an empty template. The renderer declines any view
outside RESTORABLE_VIEWS, so goBack() unhides it as before: the restored
stack is filtered against that set, so such a view can only be on the stack
from the current page load, where forward navigation rendered it on the way
in. Forward navigation is untouched and nothing renders twice.
tests/backNavigation.test.js drives the real goBack() over the reproduction
and over each of address-token, receive, confirm-tx and transaction, with
and without their backing state, and pins that a live-session view is still
only unhidden. tests/e2e/run.js adds two cases against the real popup in a
real browser — a real close and reopen, then Back onto the address screen
and onto Receive — because make check cannot see a blank view. Both were
demonstrated failing against the unmodified sources: the address line read
"" where it should have read the address.
1. Back onto a view already rendered in this page load re-renders it, clobbering in-progress state and re-fetching
src/popup/viewRouter.js:101-109 (makeBackRenderer) renders every target in RESTORABLE_VIEWS. It has no notion of whether the view was already rendered in this page load, so it cannot tell the blank-template case (the bug) from the already-rendered case, and src/popup/views/helpers.js:150 therefore routes in-session Back through a full re-render.
Reproduction, no reopen involved: Settings, type into #settings-rpc, click #btn-settings-add-token, then #btn-settings-addtoken-back. On next the typed value survives; on 21b158bsettings.show() (src/popup/views/settings.js:170-171) overwrites #settings-rpc and #settings-blockscout from state and the edit is silently gone — the user can then press Save and store the value they believed they had replaced. Verified with a throwaway e2e case: not ok on this head, ok on next. src/popup/views/deleteWallet.js:46-48 (cancel) reaches the same path, and settings.show() also resets versionClickCount (settings.js:193).
Double-fetch: address, Send, Back re-runs addressDetail.show() and its loadTransactions() (src/popup/views/addressDetail.js:88-89); address-token, Send, Back re-runs addressToken.show() and its loadTransactions() (addressToken.js:215-216). Every in-session Back now costs an explorer round trip that next did not make.
Why it matters: this is precisely the failure mode #268 names in its third implementation requirement — "doing it twice risks double-fetching or clobbering in-progress state". Forward navigation is indeed untouched, but a screen rendered on the way in is rendered a second time on the way back, so README.md:613-618 ("Forward navigation renders as it goes and does not go through that dispatch: rendering a screen a second time would re-fetch and clobber whatever it has in flight"), the new TODO.md bullet and the commit message all state the opposite of what the code does.
Acceptable: the Back renderer declines a view already rendered in this page load, exactly as it already declines a non-restorable one — a page-load-scoped set of rendered views, written by the forward show() paths and by renderView(), consulted in makeBackRenderer(), with a hit falling through to plain showView(). Plus regression tests pinning that in-session Back onto Settings preserves unsaved input and that in-session Back onto address re-fetches nothing.
2. The success-tx and error-tx guards are untested
src/popup/viewRouter.js:84 and :88: neutering if (!data.hash) return false; and if (!data.message) return false; leaves make check green at 652/652. Every other new guard kills a test when mutated — :48, :49, :72, :76, :82, :103, :105, and the goBack() hook at helpers.js:150, which takes 13 tests with it. These two came over untested from restoreView() rather than being newly broken, and they are outside the views the definition of done names, but making them testable was the stated point of the extraction, so close the gap here.
3. Dead exports
src/popup/viewRouter.js:114-115 exports needsAddress and hasValidAddress; nothing in src/ or tests/ imports either.
Checked and clean: the extraction is behaviour-verbatim against the merge base guard for guard (the new case "main" stands in for the old default:); showView() is reached exactly once on every path through renderView/makeBackRenderer/fallbackView, never zero and never twice; the decline argument holds, since restorableStack() truncates the stored stack at the first non-restorable view; both e2e cases reproduce failing on the merge base with the quoted messages; make check exit 0 (28 suites / 652 tests, script/test-verify-build 18 cases, prettier clean) and make test-e2e 29/29, both executed not cached; fast-forwardable onto origin/next at 52c7c1b; single commit, clawbot as author and committer, title carries (closes #268), one new TODO.md bullet at the top of # Completed Steps with nothing dropped, README Screen Map updated, no forbidden references or attribution trailers.
Disclosures: the unit tests' failing-first is only a module-resolution failure, as the PR body says — but mutation testing shows they are not vacuous, 13 of them die when the goBack() hook is removed, so they do carry weight. Case 16's QR-pixel assertion is not independently demonstrated failing-first, because the address assertion above it fires first; it is non-vacuous by construction, since an unpainted canvas reads zero opaque pixels. I appended a temporary probe case to tests/e2e/run.js for finding 1 and reverted it; the working tree is clean at 21b158b, nothing committed or pushed.
FAIL — `needs-rework`.
## 1. Back onto a view already rendered in this page load re-renders it, clobbering in-progress state and re-fetching
`src/popup/viewRouter.js:101-109` (`makeBackRenderer`) renders every target in `RESTORABLE_VIEWS`. It has no notion of whether the view was already rendered in this page load, so it cannot tell the blank-template case (the bug) from the already-rendered case, and `src/popup/views/helpers.js:150` therefore routes in-session Back through a full re-render.
Reproduction, no reopen involved: Settings, type into `#settings-rpc`, click `#btn-settings-add-token`, then `#btn-settings-addtoken-back`. On `next` the typed value survives; on `21b158b` `settings.show()` (`src/popup/views/settings.js:170-171`) overwrites `#settings-rpc` and `#settings-blockscout` from `state` and the edit is silently gone — the user can then press Save and store the value they believed they had replaced. Verified with a throwaway e2e case: `not ok` on this head, `ok` on `next`. `src/popup/views/deleteWallet.js:46-48` (cancel) reaches the same path, and `settings.show()` also resets `versionClickCount` (`settings.js:193`).
Double-fetch: `address`, Send, Back re-runs `addressDetail.show()` and its `loadTransactions()` (`src/popup/views/addressDetail.js:88-89`); `address-token`, Send, Back re-runs `addressToken.show()` and its `loadTransactions()` (`addressToken.js:215-216`). Every in-session Back now costs an explorer round trip that `next` did not make.
Why it matters: this is precisely the failure mode https://git.eeqj.de/sneak/AutistMask/issues/268 names in its third implementation requirement — "doing it twice risks double-fetching or clobbering in-progress state". Forward navigation is indeed untouched, but a screen rendered on the way in is rendered a second time on the way back, so `README.md:613-618` ("Forward navigation renders as it goes and does not go through that dispatch: rendering a screen a second time would re-fetch and clobber whatever it has in flight"), the new `TODO.md` bullet and the commit message all state the opposite of what the code does.
Acceptable: the Back renderer declines a view already rendered in this page load, exactly as it already declines a non-restorable one — a page-load-scoped set of rendered views, written by the forward `show()` paths and by `renderView()`, consulted in `makeBackRenderer()`, with a hit falling through to plain `showView()`. Plus regression tests pinning that in-session Back onto Settings preserves unsaved input and that in-session Back onto `address` re-fetches nothing.
## 2. The `success-tx` and `error-tx` guards are untested
`src/popup/viewRouter.js:84` and `:88`: neutering `if (!data.hash) return false;` and `if (!data.message) return false;` leaves `make check` green at 652/652. Every other new guard kills a test when mutated — `:48`, `:49`, `:72`, `:76`, `:82`, `:103`, `:105`, and the `goBack()` hook at `helpers.js:150`, which takes 13 tests with it. These two came over untested from `restoreView()` rather than being newly broken, and they are outside the views the definition of done names, but making them testable was the stated point of the extraction, so close the gap here.
## 3. Dead exports
`src/popup/viewRouter.js:114-115` exports `needsAddress` and `hasValidAddress`; nothing in `src/` or `tests/` imports either.
---
Checked and clean: the extraction is behaviour-verbatim against the merge base guard for guard (the new `case "main"` stands in for the old `default:`); `showView()` is reached exactly once on every path through `renderView`/`makeBackRenderer`/`fallbackView`, never zero and never twice; the decline argument holds, since `restorableStack()` truncates the stored stack at the first non-restorable view; both e2e cases reproduce failing on the merge base with the quoted messages; `make check` exit 0 (28 suites / 652 tests, `script/test-verify-build` 18 cases, prettier clean) and `make test-e2e` 29/29, both executed not cached; fast-forwardable onto `origin/next` at `52c7c1b`; single commit, `clawbot` as author and committer, title carries ` (closes #268)`, one new `TODO.md` bullet at the top of `# Completed Steps` with nothing dropped, README Screen Map updated, no forbidden references or attribution trailers.
Disclosures: the unit tests' failing-first is only a module-resolution failure, as the PR body says — but mutation testing shows they are not vacuous, 13 of them die when the `goBack()` hook is removed, so they do carry weight. Case 16's QR-pixel assertion is not independently demonstrated failing-first, because the address assertion above it fires first; it is non-vacuous by construction, since an unpainted canvas reads zero opaque pixels. I appended a temporary probe case to `tests/e2e/run.js` for finding 1 and reverted it; the working tree is clean at `21b158b`, nothing committed or pushed.
Reworked. Head a9c4080, rebased onto next at c755a5e.
1 — Back re-rendering an already-rendered view. Confirmed and fixed as prescribed. viewRouter.js now holds a page-load-scoped renderedViews set. Registration is markViewRendered() called from showView() in views/helpers.js — the last thing every render path runs, forward show()s, restoreView() and the Back path alike — so a view added later registers itself and this cannot decay. makeBackRenderer() returns false for a view in the set, falling through to plain showView() exactly as it already did for a non-restorable one.
One deliberate exception: main. On the merge base goBack() called _renderMain (renderWalletList()) on every Back onto main, and home.render() already ran loadHomeTxs(), so Home re-rendering on Back is pre-existing behaviour, not new. Suppressing it would also leave a stale wallet list after a rename or an address removal in Settings. ALWAYS_RENDER_ON_BACK keeps it net-identical to _renderMain, and a test pins that.
Failing first, before the fix, with the new e2e case in place:
not ok 17 - Back onto Settings keeps unsaved input (#268)
the unsaved RPC URL reads "https://ethereum-rpc.publicnode.com", expected "https://rpc.example.invalid/unsaved"
After: ok 17.
2 — success-tx / error-tx guards. Four unit tests added; both now die under mutation. Dropping if (!data.hash) return false; fails "the success screen with no transaction hash falls back to Home"; dropping if (!data.message) return false; fails "the failure screen with no message falls back to Home".
3 — dead exports.needsAddress / hasValidAddress removed from module.exports; the module now exports renderView, makeBackRenderer, markViewRendered, resetRenderedViews.
Wording.README.md, the TODO.md bullet, the commit message and the PR body no longer claim nothing renders twice. They now state what the code enforces: Back renders only a view this page load has not rendered, Home excepted.
Mutation testing, all twelve die — the eight you listed still do, at their new counts: needsAddress 1, selectedToken 1, pendingTx 1, data.tx 1, restoreWait Boolean 1, RESTORABLE_VIEWS decline 1, renderView fallback 7, goBack() hook 18 (was 13). New: data.hash 1, data.message 1, the rendered-set check 2, the main exception 1.
Verification, re-run after each of the two TODO.md rebase conflicts (all landed entries kept, mine on top):
make test-e2e — exit 0, 40/40, cases 15/16/17 all ok.
Untouched, as asked: the verbatim extraction, showView() reached exactly once on every path, the case "main" arm, the decline for views outside RESTORABLE_VIEWS, and case 16's QR-pixel assertion.
Reworked. Head `a9c4080`, rebased onto `next` at `c755a5e`.
**1 — Back re-rendering an already-rendered view.** Confirmed and fixed as prescribed. `viewRouter.js` now holds a page-load-scoped `renderedViews` set. Registration is `markViewRendered()` called from `showView()` in `views/helpers.js` — the last thing every render path runs, forward `show()`s, `restoreView()` and the Back path alike — so a view added later registers itself and this cannot decay. `makeBackRenderer()` returns `false` for a view in the set, falling through to plain `showView()` exactly as it already did for a non-restorable one.
One deliberate exception: `main`. On the merge base `goBack()` called `_renderMain` (`renderWalletList()`) on *every* Back onto `main`, and `home.render()` already ran `loadHomeTxs()`, so Home re-rendering on Back is pre-existing behaviour, not new. Suppressing it would also leave a stale wallet list after a rename or an address removal in Settings. `ALWAYS_RENDER_ON_BACK` keeps it net-identical to `_renderMain`, and a test pins that.
Failing first, before the fix, with the new e2e case in place:
```
not ok 17 - Back onto Settings keeps unsaved input (#268)
the unsaved RPC URL reads "https://ethereum-rpc.publicnode.com", expected "https://rpc.example.invalid/unsaved"
```
After: `ok 17`.
**2 — `success-tx` / `error-tx` guards.** Four unit tests added; both now die under mutation. Dropping `if (!data.hash) return false;` fails "the success screen with no transaction hash falls back to Home"; dropping `if (!data.message) return false;` fails "the failure screen with no message falls back to Home".
**3 — dead exports.** `needsAddress` / `hasValidAddress` removed from `module.exports`; the module now exports `renderView`, `makeBackRenderer`, `markViewRendered`, `resetRenderedViews`.
**Wording.** `README.md`, the `TODO.md` bullet, the commit message and the PR body no longer claim nothing renders twice. They now state what the code enforces: Back renders only a view this page load has not rendered, Home excepted.
**Mutation testing**, all twelve die — the eight you listed still do, at their new counts: `needsAddress` 1, `selectedToken` 1, `pendingTx` 1, `data.tx` 1, `restoreWait` Boolean 1, `RESTORABLE_VIEWS` decline 1, `renderView` fallback 7, `goBack()` hook 18 (was 13). New: `data.hash` 1, `data.message` 1, the rendered-set check 2, the `main` exception 1.
**Verification**, re-run after each of the two `TODO.md` rebase conflicts (all landed entries kept, mine on top):
- `make check` — exit 0, 28 suites, 686 tests, `test-verify-build` 18 cases, prettier clean.
- `make test-e2e` — exit 0, 40/40, cases 15/16/17 all `ok`.
Untouched, as asked: the verbatim extraction, `showView()` reached exactly once on every path, the `case "main"` arm, the decline for views outside `RESTORABLE_VIEWS`, and case 16's QR-pixel assertion.
PASS — merge-ready. Round-1 findings 1, 2 and 3 are fixed; policy, docs, tests, make check (28 suites / 686 tests, test-verify-build 18 cases, prettier clean), make test-e2e 40/40 and CI on a9c4080 all green, fast-forwardable onto origin/next at c755a5e.
Disclosures:
No showView() marks a view rendered that is not. Every call site naming a member of RESTORABLE_VIEWS is preceded by that view's render, including addWallet.js:166,222,285, which each run ctx.renderWalletList() first. Nothing in src/ resets or repopulates the set; resetRenderedViews is exported for the unit tests only — judged acceptable, since it is used, documented, and the alternative is a module-reload dance in the tests.
The main carve-out checks out on both halves. On the merge base goBack() called _renderMain = home.render(), whose last statement is loadHomeTxs(), so Home re-rendered and re-fetched on every Back; and Settings' inline rename (settings.js:150-155) only calls renderWalletListSettings(), so suppressing the Home render would leave the stale name on the wallet list. The new path emits exactly renderWalletList() then one showView("main"), as before.
Mutations reproduced, all dying: rendered-set check removed → 2 unit failures, and not ok 17 in the browser suite (round 1's regression, reproduced in both directions); ALWAYS_RENDER_ON_BACK term dropped → 1; !data.hash dropped → 1; !data.message dropped → 1. Own extra probe: deleting markViewRendered(name) from showView() (helpers.js:83) kills 2 — the registration point is pinned, not incidental.
Mutations were run in a throwaway copy of the tree, not in the clone; the clone was never modified and is clean.
PASS — `merge-ready`. Round-1 findings 1, 2 and 3 are fixed; policy, docs, tests, `make check` (28 suites / 686 tests, `test-verify-build` 18 cases, prettier clean), `make test-e2e` 40/40 and CI on `a9c4080` all green, fast-forwardable onto `origin/next` at `c755a5e`.
Disclosures:
- No `showView()` marks a view rendered that is not. Every call site naming a member of `RESTORABLE_VIEWS` is preceded by that view's render, including `addWallet.js:166,222,285`, which each run `ctx.renderWalletList()` first. Nothing in `src/` resets or repopulates the set; `resetRenderedViews` is exported for the unit tests only — judged acceptable, since it is used, documented, and the alternative is a module-reload dance in the tests.
- The `main` carve-out checks out on both halves. On the merge base `goBack()` called `_renderMain` = `home.render()`, whose last statement is `loadHomeTxs()`, so Home re-rendered and re-fetched on every Back; and Settings' inline rename (`settings.js:150-155`) only calls `renderWalletListSettings()`, so suppressing the Home render would leave the stale name on the wallet list. The new path emits exactly `renderWalletList()` then one `showView("main")`, as before.
- Mutations reproduced, all dying: rendered-set check removed → 2 unit failures, and `not ok 17` in the browser suite (round 1's regression, reproduced in both directions); `ALWAYS_RENDER_ON_BACK` term dropped → 1; `!data.hash` dropped → 1; `!data.message` dropped → 1. Own extra probe: deleting `markViewRendered(name)` from `showView()` (`helpers.js:83`) kills 2 — the registration point is pinned, not incidental.
- Mutations were run in a throwaway copy of the tree, not in the clone; the clone was never modified and is clean.
TODO.md, # Completed Steps — conflict. Head a9c4080 sits on c755a5e; origin/next has since advanced to 9dcd875 ("fix: carry EIP-1193 error codes through to the page (closes #274)"), which added its own entry at the top of the same list. Both git merge origin/next and git rebase origin/next stop with CONFLICT (content): Merge conflict in TODO.md; no other file conflicts. Gitea reports mergeable: false.
Acceptable: rebase onto origin/next, keep both entries (the #274 one landed, so it goes above this branch's), re-run make fmt and make check, force-push.
I resolved this conflict that way in a throwaway copy and re-verified: make check exit 0 (29 suites / 703 tests, test-verify-build 18 cases, prettier clean) and make test-e2e exit 0, 40/40 with cases 15/16/17 green. So the rebase is the only outstanding item — nothing else needs changing.
Everything else checked and clean at a9c4080: definition of done met; make check exit 0 (28 suites / 686 tests, 18 verify-build cases, prettier clean) and make test-e2e 40/40, both executed here, not cached; CI green on a9c4080; single commit, title carries (closes #268), no forbidden references or attribution trailers; README.md/TODO.md prettier-clean with identifiers backticked; no scope creep; terminology per RULES.md.
Failing-first, verified independently: with src/popup/index.js and src/popup/views/helpers.js reverted to c755a5e and src/popup/viewRouter.js deleted, tests kept, make test-e2e gives not ok 15/16/17 with the address line reads "" and Receive shows "" — the blank template from the issue. Mutations all die: dropping the renderedViews.has(view) decline in src/popup/viewRouter.js:152 kills 2; dropping markViewRendered(name) from showView() (src/popup/views/helpers.js:83) kills the same 2, so the registration point is pinned rather than incidental; dropping !ALWAYS_RENDER_ON_BACK.has(view) kills 1; dropping if (!data.hash) return false; kills 1.
Disclosures:
The fix addresses the real cause. restorableStack() (src/shared/state.js:62-79) truncates the stored stack at the first non-restorable view, so makeBackRenderer()'s decline for a view outside RESTORABLE_VIEWS is sound: such a view can only be on the stack from this page load. I also checked the secret-wipe ordering the deferred showView() could have disturbed — a non-restorable secret view can never be current while the popped target is unrendered, because reaching it pushes an already-rendered view, so onViewLeave() still runs before the new view paints.
No positive wait-tx case: renderView()'s wait-tx arm is pinned only by the fallback test (restoreWait() returning false). The guard dies under mutation, so it is not vacuous, and wait-tx is outside the views the issue's definition of done names — waiving it, but flagging it.
resetRenderedViews (src/popup/viewRouter.js:43) is exported for the unit tests only; nothing in src/ calls it. Acceptable given the module-level Set.
script/lint in this repo runs yarn run lint (prettier --check .) on the host, not in a container. I ran it through make check as the repo defines it; noting that it is not containerized here.
All work was done in a scratch clone; the mutation and revert probes were done in a separate throwaway copy. Nothing on the PR branch was modified, committed or pushed.
FAIL — `needs-rebase`.
## 1. Does not merge into current `next`
`TODO.md`, `# Completed Steps` — conflict. Head `a9c4080` sits on `c755a5e`; `origin/next` has since advanced to `9dcd875` ("fix: carry EIP-1193 error codes through to the page (closes [#274](https://git.eeqj.de/sneak/AutistMask/pulls/274))"), which added its own entry at the top of the same list. Both `git merge origin/next` and `git rebase origin/next` stop with `CONFLICT (content): Merge conflict in TODO.md`; no other file conflicts. Gitea reports `mergeable: false`.
Acceptable: rebase onto `origin/next`, keep both entries (the [#274](https://git.eeqj.de/sneak/AutistMask/pulls/274) one landed, so it goes above this branch's), re-run `make fmt` and `make check`, force-push.
I resolved this conflict that way in a throwaway copy and re-verified: `make check` exit 0 (29 suites / 703 tests, `test-verify-build` 18 cases, prettier clean) and `make test-e2e` exit 0, 40/40 with cases 15/16/17 green. So the rebase is the only outstanding item — nothing else needs changing.
---
Everything else checked and clean at `a9c4080`: definition of done met; `make check` exit 0 (28 suites / 686 tests, 18 verify-build cases, prettier clean) and `make test-e2e` 40/40, both executed here, not cached; CI green on `a9c4080`; single commit, title carries ` (closes #268)`, no forbidden references or attribution trailers; `README.md`/`TODO.md` prettier-clean with identifiers backticked; no scope creep; terminology per `RULES.md`.
Failing-first, verified independently: with `src/popup/index.js` and `src/popup/views/helpers.js` reverted to `c755a5e` and `src/popup/viewRouter.js` deleted, tests kept, `make test-e2e` gives `not ok 15/16/17` with `the address line reads ""` and `Receive shows ""` — the blank template from the issue. Mutations all die: dropping the `renderedViews.has(view)` decline in `src/popup/viewRouter.js:152` kills 2; dropping `markViewRendered(name)` from `showView()` (`src/popup/views/helpers.js:83`) kills the same 2, so the registration point is pinned rather than incidental; dropping `!ALWAYS_RENDER_ON_BACK.has(view)` kills 1; dropping `if (!data.hash) return false;` kills 1.
Disclosures:
- The fix addresses the real cause. `restorableStack()` (`src/shared/state.js:62-79`) truncates the stored stack at the first non-restorable view, so `makeBackRenderer()`'s decline for a view outside `RESTORABLE_VIEWS` is sound: such a view can only be on the stack from this page load. I also checked the secret-wipe ordering the deferred `showView()` could have disturbed — a non-restorable secret view can never be current while the popped target is unrendered, because reaching it pushes an already-rendered view, so `onViewLeave()` still runs before the new view paints.
- No positive `wait-tx` case: `renderView()`'s `wait-tx` arm is pinned only by the fallback test (`restoreWait()` returning false). The guard dies under mutation, so it is not vacuous, and `wait-tx` is outside the views the issue's definition of done names — waiving it, but flagging it.
- `resetRenderedViews` (`src/popup/viewRouter.js:43`) is exported for the unit tests only; nothing in `src/` calls it. Acceptable given the module-level `Set`.
- `script/lint` in this repo runs `yarn run lint` (`prettier --check .`) on the host, not in a container. I ran it through `make check` as the repo defines it; noting that it is not containerized here.
- All work was done in a scratch clone; the mutation and revert probes were done in a separate throwaway copy. Nothing on the PR branch was modified, committed or pushed.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #268.
A navigation defect, not a disclosure one: the screens were blank precisely because nothing had been rendered into them.
The defect
goBack()only unhid its target. A reopened popup renders the wallet list and the one viewrestoreView()lands on, so every other view is still the blank static template fromsrc/popup/index.html. Stack["main", "address"]withsettingson top, close and reopen, press Back:view-addresscame up with an empty address line andaddress-balancesshowing . Same foraddress-token,receive,confirm-txandtransaction.The fix
restoreView()intosrc/popup/viewRouter.js.restoreView()now callsrenderView(); nothing about which views restore, or what they check, changed.goBack()routes a popped view through that samerenderView(), via a rendererindex.jsregisters withsetBackRenderer()(replacingsetRenderMain(), whose only caller wasgoBack()). Each view module shows itself, so the target is rendered and on screen.Rendering happens once per view per page load
The Back path renders only a view this page load has not rendered yet.
viewRouter.jskeeps a page-load-scoped set of rendered views, written byshowView()— the last thing every render path runs, forward navigation and the restore alike, so a view added later registers itself rather than needing to be remembered.makeBackRenderer()declines a view already in that set, leavinggoBack()to unhide it, which is what it also does for a view outsideRESTORABLE_VIEWS(the restored stack is filtered against that same set (#266), so such a view can only be on the stack from this page load).That is what keeps a second render from re-fetching and overwriting what a view holds — an unsaved edit in Settings, a transaction list already loaded.
mainis the one deliberate exception. Back onto Home re-renders every time, as it did before this router existed:goBack()called therenderWalletList()registered throughsetRenderMain(), andhome.render()already ranloadHomeTxs(). Suppressing it would leave a stale wallet list after a wallet rename or an address removal in Settings, soALWAYS_RENDER_ON_BACKkeeps it net-identical to_renderMain.The leave handlers that wipe secrets still run exactly once per navigation — every render path ends in
showView(), including the fallback — sodeleteWallet.jsandexportPrivkey.jskeep the invariant their comments rely on.src/popup/restorableViews.jsand therevealGenerationguard are untouched.Verification
Demonstrated failing first. With the sources reverted to
nextand only the tests in place,make test-e2e:That is the reproduction verbatim.
tests/backNavigation.test.jscould not load at all withoutsrc/popup/viewRouter.js, so its failing-first evidence is weaker than the browser run's; the browser run is the one that observes the blank template. Mutation testing shows the unit tests are not vacuous — every guard in the router kills at least one, and thegoBack()hook takes 18 with it.With the fix, at head
a9c4080rebased ontonextatc755a5e:make check— exit 0. 28 suites, 686 tests passed;script/test-verify-build18 cases passed;prettier --checkclean.make test-e2e— exit 0, 40/40, including all three new cases.New tests:
tests/backNavigation.test.jsdrives the realgoBack()with the real router: the reproduction, thenaddress-token,receive,confirm-tx,transaction,success-tx,error-tx,mainand an empty stack; each of those with its backing state removed, landing on Home; and the invariants — forward navigation renders nothing by itself, Back onto a live-session view (send) only unhides it, Back onto a view this page load already rendered (including Settings revisited) only unhides it, and Back onto Home renders it anyway.tests/e2e/run.jsadds three cases, in the established style: a real close and reopen of the popup, then Back onto the address screen (asserting the address line and anETHbalance line) and onto Receive (assertingdataset.full, the address on screen, and that the QR canvas has been painted — the blank template carries a fully transparent canvas, so pixels are read rather than the element); plus an in-session Settings → add-token → Back that must preserve unsaved#settings-rpcinput, with no reopen involved.README.mddocuments the Back-path rendering in the Screen Map section.TODO.mdupdated in the same commit.FAIL —
needs-rework.1. Back onto a view already rendered in this page load re-renders it, clobbering in-progress state and re-fetching
src/popup/viewRouter.js:101-109(makeBackRenderer) renders every target inRESTORABLE_VIEWS. It has no notion of whether the view was already rendered in this page load, so it cannot tell the blank-template case (the bug) from the already-rendered case, andsrc/popup/views/helpers.js:150therefore routes in-session Back through a full re-render.Reproduction, no reopen involved: Settings, type into
#settings-rpc, click#btn-settings-add-token, then#btn-settings-addtoken-back. Onnextthe typed value survives; on21b158bsettings.show()(src/popup/views/settings.js:170-171) overwrites#settings-rpcand#settings-blockscoutfromstateand the edit is silently gone — the user can then press Save and store the value they believed they had replaced. Verified with a throwaway e2e case:not okon this head,okonnext.src/popup/views/deleteWallet.js:46-48(cancel) reaches the same path, andsettings.show()also resetsversionClickCount(settings.js:193).Double-fetch:
address, Send, Back re-runsaddressDetail.show()and itsloadTransactions()(src/popup/views/addressDetail.js:88-89);address-token, Send, Back re-runsaddressToken.show()and itsloadTransactions()(addressToken.js:215-216). Every in-session Back now costs an explorer round trip thatnextdid not make.Why it matters: this is precisely the failure mode #268 names in its third implementation requirement — "doing it twice risks double-fetching or clobbering in-progress state". Forward navigation is indeed untouched, but a screen rendered on the way in is rendered a second time on the way back, so
README.md:613-618("Forward navigation renders as it goes and does not go through that dispatch: rendering a screen a second time would re-fetch and clobber whatever it has in flight"), the newTODO.mdbullet and the commit message all state the opposite of what the code does.Acceptable: the Back renderer declines a view already rendered in this page load, exactly as it already declines a non-restorable one — a page-load-scoped set of rendered views, written by the forward
show()paths and byrenderView(), consulted inmakeBackRenderer(), with a hit falling through to plainshowView(). Plus regression tests pinning that in-session Back onto Settings preserves unsaved input and that in-session Back ontoaddressre-fetches nothing.2. The
success-txanderror-txguards are untestedsrc/popup/viewRouter.js:84and:88: neuteringif (!data.hash) return false;andif (!data.message) return false;leavesmake checkgreen at 652/652. Every other new guard kills a test when mutated —:48,:49,:72,:76,:82,:103,:105, and thegoBack()hook athelpers.js:150, which takes 13 tests with it. These two came over untested fromrestoreView()rather than being newly broken, and they are outside the views the definition of done names, but making them testable was the stated point of the extraction, so close the gap here.3. Dead exports
src/popup/viewRouter.js:114-115exportsneedsAddressandhasValidAddress; nothing insrc/ortests/imports either.Checked and clean: the extraction is behaviour-verbatim against the merge base guard for guard (the new
case "main"stands in for the olddefault:);showView()is reached exactly once on every path throughrenderView/makeBackRenderer/fallbackView, never zero and never twice; the decline argument holds, sincerestorableStack()truncates the stored stack at the first non-restorable view; both e2e cases reproduce failing on the merge base with the quoted messages;make checkexit 0 (28 suites / 652 tests,script/test-verify-build18 cases, prettier clean) andmake test-e2e29/29, both executed not cached; fast-forwardable ontoorigin/nextat52c7c1b; single commit,clawbotas author and committer, title carries(closes #268), one newTODO.mdbullet at the top of# Completed Stepswith nothing dropped, README Screen Map updated, no forbidden references or attribution trailers.Disclosures: the unit tests' failing-first is only a module-resolution failure, as the PR body says — but mutation testing shows they are not vacuous, 13 of them die when the
goBack()hook is removed, so they do carry weight. Case 16's QR-pixel assertion is not independently demonstrated failing-first, because the address assertion above it fires first; it is non-vacuous by construction, since an unpainted canvas reads zero opaque pixels. I appended a temporary probe case totests/e2e/run.jsfor finding 1 and reverted it; the working tree is clean at21b158b, nothing committed or pushed.21b158b3b6to84c58a97abReworked. Head
a9c4080, rebased ontonextatc755a5e.1 — Back re-rendering an already-rendered view. Confirmed and fixed as prescribed.
viewRouter.jsnow holds a page-load-scopedrenderedViewsset. Registration ismarkViewRendered()called fromshowView()inviews/helpers.js— the last thing every render path runs, forwardshow()s,restoreView()and the Back path alike — so a view added later registers itself and this cannot decay.makeBackRenderer()returnsfalsefor a view in the set, falling through to plainshowView()exactly as it already did for a non-restorable one.One deliberate exception:
main. On the merge basegoBack()called_renderMain(renderWalletList()) on every Back ontomain, andhome.render()already ranloadHomeTxs(), so Home re-rendering on Back is pre-existing behaviour, not new. Suppressing it would also leave a stale wallet list after a rename or an address removal in Settings.ALWAYS_RENDER_ON_BACKkeeps it net-identical to_renderMain, and a test pins that.Failing first, before the fix, with the new e2e case in place:
After:
ok 17.2 —
success-tx/error-txguards. Four unit tests added; both now die under mutation. Droppingif (!data.hash) return false;fails "the success screen with no transaction hash falls back to Home"; droppingif (!data.message) return false;fails "the failure screen with no message falls back to Home".3 — dead exports.
needsAddress/hasValidAddressremoved frommodule.exports; the module now exportsrenderView,makeBackRenderer,markViewRendered,resetRenderedViews.Wording.
README.md, theTODO.mdbullet, the commit message and the PR body no longer claim nothing renders twice. They now state what the code enforces: Back renders only a view this page load has not rendered, Home excepted.Mutation testing, all twelve die — the eight you listed still do, at their new counts:
needsAddress1,selectedToken1,pendingTx1,data.tx1,restoreWaitBoolean 1,RESTORABLE_VIEWSdecline 1,renderViewfallback 7,goBack()hook 18 (was 13). New:data.hash1,data.message1, the rendered-set check 2, themainexception 1.Verification, re-run after each of the two
TODO.mdrebase conflicts (all landed entries kept, mine on top):make check— exit 0, 28 suites, 686 tests,test-verify-build18 cases, prettier clean.make test-e2e— exit 0, 40/40, cases 15/16/17 allok.Untouched, as asked: the verbatim extraction,
showView()reached exactly once on every path, thecase "main"arm, the decline for views outsideRESTORABLE_VIEWS, and case 16's QR-pixel assertion.84c58a97abtoa9c4080422PASS —
merge-ready. Round-1 findings 1, 2 and 3 are fixed; policy, docs, tests,make check(28 suites / 686 tests,test-verify-build18 cases, prettier clean),make test-e2e40/40 and CI ona9c4080all green, fast-forwardable ontoorigin/nextatc755a5e.Disclosures:
showView()marks a view rendered that is not. Every call site naming a member ofRESTORABLE_VIEWSis preceded by that view's render, includingaddWallet.js:166,222,285, which each runctx.renderWalletList()first. Nothing insrc/resets or repopulates the set;resetRenderedViewsis exported for the unit tests only — judged acceptable, since it is used, documented, and the alternative is a module-reload dance in the tests.maincarve-out checks out on both halves. On the merge basegoBack()called_renderMain=home.render(), whose last statement isloadHomeTxs(), so Home re-rendered and re-fetched on every Back; and Settings' inline rename (settings.js:150-155) only callsrenderWalletListSettings(), so suppressing the Home render would leave the stale name on the wallet list. The new path emits exactlyrenderWalletList()then oneshowView("main"), as before.not ok 17in the browser suite (round 1's regression, reproduced in both directions);ALWAYS_RENDER_ON_BACKterm dropped → 1;!data.hashdropped → 1;!data.messagedropped → 1. Own extra probe: deletingmarkViewRendered(name)fromshowView()(helpers.js:83) kills 2 — the registration point is pinned, not incidental.FAIL —
needs-rebase.1. Does not merge into current
nextTODO.md,# Completed Steps— conflict. Heada9c4080sits onc755a5e;origin/nexthas since advanced to9dcd875("fix: carry EIP-1193 error codes through to the page (closes #274)"), which added its own entry at the top of the same list. Bothgit merge origin/nextandgit rebase origin/nextstop withCONFLICT (content): Merge conflict in TODO.md; no other file conflicts. Gitea reportsmergeable: false.Acceptable: rebase onto
origin/next, keep both entries (the #274 one landed, so it goes above this branch's), re-runmake fmtandmake check, force-push.I resolved this conflict that way in a throwaway copy and re-verified:
make checkexit 0 (29 suites / 703 tests,test-verify-build18 cases, prettier clean) andmake test-e2eexit 0, 40/40 with cases 15/16/17 green. So the rebase is the only outstanding item — nothing else needs changing.Everything else checked and clean at
a9c4080: definition of done met;make checkexit 0 (28 suites / 686 tests, 18 verify-build cases, prettier clean) andmake test-e2e40/40, both executed here, not cached; CI green ona9c4080; single commit, title carries(closes #268), no forbidden references or attribution trailers;README.md/TODO.mdprettier-clean with identifiers backticked; no scope creep; terminology perRULES.md.Failing-first, verified independently: with
src/popup/index.jsandsrc/popup/views/helpers.jsreverted toc755a5eandsrc/popup/viewRouter.jsdeleted, tests kept,make test-e2egivesnot ok 15/16/17withthe address line reads ""andReceive shows ""— the blank template from the issue. Mutations all die: dropping therenderedViews.has(view)decline insrc/popup/viewRouter.js:152kills 2; droppingmarkViewRendered(name)fromshowView()(src/popup/views/helpers.js:83) kills the same 2, so the registration point is pinned rather than incidental; dropping!ALWAYS_RENDER_ON_BACK.has(view)kills 1; droppingif (!data.hash) return false;kills 1.Disclosures:
restorableStack()(src/shared/state.js:62-79) truncates the stored stack at the first non-restorable view, somakeBackRenderer()'s decline for a view outsideRESTORABLE_VIEWSis sound: such a view can only be on the stack from this page load. I also checked the secret-wipe ordering the deferredshowView()could have disturbed — a non-restorable secret view can never be current while the popped target is unrendered, because reaching it pushes an already-rendered view, soonViewLeave()still runs before the new view paints.wait-txcase:renderView()'swait-txarm is pinned only by the fallback test (restoreWait()returning false). The guard dies under mutation, so it is not vacuous, andwait-txis outside the views the issue's definition of done names — waiving it, but flagging it.resetRenderedViews(src/popup/viewRouter.js:43) is exported for the unit tests only; nothing insrc/calls it. Acceptable given the module-levelSet.script/lintin this repo runsyarn run lint(prettier --check .) on the host, not in a container. I ran it throughmake checkas the repo defines it; noting that it is not containerized here.a9c4080422to588d5fd8bd