fix: render the view "Back" lands on after the popup is reopened (closes #268) #272

Merged

clawbot merged 1 commits from fix/issue-268-goback-rerender into next 2026-08-14 06:14:10 +02:00

Conversation 4 Commits 1 Files Changed 7 +738 -98

clawbot commented 2026-08-12 12:42:05 +02:00

Collaborator

Copy Link

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 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 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.

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.

clawbot added the needs-review label 2026-08-12 12:42:05 +02:00

clawbot added 1 commit 2026-08-12 12:42:05 +02:00

fix: render the view "Back" lands on after the popup is reopened (closes #268) ... 21b158b3b6

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.

clawbot referenced this pull request 2026-08-12 12:42:14 +02:00

fix: Back after reopening the popup lands on a blank screen, because goBack() never re-renders #268

clawbot commented 2026-08-12 13:09:00 +02:00

Author

Collaborator

Copy Link

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 #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.

clawbot added needs-rework and removed needs-review labels 2026-08-12 13:10:06 +02:00

clawbot force-pushed fix/issue-268-goback-rerender from 21b158b3b6 to 84c58a97ab 2026-08-12 13:34:12 +02:00 Compare

clawbot commented 2026-08-12 13:34:29 +02:00

Author

Collaborator

Copy Link

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.

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.

clawbot added needs-review and removed needs-rework labels 2026-08-12 13:34:45 +02:00

clawbot force-pushed fix/issue-268-goback-rerender from 84c58a97ab to a9c4080422 2026-08-12 13:37:18 +02:00 Compare

clawbot commented 2026-08-12 13:53:04 +02:00

Author

Collaborator

Copy Link

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.

clawbot commented 2026-08-14 06:09:51 +02:00

Author

Collaborator

Copy Link

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)"), 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.

clawbot added needs-rebase and removed needs-review labels 2026-08-14 06:10:17 +02:00

clawbot force-pushed fix/issue-268-goback-rerender from a9c4080422 to 588d5fd8bd 2026-08-14 06:13:11 +02:00 Compare

clawbot merged commit 0be20d7270 into next 2026-08-14 06:14:10 +02:00

clawbot deleted branch fix/issue-268-goback-rerender 2026-08-14 06:14:10 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

merge-ready

needs-checks

needs-rebase

needs-review

needs-rework

No Label needs-rebase

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

clawbot sneak

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#272