From 588d5fd8bd929b8fb571109ffd5da210b5d7e963 Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 12 Aug 2026 10:41:26 +0000 Subject: [PATCH] fix: render the view "Back" lands on after the popup is reopened (closes #268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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. Back onto a view already in that set unhides it and nothing more, exactly as it does for a view outside RESTORABLE_VIEWS: rendering a second time would re-fetch and overwrite what the view holds, such as an edit typed into Settings and not yet saved. Home stays the exception and is re-rendered on every Back, as it was when goBack() called the renderWalletList() registered through setRenderMain(), so the wallet list reflects what changed while the user was away from it. tests/backNavigation.test.js drives the real goBack() over the reproduction and over each of address-token, receive, confirm-tx, transaction, success-tx and error-tx, with and without their backing state, and pins that a view this page load already rendered — a live-session view, or Settings revisited — is only unhidden, while Home still re-renders. tests/e2e/run.js adds three cases against the real popup in a real browser, because make check cannot see a blank view: a real close and reopen then Back onto the address screen and onto Receive, and an in-session Back onto Settings that must keep unsaved input. All three were demonstrated failing first. --- README.md | 15 ++ TODO.md | 17 ++ src/popup/index.js | 110 +++--------- src/popup/viewRouter.js | 167 ++++++++++++++++++ src/popup/views/helpers.js | 32 +++- tests/backNavigation.test.js | 329 +++++++++++++++++++++++++++++++++++ tests/e2e/run.js | 166 ++++++++++++++++++ 7 files changed, 738 insertions(+), 98 deletions(-) create mode 100644 src/popup/viewRouter.js create mode 100644 tests/backNavigation.test.js diff --git a/README.md b/README.md index edeae29..cb2733a 100644 --- a/README.md +++ b/README.md @@ -638,6 +638,21 @@ ExportPrivKey and ShowRecoveryPhrase — are deliberately absent from that list, so the popup can never reopen onto one of them with no password prompt in front of it. +A reopened popup renders the wallet list and the one screen it restores onto, +and nothing else, so every screen on the stack behind that one is still the +blank template from `index.html`. "Back" therefore renders its target rather +than only unhiding it, through the same dispatch and data guards as the restore +(`src/popup/viewRouter.js`), and falls back to Home when the state the target +would render is gone. + +It renders only a screen this page load has not rendered yet. Forward navigation +renders as it goes, and `viewRouter.js` records every screen that reaches +`showView()`, so "Back" onto a screen already on the page unhides it and nothing +more — rendering it a second time would re-fetch and overwrite what it holds, +such as an edit typed into Settings and not yet saved. Home is the one screen +"Back" always re-renders, so the wallet list reflects anything that changed +while the user was away from it. + Every screen that holds secret material in the page registers a cleanup with `onViewLeave()` (`src/popup/views/helpers.js`), which `showView()` runs on every exit from that screen rather than only on its "Back" button, so nothing secret diff --git a/TODO.md b/TODO.md index d3898e5..86fd8b7 100644 --- a/TODO.md +++ b/TODO.md @@ -59,6 +59,23 @@ undefined identifiers, which is how `tests/inpageErrors.test.js`, and the e2e probe that printed the missing code now requires it on the page's Error as well as on the wire, for all four rejected flows ([#274](https://git.eeqj.de/sneak/AutistMask/issues/274)). +- 2026-08-12: "Back" now renders the screen it lands on instead of only unhiding + it. A reopened popup renders the wallet list and the one screen it restores + onto, so every screen further down the stack was still the blank template from + `index.html`, and Back walked straight onto it — an empty address, no + balances, no QR code. The Back path now goes through the same per-view + dispatch and data guards as the restore (`src/popup/viewRouter.js`, shared + with `restoreView()`), falling back to Home when the state the target would + render is gone. It renders only a view this page load has not rendered yet: + `viewRouter.js` records every view that reaches `showView()`, which is where + forward navigation and the restore both end, so Back onto a view already on + the page unhides it and nothing more. That is what keeps a second render from + re-fetching and overwriting what the view holds — an unsaved edit in Settings, + a transaction list already loaded. Home is the exception and is always + re-rendered, as it was before. Covered by unit tests on the real `goBack()` + and by three end-to-end cases against the real popup, each demonstrated + failing on the unfixed build + ([#268](https://git.eeqj.de/sneak/AutistMask/issues/268)). - 2026-08-12: `KNOWN_SYMBOLS` now maps a symbol to the set of contract addresses that bear it, not to one of them. A ticker is not unique: seven of the 512 bundled tokens — `FRAX`, `REUSD`, `TON`, `EURE`, `MSUSD`, `MUSD` and `JPYC` — diff --git a/src/popup/index.js b/src/popup/index.js index b609adc..4185491 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -9,16 +9,17 @@ const { $, showView, updateDebugBanner, - setRenderMain, + setBackRenderer, pushCurrentView, goBack, clearViewStack, } = require("./views/helpers"); const { applyTheme } = require("./theme"); -// Views that can be fully re-rendered from persisted state. All others fall -// back to the nearest restorable parent; see the module for why the -// secret-bearing views are absent. -const { RESTORABLE_VIEWS } = require("./restorableViews"); +// Renders a view the popup lands on without having navigated to it forward: +// on restore here, and on Back. Only the views that can be fully re-rendered +// from persisted state (RESTORABLE_VIEWS, src/popup/restorableViews.js) go +// through it; anything else falls back to the nearest restorable parent. +const { renderView, makeBackRenderer } = require("./viewRouter"); const home = require("./views/home"); const welcome = require("./views/welcome"); @@ -108,91 +109,22 @@ const ctx = { }, }; -function needsAddress(view) { - return ( - view === "address" || - view === "address-token" || - view === "receive" || - view === "transaction" - ); -} - -function hasValidAddress() { - return ( - state.selectedWallet !== null && - state.selectedAddress !== null && - state.wallets[state.selectedWallet] && - state.wallets[state.selectedWallet].addresses[state.selectedAddress] - ); -} +// The view modules the router renders through, keyed as it expects them. +const viewModules = { + main: { show: () => fallbackView() }, + addressDetail, + addressToken, + receive, + settings, + settingsAddToken, + confirmTx, + transactionDetail, + txStatus, +}; function restoreView() { - const view = state.currentView; - if (!view || !RESTORABLE_VIEWS.has(view)) { - return fallbackView(); - } - - if (needsAddress(view) && !hasValidAddress()) { - return fallbackView(); - } - - if (view === "address-token" && !state.selectedToken) { - return fallbackView(); - } - - switch (view) { - case "address": - addressDetail.show(); - break; - case "address-token": - addressToken.show(); - break; - case "receive": - receive.show(); - break; - case "settings": - settings.show(); - break; - case "settings-addtoken": - settingsAddToken.show(); - break; - case "confirm-tx": - if (state.viewData && state.viewData.pendingTx) { - confirmTx.restore(); - } else { - fallbackView(); - } - break; - case "transaction": - if (state.viewData && state.viewData.tx) { - transactionDetail.render(); - } else { - fallbackView(); - } - break; - case "wait-tx": - // Resumes the receipt poll from the persisted broadcast time. - if (!txStatus.restoreWait()) { - fallbackView(); - } - break; - case "success-tx": - if (state.viewData && state.viewData.hash) { - txStatus.renderSuccess(); - } else { - fallbackView(); - } - break; - case "error-tx": - if (state.viewData && state.viewData.message) { - txStatus.renderError(); - } else { - fallbackView(); - } - break; - default: - fallbackView(); - break; + if (!renderView(state.currentView, state, viewModules)) { + fallbackView(); } } @@ -247,7 +179,7 @@ async function init() { settings.show(); }); - setRenderMain(renderWalletList); + setBackRenderer(makeBackRenderer(state, viewModules)); welcome.init(ctx); addWallet.init(ctx); diff --git a/src/popup/viewRouter.js b/src/popup/viewRouter.js new file mode 100644 index 0000000..b37d53f --- /dev/null +++ b/src/popup/viewRouter.js @@ -0,0 +1,167 @@ +// Rendering a view the popup lands on without having navigated to it +// forward: on restore, and on Back. In both cases the view may never have +// been rendered in this page load — a reopened popup renders only the +// wallet list and the view it restores onto, so every other view is still +// the blank static template from index.html — so unhiding it is not enough. +// +// Forward navigation renders as it goes and must NOT come through here: +// rendering a second time would re-fetch and clobber whatever the view has +// in flight. +// +// The view modules are injected and nothing here touches the DOM, so the +// dispatch and its data guards can be tested directly; src/popup/index.js +// cannot be required outside a browser. + +const { RESTORABLE_VIEWS } = require("./restorableViews"); + +// The views this page load has rendered. +// +// The Back path cannot otherwise tell its two cases apart. A view the popup +// never rendered is still the blank template from index.html and has to be +// rendered; a view already on the page must NOT be rendered again, because +// a second render re-fetches and overwrites whatever the user has typed +// into it and not yet saved. +// +// Registration is showView() in views/helpers.js, which is the last thing +// every render path runs — restoreView()'s, the Back path's, and every +// forward show(). That is the point of putting it there rather than in the +// individual views: a view added later registers itself with no one having +// to remember it, so this cannot decay. +// +// Module scope is page-load scope: the popup loads this module once per +// page load, and a reopened popup gets a fresh, empty set — which is +// exactly the state that makes the Back path render. +const renderedViews = new Set(); + +function markViewRendered(view) { + if (view) renderedViews.add(view); +} + +// Begin a fresh page-load scope. The popup gets one by being loaded; the +// unit tests, which simulate several page loads against one module +// instance, ask for one. +function resetRenderedViews() { + renderedViews.clear(); +} + +// Home is the exception: Back re-renders it every time, which is what the +// popup did before this router existed (index.js registered +// renderWalletList() as setRenderMain(), and goBack() called it on every +// Back onto "main"). It must stay that way — the wallet list has to reflect +// what changed while the user was away from it, such as a wallet renamed or +// an address removed in Settings — and Home holds no unsaved input to lose. +const ALWAYS_RENDER_ON_BACK = new Set(["main"]); + +// Views that render an address the user picked and cannot be rendered +// without one. +const ADDRESS_VIEWS = new Set([ + "address", + "address-token", + "receive", + "transaction", +]); + +function needsAddress(view) { + return ADDRESS_VIEWS.has(view); +} + +function hasValidAddress(state) { + return Boolean( + state.selectedWallet !== null && + state.selectedAddress !== null && + state.wallets[state.selectedWallet] && + state.wallets[state.selectedWallet].addresses[state.selectedAddress], + ); +} + +// Render `view` from persisted state. Each view module shows itself, so a +// true return means the view is both rendered and on screen. +// +// Returns false when the view is not one the popup renders from state, or +// when the state it would render is gone — a token no longer selected, a +// transaction no longer persisted. The caller falls back rather than +// putting an empty template on screen. +function renderView(view, state, views) { + if (!view || !RESTORABLE_VIEWS.has(view)) return false; + if (needsAddress(view) && !hasValidAddress(state)) return false; + if (view === "address-token" && !state.selectedToken) return false; + + const data = state.viewData || {}; + switch (view) { + case "main": + views.main.show(); + return true; + case "address": + views.addressDetail.show(); + return true; + case "address-token": + views.addressToken.show(); + return true; + case "receive": + views.receive.show(); + return true; + case "settings": + views.settings.show(); + return true; + case "settings-addtoken": + views.settingsAddToken.show(); + return true; + case "confirm-tx": + if (!data.pendingTx) return false; + views.confirmTx.restore(); + return true; + case "transaction": + if (!data.tx) return false; + views.transactionDetail.render(); + return true; + case "wait-tx": + // Resumes the receipt poll from the persisted broadcast time, + // and answers false when there is nothing resumable left. + return Boolean(views.txStatus.restoreWait()); + case "success-tx": + if (!data.hash) return false; + views.txStatus.renderSuccess(); + return true; + case "error-tx": + if (!data.message) return false; + views.txStatus.renderError(); + return true; + default: + return false; + } +} + +// The Back-path renderer, registered with setBackRenderer() in +// views/helpers.js. +// +// Returns false — leaving goBack() to unhide the view, as it always did — +// in the two cases where the view is known to be on the page already: +// +// - It is not one the popup renders from persisted state. The restored +// stack is filtered against RESTORABLE_VIEWS, so such a view can only +// be on the stack from this page load, where forward navigation +// rendered it on the way in. +// - This page load has rendered it. Re-rendering would re-fetch and +// clobber what it holds; Home is rendered anyway, see above. +// +// What is left is the case the router exists for: a view on the stack that +// this page load has never rendered, whose template is still blank. +function makeBackRenderer(state, views) { + return function renderBack(view) { + if (!RESTORABLE_VIEWS.has(view)) return false; + if (renderedViews.has(view) && !ALWAYS_RENDER_ON_BACK.has(view)) { + return false; + } + if (!renderView(view, state, views)) { + views.main.show(); + } + return true; + }; +} + +module.exports = { + renderView, + makeBackRenderer, + markViewRendered, + resetRenderedViews, +}; diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index ab8468a..d16885d 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -7,6 +7,7 @@ const { getAddressValueUsd, } = require("../../shared/prices"); const { state, saveState, currentNetwork } = require("../../shared/state"); +const { markViewRendered } = require("../viewRouter"); // When views are added, removed, or transitions between them change, // update the view-navigation documentation in README.md to match. @@ -76,6 +77,10 @@ function showView(name) { } clearFlash(); state.currentView = name; + // A view's show() ends here, so this is where the Back path learns the + // view is no longer the blank template from index.html and must not be + // rendered a second time. See viewRouter.js. + markViewRendered(name); saveState(); updateDebugBanner(name); } @@ -111,12 +116,19 @@ function updateDebugBanner(viewName) { } } -// Callback to re-render the main/home view when navigating back to it. -// Set once by index.js via setRenderMain(). -let _renderMain = null; +// Callback that renders a view being navigated BACK onto. Set once by +// index.js via setBackRenderer(), which routes the view through the same +// per-view render and data guards restoreView() uses. +// +// It answers true when it took the navigation — the view is rendered and +// shown, or its backing data was gone and it fell back — and false for a +// view the popup does not render from persisted state. Those can only be +// on the stack from this page load, because the stack is filtered on load, +// so they have already been rendered and only need unhiding. +let _renderBack = null; -function setRenderMain(fn) { - _renderMain = fn; +function setBackRenderer(fn) { + _renderBack = fn; } // Push the current view onto the navigation stack so goBack() can @@ -136,9 +148,11 @@ function goBack() { } else { target = "main"; } - if (target === "main" && _renderMain) { - _renderMain(); - } + // A popped view is landed on, not navigated to. If the popup has been + // closed and reopened since the view was pushed, nothing has ever + // rendered it in this page load and its template is still blank, so it + // has to be rendered here rather than merely unhidden. + if (_renderBack && _renderBack(target)) return; showView(target); } @@ -470,7 +484,7 @@ module.exports = { showView, onViewLeave, updateDebugBanner, - setRenderMain, + setBackRenderer, pushCurrentView, goBack, clearViewStack, diff --git a/tests/backNavigation.test.js b/tests/backNavigation.test.js new file mode 100644 index 0000000..edcaae7 --- /dev/null +++ b/tests/backNavigation.test.js @@ -0,0 +1,329 @@ +// Back after reopening the popup (#268). +// +// A reopened popup renders the wallet list and the one view it restores +// onto; every other view is still the blank static template from +// index.html. goBack() used to only unhide its target, so Back landed on +// that blank template for any view the popup had not rendered in this page +// load. These tests drive the real goBack() with the real router wired to +// recording view modules, so what is asserted is which view render ran — +// the thing that was missing. +// +// The rendering itself is asserted against the real popup in a real +// browser by tests/e2e/run.js; here the DOM is a stub, because goBack() +// only needs showView() to work. + +const els = new Map(); + +function fakeEl() { + return { + textContent: "", + innerHTML: "", + classList: { + toggle() {}, + add() {}, + remove() {}, + contains: () => false, + }, + remove() {}, + }; +} + +globalThis.document = { + getElementById(id) { + if (!els.has(id)) els.set(id, fakeEl()); + return els.get(id); + }, +}; + +// helpers.js pulls in state.js, which reads chrome.storage.local at load. +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { + showView, + goBack, + setBackRenderer, + pushCurrentView, +} = require("../src/popup/views/helpers"); +const { + makeBackRenderer, + markViewRendered, + resetRenderedViews, +} = require("../src/popup/viewRouter"); +const { state } = require("../src/shared/state"); + +const ADDRESS = "0x1111111111111111111111111111111111111111"; +const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"; + +let calls; + +// Stand-ins for the view modules. Each records itself and then shows its +// view, which is what every real view render ends with — so the assertions +// can tell "rendered and shown" apart from "merely unhidden". +function recorder(name, view) { + return () => { + calls.push(name); + showView(view); + }; +} + +function makeViews() { + return { + main: { show: recorder("main", "main") }, + addressDetail: { show: recorder("addressDetail", "address") }, + addressToken: { show: recorder("addressToken", "address-token") }, + receive: { show: recorder("receive", "receive") }, + settings: { show: recorder("settings", "settings") }, + settingsAddToken: { + show: recorder("settingsAddToken", "settings-addtoken"), + }, + confirmTx: { restore: recorder("confirmTx", "confirm-tx") }, + transactionDetail: { + render: recorder("transactionDetail", "transaction"), + }, + txStatus: { + restoreWait: () => { + calls.push("waitTx"); + showView("wait-tx"); + return true; + }, + renderSuccess: recorder("successTx", "success-tx"), + renderError: recorder("errorTx", "error-tx"), + }, + }; +} + +// The popup as it stands just after a reopen: one wallet with one address, +// the view the popup restored onto, and the stack behind it. +// +// A reopen is a fresh page load, so the record of what has been rendered +// starts empty — that emptiness is what makes the Back path render at all. +// Returns the view modules so a test can drive forward navigation through +// the same recorders the router renders through. +function reopenedOn(view, stack, extra) { + calls = []; + resetRenderedViews(); + state.wallets = [ + { + name: "Wallet 1", + addresses: [{ address: ADDRESS, balance: "0", tokenBalances: [] }], + }, + ]; + state.selectedWallet = 0; + state.selectedAddress = 0; + state.selectedToken = null; + state.viewData = null; + state.currentView = view; + state.viewStack = stack.slice(); + Object.assign(state, extra || {}); + // Restoring onto a view renders it, so the reopened popup has that one + // view on the page and nothing else. + markViewRendered(view); + const views = makeViews(); + setBackRenderer(makeBackRenderer(state, views)); + return views; +} + +// The reproduction from the issue, step for step. +describe("Back onto a view the reopened popup never rendered", () => { + test("Back from settings renders the address detail underneath", () => { + reopenedOn("settings", ["main", "address"]); + goBack(); + expect(calls).toEqual(["addressDetail"]); + expect(state.currentView).toBe("address"); + expect(state.viewStack).toEqual(["main"]); + }); + + test("Back onto the token detail renders it", () => { + reopenedOn("settings", ["main", "address", "address-token"], { + selectedToken: TOKEN, + }); + goBack(); + expect(calls).toEqual(["addressToken"]); + expect(state.currentView).toBe("address-token"); + }); + + test("Back onto Receive renders it", () => { + reopenedOn("settings", ["main", "address", "receive"]); + goBack(); + expect(calls).toEqual(["receive"]); + expect(state.currentView).toBe("receive"); + }); + + test("Back onto the transaction detail renders it", () => { + reopenedOn("settings", ["main", "transaction"], { + viewData: { tx: { hash: "0xdead" } }, + }); + goBack(); + expect(calls).toEqual(["transactionDetail"]); + expect(state.currentView).toBe("transaction"); + }); + + test("Back onto the transaction confirmation restores it", () => { + reopenedOn("settings", ["main", "confirm-tx"], { + viewData: { pendingTx: { to: ADDRESS, amount: "1" } }, + }); + goBack(); + expect(calls).toEqual(["confirmTx"]); + expect(state.currentView).toBe("confirm-tx"); + }); + + test("Back onto the success screen renders it", () => { + reopenedOn("settings", ["main", "success-tx"], { + viewData: { hash: "0xdead" }, + }); + goBack(); + expect(calls).toEqual(["successTx"]); + expect(state.currentView).toBe("success-tx"); + }); + + test("Back onto the failure screen renders it", () => { + reopenedOn("settings", ["main", "error-tx"], { + viewData: { message: "execution reverted" }, + }); + goBack(); + expect(calls).toEqual(["errorTx"]); + expect(state.currentView).toBe("error-tx"); + }); + + test("Back onto Home renders the wallet list", () => { + reopenedOn("settings", ["main"]); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("Back with an empty stack renders Home", () => { + reopenedOn("settings", []); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); +}); + +// The guards are restoreView()'s, so a popped view whose backing data is +// gone lands on Home rather than on an empty template. +describe("Back onto a view whose backing data is gone", () => { + test("the token detail with no token selected falls back to Home", () => { + reopenedOn("settings", ["main", "address-token"]); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("the transaction detail with no transaction falls back to Home", () => { + reopenedOn("settings", ["main", "transaction"]); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("the confirmation with no pending transaction falls back to Home", () => { + reopenedOn("settings", ["main", "confirm-tx"]); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("an address view with no address selected falls back to Home", () => { + reopenedOn("settings", ["main", "receive"], { + selectedAddress: null, + }); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("the success screen with no transaction hash falls back to Home", () => { + reopenedOn("settings", ["main", "success-tx"]); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("the failure screen with no message falls back to Home", () => { + reopenedOn("settings", ["main", "error-tx"]); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); + + test("a wait that can no longer be resumed falls back to Home", () => { + reopenedOn("settings", ["main", "wait-tx"]); + const views = makeViews(); + views.txStatus.restoreWait = () => false; + setBackRenderer(makeBackRenderer(state, views)); + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); +}); + +// Forward navigation renders as it goes, and a second render would re-fetch +// and clobber whatever the view holds — an unsaved edit, a request in +// flight. So the Back path renders only a view this page load has never +// rendered, and merely unhides every other one: the views it does not +// render from persisted state, and the views already on the page. +describe("what the Back path leaves alone", () => { + test("forward navigation renders nothing by itself", () => { + reopenedOn("address", ["main"]); + pushCurrentView(); + showView("send"); + expect(calls).toEqual([]); + expect(state.viewStack).toEqual(["main", "address"]); + }); + + test("Back onto a live-session view only unhides it", () => { + reopenedOn("confirm-tx", ["main", "address", "send"]); + goBack(); + expect(calls).toEqual([]); + expect(state.currentView).toBe("send"); + }); + + test("Back renders its target exactly once", () => { + reopenedOn("settings", ["main", "address"]); + goBack(); + expect(calls.filter((c) => c === "addressDetail")).toHaveLength(1); + }); + + test("Back onto a view this page load already rendered only unhides it", () => { + const views = reopenedOn("main", []); + pushCurrentView(); + views.addressDetail.show(); + pushCurrentView(); + views.settings.show(); + calls = []; + goBack(); + expect(calls).toEqual([]); + expect(state.currentView).toBe("address"); + }); + + // The unit mirror of the regression the browser suite pins: Settings + // reassigns its fields from persisted state on every render, so a + // re-render on the way back discards an edit the user has not saved. + test("Back onto Settings visited earlier in this page load does not re-render it", () => { + const views = reopenedOn("main", []); + pushCurrentView(); + views.settings.show(); + pushCurrentView(); + views.settingsAddToken.show(); + calls = []; + goBack(); + expect(calls).toEqual([]); + expect(state.currentView).toBe("settings"); + }); + + // Home is the deliberate exception, unchanged from the popup's + // behaviour before the router existed: it re-renders on every Back so + // the wallet list reflects what changed while the user was away. + test("Back onto Home renders it again even when it is already on the page", () => { + const views = reopenedOn("main", []); + pushCurrentView(); + views.addressDetail.show(); + calls = []; + goBack(); + expect(calls).toEqual(["main"]); + expect(state.currentView).toBe("main"); + }); +}); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 409d198..3998316 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -419,6 +419,172 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env) assertWiped(st, env.phrase, "after reopening the popup"); }); +// ------------------------------- Back after reopening the popup (#268) + +// A reopened popup renders the wallet list and the view it restores onto, +// and nothing else: every other screen is still the blank static template +// from index.html. Back used to only unhide its target, which is why these +// have to run against the real popup — the template is present and +// well-formed, so only its emptiness distinguishes the defect, and only a +// real reopen produces it. + +// Everything the address screen must have on it, read out of the DOM. +function addressScreenState(page) { + return page.evaluate(() => { + const line = document.getElementById("address-line"); + const balances = document.getElementById("address-balances"); + return { + hidden: document + .getElementById("view-address") + .classList.contains("hidden"), + line: line ? line.innerText.trim() : "", + balances: balances ? balances.innerText.trim() : "", + }; + }); +} + +// Close and reopen the page rather than reload it: that is what the toolbar +// popup does, and it is the only thing that produces the unrendered views. +async function reopenPopup(env, restoredView) { + await env.page.close(); + env.page = await openPopup(env.ctx, env.popupUrl); + await visible(env.page, restoredView); +} + +// The reproduction from the issue, step for step. +test("Back after reopening the popup renders the address screen (#268)", async (env) => { + await openAddressDetail(env.page); + const before = await addressScreenState(env.page); + assert( + before.line.length > 0, + "the address screen was blank to begin with", + ); + + await env.page.click("#btn-settings"); + await visible(env.page, "#view-settings"); + + await reopenPopup(env, "#view-settings"); + + await env.page.click("#btn-settings-back"); + await visible(env.page, "#view-address"); + + const after = await addressScreenState(env.page); + assert( + after.line === before.line, + "the address line reads " + + JSON.stringify(after.line) + + ", expected " + + JSON.stringify(before.line), + ); + assert( + after.balances.includes("ETH"), + "the balances read " + JSON.stringify(after.balances), + ); +}); + +// The same defect one screen further in. Receive holds the address twice +// over — as text and as the QR code the sender scans — and a blank one is +// worse than a missing screen. +// Everything the Receive screen must have on it. The QR code is read as +// pixels, not as an element: the blank template carries the canvas too, a +// default 300x150 one with nothing drawn on it and every pixel fully +// transparent. A drawn QR paints an opaque background across the whole +// canvas, so a single opaque pixel is the whole question. +function receiveScreenState(page) { + return page.evaluate(() => { + const block = document.getElementById("receive-address-block"); + const canvas = document.getElementById("receive-qr"); + const px = canvas + .getContext("2d") + .getImageData(0, 0, canvas.width, canvas.height).data; + let opaque = 0; + for (let i = 3; i < px.length; i += 4) { + if (px[i] > 0) opaque += 1; + } + return { + address: block.dataset.full || "", + text: block.innerText.trim(), + qrOpaquePixels: opaque, + }; + }); +} + +test("Back after reopening the popup renders the Receive screen (#268)", async (env) => { + await openAddressDetail(env.page); + await env.page.click("#btn-receive"); + await visible(env.page, "#view-receive"); + const before = await receiveScreenState(env.page); + assert( + /^0x[0-9a-fA-F]{40}$/.test(before.address), + "Receive showed no address to begin with: " + + JSON.stringify(before.address), + ); + + await env.page.click("#btn-settings"); + await visible(env.page, "#view-settings"); + + await reopenPopup(env, "#view-settings"); + + await env.page.click("#btn-settings-back"); + await visible(env.page, "#view-receive"); + + const shown = await receiveScreenState(env.page); + assert( + shown.address === before.address, + "Receive shows " + + JSON.stringify(shown.address) + + ", expected " + + JSON.stringify(before.address), + ); + assert( + shown.text.includes(before.address), + "the Receive address is not on screen: " + JSON.stringify(shown.text), + ); + assert(shown.qrOpaquePixels > 0, "Receive shows an unpainted QR code"); + + // Leave the suite where it found it. + await env.page.click("#btn-receive-back"); + await visible(env.page, "#view-address"); + await env.page.click("#btn-address-back"); + await visible(env.page, "#view-main"); +}); + +// The other half of the requirement: Back renders a screen this page load +// never rendered, and must NOT re-render one it already has on screen. +// settings.show() reassigns #settings-rpc from persisted state, so +// re-rendering Settings on the way back would silently revert whatever the +// user typed and had not saved yet — and they could then press Save and +// store the value they believed they had replaced. No reopen here: this is +// an ordinary in-session forward-and-back, which is exactly why the render +// must not happen. +test("Back onto Settings keeps unsaved input (#268)", async (env) => { + await visible(env.page, "#view-main"); + await env.page.click("#btn-settings"); + await visible(env.page, "#view-settings"); + + const typed = "https://rpc.example.invalid/unsaved"; + await env.page.fill("#settings-rpc", typed); + + await env.page.click("#btn-settings-add-token"); + await visible(env.page, "#view-settings-addtoken"); + await env.page.click("#btn-settings-addtoken-back"); + await visible(env.page, "#view-settings"); + + const kept = await env.page.inputValue("#settings-rpc"); + assert( + kept === typed, + "the unsaved RPC URL reads " + + JSON.stringify(kept) + + ", expected " + + JSON.stringify(typed), + ); + + // Leave the suite where it found it. The typed value was never saved, + // and Settings reloads the field from state next time it renders. + await env.page.click("#btn-settings-back"); + await visible(env.page, "#view-main"); +}); + // -------------------------------------------- address removal (#162) // Number of address rows across every wallet in the list, counted in the DOM