docs: README end-to-end limits match what the browser suites do (closes #293)

The window.close override the issue named was removed with #275, so it
needs no entry. Every other place either suite changes or works around
the shipped extension is now listed: the popup loaded in a tab, Chrome's
wait before each site-connection request, its recording wrapper and click
listener, its clipboard grant, two layout tests that write into the page,
the forced leave during a decrypt, and Firefox's setting that forces the
site-connection prompt into a window. Firefox's tolerated error is
described as the leftover it is, with #487 to remove it; the phishing
blocklist is no longer listed as a failing fetch, and two stale figures
are corrected.

Model: opus-5-5

README.md

@@ -334,7 +334,9 @@ There are two suites, one per browser, and they share no code. Chrome runs on
 Playwright; Firefox has its own WebDriver client, because Playwright cannot
 observe errors on a Firefox extension page at all — see
 [Firefox](#firefox-make-test-e2e-firefox) below. Both require docker, and both
-are outside `make check`.
+are outside `make check`. Neither opens the popup from the toolbar button: both
+load its page in an ordinary tab, so what the toolbar popup itself adds, its
+size and its closing when it loses focus, is covered by neither.
 
 ### Chrome (`make test-e2e`)
 
@@ -420,14 +422,36 @@ required to be present, so that check cannot pass by observing nothing. That
 last one is the standing floor under
 [#157](https://git.eeqj.de/sneak/AutistMask/issues/157).
 
-Two limits of that coverage, neither of them papered over. The RPC is stubbed
-throughout, so this is **not** a real dApp against a real network with real
-funds; that remains a human pass before 1.0.0. The site-connection prompt is
-raised through `chrome.action.openPopup()`, and headless Chromium's
-browser-action popup is not a page Playwright can see or click, so that one
-prompt is driven at the URL the extension itself puts on the action — the same
-page and the same approval id, but whether a real toolbar click shows it is not
-observable here.
+The limits of that coverage and of the rest of the Chrome suite, none of them
+papered over:
+
+- The RPC is stubbed throughout, so this is **not** a real dApp against a real
+  network with real funds; that remains a human pass before 1.0.0.
+- The site-connection prompt is raised through `chrome.action.openPopup()`, and
+  headless Chromium's browser-action popup is not a page Playwright can see or
+  click, so that one prompt is driven at the URL the extension itself puts on
+  the action — the same page and the same approval id, but whether a real
+  toolbar click shows it is not observable here.
+- Each site-connection request is made 1.5 seconds after the tab it will be
+  driven in is opened (`APPROVAL_TAB_SETTLE_MS` in `tests/e2e/run.js`), because
+  opening that tab closes the previous prompt's toolbar popup; a request made
+  just after that popup closes is not covered.
+- In the tests that approve a signature or a transaction, the approval window
+  runs with `chrome.runtime.sendMessage` wrapped to record what it sends, and in
+  the tests that reject a site connection the prompt gets a click listener that
+  records that Reject was pressed; neither changes what the window does.
+- The tap to copy test first grants clipboard permission to every page in the
+  browser, which the manifest does not ask for, so whether a real popup may
+  write to the clipboard on a click alone is not covered.
+- The layout tests for an over-long flash message and for the password error
+  lines write the text straight into the page instead of letting the popup's
+  code put it there, and the second brings each screen up by toggling its
+  `hidden` class rather than navigating to it; they cover the layout, not the
+  code that fills it.
+- Leaving the recovery phrase or private key screen while its decrypt runs is
+  forced by clicking Reveal and the settings gear in one page task, which a
+  person cannot do; the case a person can hit, the first decrypt after the popup
+  opens while libsodium is still loading, is not driven.
 
 Any test that drives a failure path on purpose declares the `console.error` it
 is about to provoke, via `errors.expect()`. That is not a mute: the declaration
@@ -441,8 +465,8 @@ collecting for a fixed grace period after the last test returns
 the context. A request whose _first_ dispatch falls after that window is never
 seen at all and cannot fail the run. In practice a request a test fires without
 awaiting reaches the route handler about 10ms later, and anything on a repeating
-timer gets observed on an earlier tick during the ~20s suite — but a one-shot
-call deliberately deferred past the window will escape.
+timer gets observed on an earlier tick during the suite — but a one-shot call
+deliberately deferred past the window will escape.
 
 That interception covers the MV3 background service worker as well as the popup
 page, which it does not by default — `script/test-e2e` sets
@@ -574,25 +598,26 @@ without an `await`. Demonstrated, not assumed: a `throw` placed past the first
 and on Chrome (`pageerror`), with the rest of the run unaffected because the
 approval view had already rendered.
 
-One error is tolerated rather than fatal, listed in `ALLOWED_ERRORS` in
-`tests/e2e/firefox/run.js` with the issue that will delete it, and printed on
-every occurrence so the concession stays visible in the run output. It is
-Firefox reporting the site-approval popup's unawaited `sendMessage` settling
-after `window.close()` unloaded the context — the same teardown ordering as
-[#275](https://git.eeqj.de/sneak/AutistMask/issues/275), and unsuppressable from
-the calling code, because `BaseContext.wrapPromise` reports it whether or not a
-handler is attached. Errors are read from the privileged `nsIConsoleService` in
-Marionette's chrome context and filtered to non-warning entries whose
-`sourceName` is the extension origin. That mechanism is not a stylistic choice.
-WebDriver BiDi's `log.entryAdded` delivers **nothing** for extension pages: on a
-plain `http://` page it reports uncaught errors with stack traces, and on the
-`moz-extension://` popup it reports zero events, because Firefox's remote agent
-excludes extension browsing contexts from BiDi observation. Any harness built on
-Playwright-BiDi or Puppeteer-BiDi would therefore see nothing and report
-success, which is exactly the vacuous check this repo has already shipped twice.
-Do not migrate this suite to BiDi.
+One error is still tolerated rather than fatal, listed in `ALLOWED_ERRORS` in
+`tests/e2e/firefox/run.js` and printed on every occurrence so the concession
+stays visible in the run output: Firefox reporting an extension promise that
+settled after its page unloaded, from anywhere in the popup. Its cause was the
+site-approval popup's unawaited `sendMessage` before `window.close()`, which
+[#275](https://git.eeqj.de/sneak/AutistMask/issues/275) removed; Firefox runs
+since have not printed it, and
+[#487](https://git.eeqj.de/sneak/AutistMask/issues/487) removes the entry.
+Errors are read from the privileged `nsIConsoleService` in Marionette's chrome
+context and filtered to non-warning entries whose `sourceName` is the extension
+origin. That mechanism is not a stylistic choice. WebDriver BiDi's
+`log.entryAdded` delivers **nothing** for extension pages: on a plain `http://`
+page it reports uncaught errors with stack traces, and on the `moz-extension://`
+popup it reports zero events, because Firefox's remote agent excludes extension
+browsing contexts from BiDi observation. Any harness built on Playwright-BiDi or
+Puppeteer-BiDi would therefore see nothing and report success, which is exactly
+the vacuous check this repo has already shipped twice. Do not migrate this suite
+to BiDi.
 
-Two limits are worth knowing, both real differences from the Chrome suite:
+Three limits are worth knowing, all real differences from the Chrome suite:
 
 - **Error capture is poll-based, not event-streamed.** The console is drained at
   each step boundary, so an error is attributed to the step it was drained
@@ -609,24 +634,30 @@ Two limits are worth knowing, both real differences from the Chrome suite:
   and silently evicts the oldest, so more than 250 console messages between two
   drains destroys the excess unread. 400 throws inside one step are reported as
   exactly the newest 250, three runs running. That buffer is shared with
-  Firefox's own console noise; a clean run peaks at 4 of 250 at the install
-  drain and 0 at every later drain, so the three steps here have wide headroom,
-  but a step that logs heavily could evict unread errors. What poll-based costs
-  is location, not coverage: an error cannot be placed within a step the way the
-  Chrome suite's `pageerror` events place it.
+  Firefox's own console noise; a clean run, measured when the suite had three
+  steps (popup load, wallet creation and Add Token), peaked at 4 of 250 at the
+  install drain and 0 at every later drain, but a step that logs heavily could
+  evict unread errors. What poll-based costs is location, not coverage: an error
+  cannot be placed within a step the way the Chrome suite's `pageerror` events
+  place it.
 - **Almost nothing is stubbed, which inverts the coverage of network-dependent
   code.** The container still runs with `--network none`, so the run is offline
   and no request can escape. The one thing it can reach is the loopback fixture
   in `tests/e2e/firefox/dapp.js`, which serves the dApp page and a JSON-RPC node
   and which the extension's `rpcUrl` is pointed at for the dApp steps; a
   JSON-RPC method that fixture does not model fails the run rather than
-  answering `null`. Everything else — Blockscout, the price feed, the phishing
-  blocklist — has no fixture and simply fails, and the extension swallows its
-  own fetch failures, so only the _failure_ branches of that code are ever
-  executed. A `ReferenceError` in the success path of `renderTransactions`, or
-  of price rendering, passes this suite green. The offline run is also weaker
-  than the Chrome suite's interception for those calls: it proves nothing got
-  out, but it cannot report which requests were attempted.
+  answering `null`. Everything else, Blockscout and the price feed among it, has
+  no fixture and simply fails, and the extension swallows its own fetch
+  failures, so only the _failure_ branches of that code are ever executed. A
+  `ReferenceError` in the success path of `renderTransactions`, or of price
+  rendering, passes this suite green. The offline run is also weaker than the
+  Chrome suite's interception for those calls: it proves nothing got out, but it
+  cannot report which requests were attempted.
+- **The site-connection prompt always opens in a window of its own.** The
+  profile turns off `extensions.openPopupWithoutUserGesture.enabled`, so
+  `src/background/index.js` falls back from the toolbar popup, which WebDriver
+  cannot see, to `windows.create()`; the toolbar popup path is not covered on
+  Firefox.
 
 Neither `make test-e2e` nor `make test-e2e-firefox` is part of `make check` or
 `make test`. `REPO_POLICIES.md` caps `make test` at 60 seconds and a browser
@@ -1283,11 +1314,14 @@ behind a "···" menu.
 
 Navigation uses a stack model (like iOS): each forward action pushes the current
 screen onto `state.viewStack`, and "Back" pops it (`pushCurrentView()` and
-`goBack()` in `src/popup/views/helpers.js`). The root screen is either Welcome
-(no wallets) or Home (has wallets). Each screen below gives its view id in
-parentheses; the registry of view ids is the `VIEWS` array in
-`src/popup/views/helpers.js`, and the markup for a screen is the element with id
-`view-` plus that view id in `src/popup/index.html`.
+`goBack()` in `src/popup/views/helpers.js`). "Back" skips an entry for the
+screen already showing: ShowRecoveryPhrase and the two delete screens take
+themselves off the stack when left, so the Settings gear on one of them leaves
+Settings under Settings, and "Back" from there goes to the screen before
+Settings. The root screen is either Welcome (no wallets) or Home (has wallets).
+Each screen below gives its view id in parentheses; the registry of view ids is
+the `VIEWS` array in `src/popup/views/helpers.js`, and the markup for a screen
+is the element with id `view-` plus that view id in `src/popup/index.html`.
 
 Three elements sit outside the screens and are present on all of them: the title
 bar ("AutistMask by @sneak" plus the Settings gear), the flash message line

TODO.md

@@ -45,6 +45,29 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-10-07: The README's end-to-end limits now match what the two browser
+  suites do to the extension
+  ([#293](https://git.eeqj.de/sneak/AutistMask/issues/293)). The `window.close`
+  override the issue named went with
+  [#275](https://git.eeqj.de/sneak/AutistMask/issues/275), so it needs no entry.
+  Added: both suites load the popup in a tab rather than from the toolbar;
+  Chrome waits 1.5 seconds before each site-connection request, records what
+  approval windows send and click, grants clipboard permission, writes text
+  straight into the page in two layout tests, and forces the leave during a
+  decrypt; Firefox forces the site-connection prompt into a window. Corrected:
+  Firefox's one tolerated error, whose cause that fix removed (the entry goes in
+  [#487](https://git.eeqj.de/sneak/AutistMask/issues/487)), the phishing
+  blocklist the extension no longer fetches, and two stale figures.
+
+- 2026-10-07: Back from Settings no longer shows Settings again after the
+  settings gear was pressed on the recovery phrase or a delete wallet screen
+  opened from Settings, with or without a reopen in between
+  ([#481](https://git.eeqj.de/sneak/AutistMask/issues/481)). Those screens take
+  themselves off the Back stack when left, which leaves Settings under Settings;
+  `goBack()` now skips an entry for the screen already showing.
+  `tests/showPhrase.test.js`, `tests/deleteWalletLostPassword.test.js` and
+  `tests/backNavigation.test.js` drive each path.
+
 - 2026-10-07: `script/discard-dist-on-failure` returns the failed step's own
   exit status even when it cannot write its message, to a closed stderr or to a
   pipe nobody reads any more

src/popup/views/helpers.js

@@ -216,10 +216,21 @@ function pushCurrentView() {
 
 // Pop the navigation stack and show the previous view.  If the stack
 // is empty, fall back to the main (home) view.
+//
+// An entry for the view already showing is skipped: landing on it would
+// make Back seem to do nothing. Settings, the recovery phrase or delete
+// wallet screen, then the gear leaves Settings under Settings, because that
+// screen takes itself off the stack when left, and a reopened popup cuts it
+// off the restored stack the same way
+// (https://git.eeqj.de/sneak/AutistMask/issues/481).
 function goBack() {
+    const stack = state.viewStack;
+    while (stack.length > 0 && stack[stack.length - 1] === state.currentView) {
+        stack.pop();
+    }
     let target;
-    if (state.viewStack.length > 0) {
-        target = state.viewStack.pop();
+    if (stack.length > 0) {
+        target = stack.pop();
     } else {
         target = "main";
     }

tests/backNavigation.test.js

@@ -52,6 +52,7 @@ const {
     resetRenderedViews,
 } = require("../src/popup/viewRouter");
 const { state } = require("../src/shared/state");
+const { restorableStack } = require("../src/shared/persistedState");
 
 const ADDRESS = "0x1111111111111111111111111111111111111111";
 const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
@@ -209,6 +210,23 @@ describe("Back onto a view the reopened popup never rendered", () => {
     });
 });
 
+// https://git.eeqj.de/sneak/AutistMask/issues/481. Settings, the recovery
+// phrase or delete wallet screen, the gear, then a reopen: the restored stack
+// is cut at the screen the gear left, which leaves Settings under the Settings
+// the popup reopens onto.
+describe("Back from Settings reopened over its own entry", () => {
+    test.each(["show-phrase", "delete-wallet-confirm"])(
+        "goes to the screen under it after leaving %s",
+        (left) => {
+            const stored = ["main", "settings", left];
+            reopenedOn("settings", restorableStack(stored, "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", () => {

tests/deleteWalletLostPassword.test.js

@@ -618,9 +618,11 @@ describe("the password route's confirm button", () => {
 
 // https://git.eeqj.de/sneak/AutistMask/issues/480: leaving either delete
 // screen drops its wallet selection, so Back onto one showed a screen whose
-// button could only answer "No wallet selected for deletion."
+// button could only answer "No wallet selected for deletion." Taking the
+// screen off the stack leaves Settings under Settings, and Back must not land
+// there either (https://git.eeqj.de/sneak/AutistMask/issues/481).
 describe("Back from Settings after leaving by the settings gear", () => {
-    test("does not land on the delete screen", () => {
+    test("goes past the delete screen to the screen under Settings", () => {
         const { helpers, deleteWallet, state } = load();
         deleteWallet.show(1);
         // The settings gear: push the current view, then show Settings.
@@ -629,10 +631,10 @@ describe("Back from Settings after leaving by the settings gear", () => {
 
         expect(state.viewStack).toEqual(["main", "settings"]);
         helpers.goBack();
-        expect(state.currentView).not.toBe("delete-wallet-confirm");
+        expect(state.currentView).toBe("main");
     });
 
-    test("does not land on the lost-password screen", async () => {
+    test("goes past the lost-password screen to the screen under Settings", async () => {
         const { helpers, deleteWallet, state } = load();
         await openLostPassword(deleteWallet, 1);
         // The settings gear: push the current view, then show Settings.
@@ -641,7 +643,7 @@ describe("Back from Settings after leaving by the settings gear", () => {
 
         expect(state.viewStack).toEqual(["main", "settings"]);
         helpers.goBack();
-        expect(state.currentView).not.toBe(VIEW);
+        expect(state.currentView).toBe("main");
     });
 
     // The lost-password screen's own Back is "Back returns to the delete

tests/showPhrase.test.js

@@ -131,8 +131,10 @@ 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", () => {
+    // that could only answer "No wallet is selected." Taking the screen off
+    // the stack leaves Settings under Settings, and Back must not land there
+    // either (https://git.eeqj.de/sneak/AutistMask/issues/481).
+    test("goes to the screen under Settings", () => {
         const { helpers, state, showPhrase } = load();
 
         // Opened from the wallet list in Settings, then left by the gear:
@@ -143,7 +145,21 @@ describe("Back from Settings after leaving by the settings gear", () => {
 
         expect(state.viewStack).toEqual(["main", "settings"]);
         helpers.goBack();
-        expect(state.currentView).not.toBe(SHOW_PHRASE_VIEW);
+        expect(state.currentView).toBe("main");
+    });
+
+    // Each round trip leaves one more Settings under Settings.
+    test("goes to the screen under Settings after two round trips", () => {
+        const { helpers, state, showPhrase } = load();
+        for (let i = 0; i < 2; i++) {
+            showPhrase.show(0);
+            helpers.pushCurrentView();
+            helpers.showView("settings");
+        }
+
+        expect(state.viewStack).toEqual(["main", "settings", "settings"]);
+        helpers.goBack();
+        expect(state.currentView).toBe("main");
     });
 
     // This Back takes Settings off the stack before the screen is left, so