diff --git a/README.md b/README.md index 58ba78c..c918f89 100644 --- a/README.md +++ b/README.md @@ -1468,6 +1468,9 @@ view would leave a wallet one click from deletion. - "Reveal" (wrong password) → full-sentence error on the error line, nothing revealed (no screen change) - "Back" → previous screen (AddressDetail) + - Settings gear → **Settings**, whose "Back" goes to AddressDetail: leaving + drops the address the screen was showing, so it also takes the screen off + the Back stack - **Secret handling**: nothing is decrypted, no key is derived, and nothing is written into the page until the password is accepted; the key is never stored in state, and it is wiped from the page whenever the screen is left by any @@ -1795,6 +1798,9 @@ view would leave a wallet one click from deletion. - "Reveal" (wrong password) → full-sentence error, nothing revealed (no screen change) - "Back" → previous screen (Settings) + - Settings gear → **Settings**, whose "Back" never lands back on this + screen: leaving drops the wallet the screen was showing, so it also takes + the screen off the Back stack - **Secret handling**: nothing is decrypted or written into the page until the password is accepted; the phrase is never stored in state, and it is wiped from the page whenever the screen is left by any route, including the Settings diff --git a/TODO.md b/TODO.md index 646cb03..a14785d 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,15 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-06: Back from Settings no longer lands on the private key export or + recovery phrase screen after either was left by the settings gear + ([#461](https://git.eeqj.de/sneak/AutistMask/issues/461)). Leaving drops the + screen's selection, so its leave handler now also takes it off the Back stack, + as a reopened popup already does. `tests/exportPrivkey.test.js` and + `tests/showPhrase.test.js` drive the gear and then Back, and + `leavePrivkeyScreen()` in `tests/e2e/run.js` expects the address screen after + Settings. + - 2026-10-06: The private key export screen opens again in the same popup session ([#460](https://git.eeqj.de/sneak/AutistMask/issues/460)). `show()` found the address line through the element inside it, which its own rendering diff --git a/src/popup/views/exportPrivkey.js b/src/popup/views/exportPrivkey.js index 5daa7ed..b027c13 100644 --- a/src/popup/views/exportPrivkey.js +++ b/src/popup/views/exportPrivkey.js @@ -152,7 +152,16 @@ async function reveal() { } function init() { - onViewLeave(VIEW, clear); + // Leaving drops the address selection, so the screen also comes off the + // Back stack, where the settings gear has just put it: Back from Settings + // must not land on a password prompt that can only fail. A reopened popup + // drops it from the stack the same way + // (https://git.eeqj.de/sneak/AutistMask/issues/461). + onViewLeave(VIEW, () => { + clear(); + const stack = state.viewStack; + if (stack[stack.length - 1] === VIEW) stack.pop(); + }); // No wipe here: goBack() routes through showView(), which runs the // leave hook. A per-button wipe would only cover this one path. diff --git a/src/popup/views/showPhrase.js b/src/popup/views/showPhrase.js index d8962c2..839cf67 100644 --- a/src/popup/views/showPhrase.js +++ b/src/popup/views/showPhrase.js @@ -134,7 +134,16 @@ async function reveal() { } function init() { - onViewLeave(VIEW, clear); + // Leaving drops the wallet selection, so the screen also comes off the + // Back stack, where the settings gear has just put it: Back from Settings + // must not land on a password prompt that can only fail. A reopened popup + // drops it from the stack the same way + // (https://git.eeqj.de/sneak/AutistMask/issues/461). + onViewLeave(VIEW, () => { + clear(); + const stack = state.viewStack; + if (stack[stack.length - 1] === VIEW) stack.pop(); + }); $("btn-show-phrase-back").addEventListener("click", () => { goBack(); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 47f76a0..5951dfc 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -1037,13 +1037,13 @@ async function revealPrivkey(page) { } // Leave the export screen, or the Settings screen the gear left it for, for -// Home. The gear put the export screen on the Back stack, so from Settings the -// way home passes through it, already emptied +// Home. Leaving takes the export screen off the Back stack, so from Settings +// Back goes to the address screen it was opened from // (https://git.eeqj.de/sneak/AutistMask/issues/461). async function leavePrivkeyScreen(page) { if (await page.isVisible("#view-settings")) { await page.click("#btn-settings-back"); - await visible(page, "#view-export-privkey"); + await visible(page, "#view-address"); } if (await page.isVisible("#view-export-privkey")) { await page.click("#btn-export-privkey-back"); diff --git a/tests/exportPrivkey.test.js b/tests/exportPrivkey.test.js index 48eb0a3..8629431 100644 --- a/tests/exportPrivkey.test.js +++ b/tests/exportPrivkey.test.js @@ -336,6 +336,36 @@ describe("opening the screen again in the same popup session", () => { }); }); +describe("Back from Settings after leaving by the settings gear", () => { + // https://git.eeqj.de/sneak/AutistMask/issues/461: leaving drops the + // address selection, so Back onto this screen showed a password prompt + // that could only answer "No address is selected." + test("goes to the address screen it was opened from", () => { + const { helpers, state, exportPrivkey } = load(); + state.viewStack = ["main"]; + exportPrivkey.show(0, 0); + // The settings gear: push the current view, then show Settings. + helpers.pushCurrentView(); + helpers.showView("settings"); + + helpers.goBack(); + + expect(state.currentView).toBe("address"); + expect(state.viewStack).toEqual(["main"]); + }); + + test("its own Back button leaves the rest of the stack alone", async () => { + const { state, exportPrivkey } = load(); + state.viewStack = ["main"]; + exportPrivkey.show(0, 0); + + await click("btn-export-privkey-back"); + + expect(state.currentView).toBe("address"); + expect(state.viewStack).toEqual(["main"]); + }); +}); + describe("views the popup may reopen onto", () => { // Restoring onto this screen would put a private key on display with no // password prompt in front of it, on a popup reopened by accident. diff --git a/tests/showPhrase.test.js b/tests/showPhrase.test.js index 4a76499..db675c8 100644 --- a/tests/showPhrase.test.js +++ b/tests/showPhrase.test.js @@ -1,12 +1,17 @@ // Tests for the recovery phrase display (issue #161). // -// These cover the parts that do not need a DOM: which wallet types may be -// offered the action at all, the exclusion of the screen from the set of -// views the popup may reopen onto, and the absence of any path from this -// module to the logger. The DOM behaviour it guards — nothing rendered -// before the password is accepted, a wrong password revealing nothing, and -// the wipe on leaving — is driven against the real popup in a real browser -// by tests/e2e/run.js, which is where every other view behaviour is tested. +// These cover which wallet types may be offered the action at all, the +// exclusion of the screen from the set of views the popup may reopen onto, +// the absence of any path from this module to the logger, and, against a +// minimal DOM stub, where Back goes after the screen is left by the settings +// gear. The rest of the DOM behaviour it guards — nothing rendered before the +// password is accepted, a wrong password revealing nothing, and the wipe on +// leaving — is driven against the real popup in a real browser by +// tests/e2e/run.js, which is where every other view behaviour is tested. + +jest.mock("../src/shared/vault", () => ({ + decryptWithPassword: jest.fn(), +})); const fs = require("fs"); const path = require("path"); @@ -74,6 +79,64 @@ describe("views the popup may reopen onto", () => { }); }); +// Just enough document for helpers.showView() and this view: every element +// is made on first lookup and keeps what the view writes to it. +function makeDocument() { + const els = new Map(); + function makeElement() { + const classes = new Set(); + return { + textContent: "", + value: "", + style: {}, + classList: { + add: (name) => classes.add(name), + remove: (name) => classes.delete(name), + contains: (name) => classes.has(name), + toggle: (name, on) => + on ? classes.add(name) : classes.delete(name), + }, + addEventListener: () => {}, + }; + } + return { + getElementById(id) { + // Created on demand by helpers.js; absent on a mainnet popup + // that is not a debug build. + if (id === "debug-banner") return null; + if (!els.has(id)) els.set(id, makeElement()); + return els.get(id); + }, + }; +} + +describe("Back from Settings after leaving by the settings gear", () => { + // https://git.eeqj.de/sneak/AutistMask/issues/461: leaving drops the + // wallet selection, so Back onto this screen showed a password prompt + // that could only answer "No wallet is selected." + test("does not land on the recovery phrase screen", () => { + jest.resetModules(); + globalThis.document = makeDocument(); + const helpers = loadHelpers(); + const { state } = require("../src/shared/state"); + const showPhrase = require("../src/popup/views/showPhrase"); + showPhrase.init(); + state.wallets = [{ name: "Wallet 1", type: "hd", addresses: [] }]; + state.currentView = "settings"; + state.viewStack = ["main"]; + + // Opened from the wallet list in Settings, then left by the gear: + // push the current view, then show Settings. + showPhrase.show(0); + helpers.pushCurrentView(); + helpers.showView("settings"); + + expect(state.viewStack).toEqual(["main", "settings"]); + helpers.goBack(); + expect(state.currentView).not.toBe(SHOW_PHRASE_VIEW); + }); +}); + describe("the phrase cannot reach the logger", () => { const source = fs.readFileSync( path.join(__dirname, "..", "src", "popup", "views", "showPhrase.js"),