fix: closing a transaction or signature approval window leaves the request pending (closes #673)

The browser can close the approval window when the user clicks away from it,
for example to copy the password from a password manager. Closing it used to
reject the request with 4001. Now the request stays pending and the site gets
no answer. Until it is answered, the toolbar popup opens the oldest such
approval. A worker that starts holds no approvals, so it points the toolbar
popup back at the wallet.

Reject now closes its window only after the background has the rejection. Sent
as the window closed, the rejection could be lost, which the old window-close
rejection hid.

Site-connection and network-switch windows still reject the request when
closed.

Model: opus-5-5

README.md

@@ -512,7 +512,9 @@ window of its own, and the password is required to be absent from every message
 the approval window sends to the background — with the message that would carry
 it 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).
+[#157](https://git.eeqj.de/sneak/AutistMask/issues/157). A transaction or
+signature approval whose window is closed is required to leave the page without
+an answer. The suite then answers it from the page the toolbar button opens.
 
 The limits of that coverage and of the rest of the Chrome suite, none of them
 papered over:
@@ -521,9 +523,9 @@ papered over:
   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.
+  click. So that prompt, and an approval reopened after its window closed, are
+  opened in a tab at the URL the extension puts on the action. Whether a real
+  toolbar click shows that page 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
@@ -642,15 +644,24 @@ above, it does not fail on page errors.
 
 `make test-e2e-firefox` builds `dist/firefox/` and drives the **real popup in a
 real Firefox**, installed as an unpacked MV2 temporary add-on via geckodriver.
-It covers popup load, the StateRecovery screen (the same cases as the Chrome
-suite), wallet creation through the UI, the Add Token screen, and the dApp round
-trips — `eth_requestAccounts`, `personal_sign`, `wallet_switchEthereumChain`
-rejected and then approved, `eth_sendTransaction`, and a closed approval window
-rejecting with EIP-1193 4001 — driven through the real content script,
-background page and approval windows. The popup open all along is required to
-show the `[TESTNET]` banner once the switch to Sepolia is approved and to drop
-it once the switch back is. The `personal_sign` prompt is also required to fit
-its window without scrolling sideways, with the extension's pages at 170% zoom.
+It covers:
+
+- popup load
+- the StateRecovery screen, with the same cases as the Chrome suite
+- wallet creation through the UI
+- the Add Token screen
+- `eth_requestAccounts` from a dApp
+- `personal_sign` from a dApp
+- `wallet_switchEthereumChain` from a dApp, rejected and then approved
+- `eth_sendTransaction` from a dApp
+- an approval window closed without an answer, which leaves its request pending
+  until it is rejected
+
+Each dApp request runs through the real content script and background page, and
+opens a real approval window. The popup open all along is required to show the
+`[TESTNET]` banner once the switch to Sepolia is approved and to drop it once
+the switch back is. The `personal_sign` prompt is also required to fit its
+window without scrolling sideways, with the extension's pages at 170% zoom.
 
 The suite lives in `tests/e2e/firefox/`. Its WebDriver client (`driver.js`) has
 **no npm dependencies at all**: it is built on global `fetch` and
@@ -2637,8 +2648,12 @@ view would leave a wallet one click from deletion.
       Data Model) → full-sentence error on the error line saying why, nothing
       signed (no screen change)
     - "Reject" → closes popup (returns rejection to background)
-    - Popup window closed without answering → the request is rejected with
-      EIP-1193 code 4001
+    - Popup window closed without answering → the request stays pending, and the
+      site gets no answer. The browser can close this window when the user
+      clicks away from it, for example to copy the password from a password
+      manager. Until the request is answered, the toolbar button opens this
+      screen instead of the wallet. When several requests wait with their
+      windows closed, the toolbar button opens the oldest first.
 
 #### SignApproval (`approve-sign`)
 
@@ -2727,8 +2742,9 @@ view would leave a wallet one click from deletion.
     - "Sign" (wrong password, or a signing failure) → error line, no screen
       change
     - "Reject" → closes popup (returns rejection to background)
-    - Popup window closed without answering → the request is rejected with
-      EIP-1193 code 4001
+    - Popup window closed without answering → the request stays pending, as on
+      TxApproval. Until the request is answered, the toolbar button opens this
+      screen instead of the wallet.
 
 #### NetworkApproval (`approve-network`)
 

docs/README.md

@@ -350,6 +350,13 @@ enter your password and click "Confirm" to authorize it. Message and typed-data
 signature requests work the same way, with a "Sign" button, and also require
 your password.
 
+If a transaction or signature popup closes before you answer, for example when
+you click away to copy your password, the request stays open. Closing the popup
+does not turn the request down. Click the AutistMask icon in your browser
+toolbar to bring the request back. Until you answer, the icon opens the request
+instead of your wallet. If several requests are waiting this way, the oldest
+comes first.
+
 A connected site can also ask to switch the wallet's network. A popup shows the
 site's origin, the current network and the requested network. Nothing changes
 unless you click "Switch"; clicking "Reject" or closing the popup leaves

src/background/index.js

@@ -383,14 +383,25 @@ async function proxyRpc(method, params) {
     return json.result;
 }
 
-// The site-connection approval the toolbar popup is set to open, or null while
-// it opens the wallet. Set only by resetPopupUrl() and showInToolbarPopup().
+// The approval the toolbar popup is set to open, or null while it opens the
+// wallet. Set only by updateToolbarPopup() and showInToolbarPopup().
 let toolbarPopupApprovalId = null;
 
-function resetPopupUrl() {
-    toolbarPopupApprovalId = null;
+// Point the toolbar popup at the oldest pending approval whose window closed,
+// or at the wallet when there is none. The user can then answer that approval
+// from the toolbar popup. pendingApprovals lists approvals in the order they
+// were added, so the first one found is the oldest.
+function updateToolbarPopup() {
+    const closed = Object.values(pendingApprovals).find(
+        (approval) => approval.windowClosed,
+    );
+    toolbarPopupApprovalId = closed ? closed.id : null;
     if (actionNs && typeof actionNs.setPopup === "function") {
-        actionNs.setPopup({ popup: "src/popup/index.html" });
+        actionNs.setPopup({
+            popup: closed
+                ? "src/popup/index.html?approval=" + closed.id
+                : "src/popup/index.html",
+        });
     }
 }
 
@@ -420,15 +431,16 @@ function settleApproval(id, result, options) {
     // approval stops being alive.
     releaseTxApprovalSlotFor(id);
     approval.resolve(result);
-    resetPopupUrl();
+    updateToolbarPopup();
     return true;
 }
 
 // What a pending approval resolves to when it is given up on rather than
-// answered: the window was closed, or could not be opened at all. A tx or sign
-// approval answers the requesting page in EIP-1193 shape; a site-connection or
-// network-switch approval answers its handler in the shape the popup's
-// decision has, as a refusal.
+// answered: a site-connection or network-switch window was closed, or the
+// window could not be opened at all. An address switch in the wallet gives up
+// pending approvals too. A tx or sign approval answers the requesting page in
+// EIP-1193 shape; a site-connection or network-switch approval answers its
+// handler in the shape the popup's decision has, as a refusal.
 function abandonedResult(approval, code, message) {
     if (approval.type === "tx" || approval.type === "sign") {
         return { error: { code, message } };
@@ -436,8 +448,9 @@ function abandonedResult(approval, code, message) {
     return { approved: false, remember: false };
 }
 
-// A window the user closed without answering is a refusal by the user, which
-// is 4001 and the wording every other rejection path already uses.
+// A site-connection or network-switch window the user closed without answering
+// is a refusal by the user, which is 4001 and the wording every other rejection
+// path already uses.
 const APPROVAL_REJECTED_CODE = 4001;
 const APPROVAL_REJECTED_MESSAGE = "User rejected the request.";
 
@@ -469,27 +482,10 @@ function claimApproval(approval) {
 }
 
 // Release an approval whose attempt failed in a way the user can retry.
-// Nothing was broadcast, so the next attempt may claim it.
-//
-// Unless the window it would be retried in is already gone. The user closed it
-// while the attempt was running and settleApproval() declined then, correctly,
-// because the attempt still owned the approval; the attempt has now failed, so
-// nothing owns it and nothing can reach it. Left standing it would hold the
-// requesting page's promise open forever and, with it, the transaction
-// approval slot. It is settled here as the rejection the closed window
-// already meant.
+// Nothing was broadcast, so the next attempt may claim it. If the user closed
+// its window while the attempt ran, the toolbar popup opens it for that retry.
 function releaseApproval(approval) {
     approval.attemptInFlight = false;
-    if (approval.windowClosed) {
-        settleApproval(
-            approval.id,
-            abandonedResult(
-                approval,
-                APPROVAL_REJECTED_CODE,
-                APPROVAL_REJECTED_MESSAGE,
-            ),
-        );
-    }
 }
 
 // Open approval in a separate popup window, unless it is no longer pending.
@@ -712,9 +708,7 @@ function isExtensionSender(sender) {
 // had refused.
 //
 // TX and sign approvals do not decide here. They stay pending across a
-// disconnect. When their window closes, the windows.onRemoved listener below
-// rejects them, unless an approved attempt is still running, in which case
-// that attempt answers the page.
+// disconnect, and across their window closing, until the user answers them.
 runtime.onConnect.addListener((port) => {
     if (port.name.startsWith("approval:")) {
         const id = port.name.split(":")[1];
@@ -742,9 +736,7 @@ runtime.onConnect.addListener((port) => {
             const approval = pendingApprovals[id];
             if (approval) {
                 if (approval.type === "tx" || approval.type === "sign") {
-                    // When its window closes, windows.onRemoved rejects it,
-                    // unless an approved attempt is still running, in which
-                    // case that attempt answers the page.
+                    // It stays pending until the user answers it.
                     return;
                 }
                 settleApproval(id, { approved: false, remember: false });
@@ -1297,7 +1289,7 @@ async function broadcastAccountsChanged() {
             windowsRemove(approval.windowId).catch(() => {});
         }
     }
-    resetPopupUrl();
+    updateToolbarPopup();
     // An address switch ends every connection allowed without "Remember". This
     // waits on storage, so it comes after the pending approvals above, which
     // are still rejected the moment the switch arrives.
@@ -1489,36 +1481,44 @@ if (runtime.onStartup) {
 }
 startBackgroundJobs();
 
-// When approval window is closed without a response, treat as rejection.
-// "Without a response" is the operative part: the popup stays open across the
-// verify and broadcast it is waiting on, so a user closing an apparently-hung
-// window is an ordinary event with an attempt already in flight behind it.
-// settleApproval() refuses those, which leaves the attempt to report its real
-// outcome to the page — and the window is recorded as gone, so that an attempt
-// which then fails retryably settles instead of waiting in a window that no
-// longer exists.
+// The toolbar popup keeps its URL when the worker stops. A worker that starts
+// holds no approvals, so the toolbar popup is pointed back at the wallet.
+updateToolbarPopup();
+
+// A transaction or signature approval stays pending when its window closes.
+// The browser can close that window when the user clicks away from it, for
+// example to copy the password from a password manager. The approval is marked
+// as closed, and from then on the toolbar popup opens it (see
+// updateToolbarPopup()). An attempt already running when the window closes
+// goes on and reports its outcome to the page.
 //
-// A site-connection or network-switch approval whose popup connected its port
-// is not decided here. That popup approves and closes in the same breath, and
-// this event races the decision on a channel of its own — the same race the
-// port exists to end. Its port disconnect says the same thing this event does,
-// in an order that is defined, so the disconnect is left to say it. The window
-// closing before any port connected is the one case with nothing else to speak
-// for it, and is rejected here so the dApp is not left waiting on a window that
-// is gone.
+// This listener rejects a site-connection or network-switch approval whose
+// window closes before its popup connected its port. Once the port is
+// connected, the approval is left to the port. That popup approves and closes
+// in the same breath, and this event races the decision on a channel of its
+// own — the same race the port exists to end. Its port disconnect says the
+// same thing this event does, in an order that is defined, so the disconnect is
+// left to say it. The window closing before any port connected is the one case
+// with nothing else to speak for it, and is rejected here so the dApp is not
+// left waiting on a window that is gone.
 if (windowsNs && windowsNs.onRemoved) {
     windowsNs.onRemoved.addListener((windowId) => {
         for (const [id, approval] of Object.entries(pendingApprovals)) {
             if (approval.windowId !== windowId) continue;
-            const decidedOnPort =
-                approval.type !== "tx" && approval.type !== "sign";
-            if (decidedOnPort && approval.portConnected) continue;
-            const rejection = abandonedResult(
-                approval,
-                APPROVAL_REJECTED_CODE,
-                APPROVAL_REJECTED_MESSAGE,
+            if (approval.type === "tx" || approval.type === "sign") {
+                approval.windowClosed = true;
+                updateToolbarPopup();
+                continue;
+            }
+            if (approval.portConnected) continue;
+            settleApproval(
+                id,
+                abandonedResult(
+                    approval,
+                    APPROVAL_REJECTED_CODE,
+                    APPROVAL_REJECTED_MESSAGE,
+                ),
             );
-            if (!settleApproval(id, rejection)) approval.windowClosed = true;
         }
     });
 }
@@ -1663,7 +1663,9 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 });
                 return false;
             }
-            return true;
+            // The popup waits for this answer before it closes its window.
+            sendResponse(null);
+            return false;
         }
 
         // The popup signs; it reports back here when it could not. Keep the
@@ -1863,7 +1865,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 });
                 return false;
             }
-            return true;
+            sendResponse(null);
+            return false;
         }
 
         // The popup signs; it reports back here when it could not. Keep the

src/popup/index.js

@@ -281,9 +281,9 @@ async function followStoredSettings() {
 // zoom it reports, 170% among them, so once the page has its new width (its
 // resize event) it is measured again, and the window widened again while the
 // page still falls short, three times at most. A window that cannot grow sends
-// no resize event, and the page stops there. The connection prompt can also
-// show in the toolbar popup, whose window is the user's browser window; that
-// one is left alone.
+// no resize event, and the page stops there. An approval can also show in the
+// toolbar popup, whose window is the user's browser window; that one is left
+// alone.
 async function widenApprovalWindow() {
     const page = document.documentElement;
     if (page.scrollWidth <= page.clientWidth) return;

src/popup/views/approval.js

@@ -55,7 +55,7 @@ const {
 } = require("../../shared/approvalVerify");
 const txStatus = require("./txStatus");
 const uniswap = require("../../shared/uniswap");
-const { notify, runtimeApi, sendMessage } = require("../../shared/browserApi");
+const { runtimeApi, sendMessage } = require("../../shared/browserApi");
 
 const erc20Iface = new Interface(ERC20_ABI);
 
@@ -1142,12 +1142,15 @@ function init(_ctx) {
         }
     });
 
-    $("btn-reject-tx").addEventListener("click", () => {
-        notify({
+    // The window closes only once the background has the rejection. A message
+    // sent as the window closes can be lost, and a closed window does not
+    // reject the request.
+    $("btn-reject-tx").addEventListener("click", async () => {
+        await sendMessage({
             type: "AUTISTMASK_TX_RESPONSE",
             id: approvalId,
             approved: false,
-        });
+        }).catch(() => {});
         window.close();
     });
 
@@ -1270,12 +1273,13 @@ function init(_ctx) {
         if (outcome.retryable) setSignButtonBusy(false);
     });
 
-    $("btn-reject-sign").addEventListener("click", () => {
-        notify({
+    // As for the transaction's Reject above.
+    $("btn-reject-sign").addEventListener("click", async () => {
+        await sendMessage({
             type: "AUTISTMASK_SIGN_RESPONSE",
             id: approvalId,
             approved: false,
-        });
+        }).catch(() => {});
         window.close();
     });
 }

tests/approvalWindow.test.js

@@ -12,6 +12,7 @@ const {
     cleanupPopup,
     settle,
     unversionedValidProfile,
+    ADDRESS,
 } = require("./support/popupBoot");
 
 afterEach(cleanupPopup);
@@ -174,3 +175,87 @@ test("a connection prompt in the toolbar popup leaves the browser window alone",
     expect(windows.updates).toEqual([]);
     expect(env.pageErrors).toEqual([]);
 });
+
+// A closed transaction or signature window leaves its request pending
+// (https://git.eeqj.de/sneak/AutistMask/issues/673). So Reject closes the
+// window only once the background has the rejection. A message sent as the
+// window closes can be lost, and the request would stay pending although the
+// user rejected it.
+test.each([
+    {
+        kind: "transaction",
+        view: "approve-tx",
+        reject: "btn-reject-tx",
+        type: "AUTISTMASK_TX_RESPONSE",
+        approval: {
+            type: "tx",
+            approvedFrom: ADDRESS,
+            approvedTx: {
+                chainId: 1,
+                to: ADDRESS,
+                value: "0",
+                data: "0x",
+                gasLimit: "21000",
+                maxFeePerGas: "1000000000",
+                maxPriorityFeePerGas: "1000000000",
+                nonce: 0,
+            },
+        },
+    },
+    {
+        kind: "signature",
+        view: "approve-sign",
+        reject: "btn-reject-sign",
+        type: "AUTISTMASK_SIGN_RESPONSE",
+        approval: {
+            type: "sign",
+            approvedFrom: ADDRESS,
+            signParams: {
+                method: "personal_sign",
+                message: "0x48656c6c6f",
+                from: ADDRESS,
+            },
+        },
+    },
+])(
+    "Reject on a $kind approval closes the window only once the background has answered",
+    async ({ view, reject, type, approval }) => {
+        let describeApproval = null;
+        let answerReject = null;
+        const sent = [];
+        const env = await bootPopup(unversionedValidProfile(), {
+            search: "?approval=approval-1",
+            runtime: {
+                connect: () => ({ postMessage: () => {} }),
+                sendMessage: (msg, reply) => {
+                    if (msg.type === "AUTISTMASK_GET_APPROVAL") {
+                        describeApproval = reply;
+                    }
+                    if (msg.type === type) {
+                        sent.push(msg);
+                        answerReject = reply;
+                    }
+                },
+            },
+        });
+        describeApproval({
+            ...approval,
+            origin: "https://dapp.example",
+            isPhishingDomain: false,
+        });
+        await settle();
+        expect(env.visibleViews()).toEqual([view]);
+        const close = jest.fn();
+        globalThis.window.close = close;
+
+        const clicked = env.click(reject);
+        await settle();
+        expect(sent).toEqual([{ type, id: "approval-1", approved: false }]);
+        expect(close).not.toHaveBeenCalled();
+
+        answerReject(null);
+        await clicked;
+        expect(close).toHaveBeenCalledTimes(1);
+        expect(env.pageErrors).toEqual([]);
+    },
+);

tests/backgroundApproval.test.js

@@ -241,10 +241,11 @@ function loadBackground(options) {
     let connectListener = null;
     const created = [];
     const removed = [];
-    // Every URL the background put on the browser action. A site approval
-    // raised through action.openPopup() opens no window at all, so this is
-    // the only place its id appears.
-    const actionPopups = [];
+    // Every URL the background put on the browser action. When a test gives
+    // the URL the action holds before the worker starts, that URL comes
+    // first. A site approval raised through action.openPopup() opens no
+    // window at all, so this is the only place its id appears.
+    const actionPopups = opts.toolbarPopup ? [opts.toolbarPopup] : [];
     const openPopup = jest.fn(() => Promise.resolve());
 
     global.chrome = {
@@ -774,10 +775,9 @@ describe("one transaction approval at a time", () => {
 
     // The user closes an approval window that looks hung while the attempt
     // behind it is still running, and that attempt then fails in a way that
-    // would normally leave the approval standing for a retry. There is no
-    // window left to retry in, so leaving it standing answers the requesting
-    // page never — and holds the slot for the life of the worker with it.
-    test("an approval whose window closed under a failed attempt is answered, and frees the next request", async () => {
+    // leaves the approval standing for a retry. The toolbar popup opens it, so
+    // the user can still retry it, and answering it frees the slot.
+    test("an approval whose window closed under a failed attempt is retried from the toolbar popup, and frees the next request", async () => {
         const stalled = deferred();
         const bg = loadBackground();
 
@@ -808,11 +808,28 @@ describe("one transaction approval at a time", () => {
 
         stalled.resolve();
         await settle();
-        expect(first.result()).toEqual({
-            error: { code: 4001, message: "User rejected the request." },
-        });
+        expect(first.result()).toBeNull();
+        expect(bg.actionPopups[bg.actionPopups.length - 1]).toBe(
+            "src/popup/index.html?approval=" + first.id(),
+        );
 
         bg.setStateReadHook(null);
+        bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id: first.id(),
+                approved: true,
+                rawSignedTx: await signedAtNonce(NONCE),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(first.result()).toEqual({ result: "0xfeed" });
+        expect(bg.actionPopups[bg.actionPopups.length - 1]).toBe(
+            "src/popup/index.html",
+        );
+
         const second = bg.requestTx();
         await settle();
         expect(second.result()).toBeNull();
@@ -853,8 +870,10 @@ describe("one transaction approval at a time", () => {
         const first = bg.requestTx();
         await settle();
 
-        // The user closes the approval window, which rejects it.
-        bg.closeWindow(1);
+        bg.send(
+            { type: "AUTISTMASK_TX_RESPONSE", id: first.id(), approved: false },
+            { url: bg.fromPopup.url },
+        );
         await settle();
         expect(first.result()).toEqual({
             error: { code: 4001, message: "User rejected the request." },
@@ -897,7 +916,7 @@ describe("the worker keeps running while an approval is pending", () => {
         try {
             const bg = loadBackground();
             const calls = global.chrome.runtime.getPlatformInfo;
-            bg.requestTx();
+            const pending = bg.requestTx();
             await settle();
             expect(startTimer).toHaveBeenCalledTimes(1);
             expect(startTimer.mock.calls[0][1]).toBe(20000);
@@ -906,11 +925,23 @@ describe("the worker keeps running while an approval is pending", () => {
             tick();
             expect(calls).toHaveBeenCalledTimes(1);
 
-            // The user closes the approval window, which rejects it.
+            // A closed window leaves the approval pending, so the calls go on.
             bg.closeWindow(1);
             await settle();
             tick();
-            expect(calls).toHaveBeenCalledTimes(1);
+            expect(calls).toHaveBeenCalledTimes(2);
+
+            bg.send(
+                {
+                    type: "AUTISTMASK_TX_RESPONSE",
+                    id: pending.id(),
+                    approved: false,
+                },
+                { url: bg.fromPopup.url },
+            );
+            await settle();
+            tick();
+            expect(calls).toHaveBeenCalledTimes(2);
             expect(stopTimer).toHaveBeenCalledWith(1);
         } finally {
             startTimer.mockRestore();
@@ -1841,6 +1872,153 @@ describe("the interlock releases a failed attempt", () => {
     });
 });
 
+// A transaction or signature approval stays pending when its window closes.
+// The browser can close that window when the user clicks away from it, for
+// example to copy the password from a password manager. The toolbar popup then
+// opens the approval, and the user answers it there
+// (https://git.eeqj.de/sneak/AutistMask/issues/673).
+describe("a closed approval window leaves the request pending", () => {
+    // The URL the toolbar popup opens now.
+    function toolbarPopup(bg) {
+        return bg.actionPopups[bg.actionPopups.length - 1];
+    }
+
+    // The timer that keeps the worker running while an approval is pending is
+    // stubbed, so no test here leaves that timer running when it ends.
+    let keepRunningTimer;
+    beforeEach(() => {
+        keepRunningTimer = jest
+            .spyOn(global, "setInterval")
+            .mockImplementation(() => 1);
+    });
+    afterEach(() => {
+        keepRunningTimer.mockRestore();
+    });
+
+    test("the toolbar popup opens a closed transaction approval, and approving it there sends", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+
+        bg.closeWindow(1);
+        await settle();
+        expect(pending.result()).toBeNull();
+        expect(toolbarPopup(bg)).toBe(
+            "src/popup/index.html?approval=" + pending.id(),
+        );
+
+        bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id: pending.id(),
+                approved: true,
+                rawSignedTx: await signedAtNonce(NONCE),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+        expect(toolbarPopup(bg)).toBe("src/popup/index.html");
+    });
+
+    test("Reject in the toolbar popup answers 4001, and the toolbar popup opens the wallet again", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestSign();
+        await settle();
+
+        bg.closeWindow(1);
+        await settle();
+        expect(pending.result()).toBeNull();
+        expect(toolbarPopup(bg)).toBe(
+            "src/popup/index.html?approval=" + pending.id(),
+        );
+
+        const reject = bg.send(
+            {
+                type: "AUTISTMASK_SIGN_RESPONSE",
+                id: pending.id(),
+                approved: false,
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+        // The popup closes its window only once this answer arrives.
+        expect(reject.sendResponse).toHaveBeenCalledWith(null);
+        expect(toolbarPopup(bg)).toBe("src/popup/index.html");
+    });
+
+    test("with two closed, the toolbar popup opens the older one first, then the other", async () => {
+        const bg = loadBackground();
+        const sign = bg.requestSign();
+        await settle();
+        const signId = sign.id();
+        const tx = bg.requestTx();
+        await settle();
+
+        // The signature request is in window 1 and the transaction in window
+        // 2. The newer window is closed first.
+        bg.closeWindow(2);
+        bg.closeWindow(1);
+        await settle();
+        expect(toolbarPopup(bg)).toBe(
+            "src/popup/index.html?approval=" + signId,
+        );
+
+        bg.send(
+            { type: "AUTISTMASK_SIGN_RESPONSE", id: signId, approved: false },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(toolbarPopup(bg)).toBe(
+            "src/popup/index.html?approval=" + tx.id(),
+        );
+
+        const reject = bg.send(
+            { type: "AUTISTMASK_TX_RESPONSE", id: tx.id(), approved: false },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(tx.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+        expect(reject.sendResponse).toHaveBeenCalledWith(null);
+        expect(toolbarPopup(bg)).toBe("src/popup/index.html");
+    });
+
+    test("an address switch still rejects a closed approval", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        bg.closeWindow(1);
+        await settle();
+        expect(pending.result()).toBeNull();
+
+        bg.send(
+            { type: "AUTISTMASK_ACTIVE_CHANGED" },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+        expect(toolbarPopup(bg)).toBe("src/popup/index.html");
+    });
+
+    // The worker can stop while an approval waits with its window closed. The
+    // approval is lost with the worker, but the toolbar popup keeps its URL.
+    // The next worker points the toolbar popup back at the wallet.
+    test("a worker that starts points the toolbar popup back at the wallet", () => {
+        const bg = loadBackground({
+            toolbarPopup: "src/popup/index.html?approval=lost-approval",
+        });
+        expect(toolbarPopup(bg)).toBe("src/popup/index.html");
+    });
+});
+
 // The claim is what makes one approval one broadcast, so it has to hold
 // against everything else that retires an approval, not just against a second
 // AUTISTMASK_TX_RESPONSE. Each of these paths used to resolve the waiting
@@ -1961,20 +2139,6 @@ describe("a claimed approval outlives every other retirement path", () => {
     });
 
     // The refusals above must not cost the rejection its ordinary meaning.
-    test("with no attempt running, closing the window still rejects", async () => {
-        const bg = loadBackground();
-        const pending = bg.requestTx();
-        await settle();
-
-        bg.closeWindow(1);
-        await settle();
-
-        expect(pending.result()).toEqual({
-            error: { code: 4001, message: "User rejected the request." },
-        });
-        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
-    });
-
     test("with no attempt running, an active-address switch still rejects and closes", async () => {
         const bg = loadBackground();
         const pending = bg.requestTx();
@@ -2584,15 +2748,12 @@ describe("a site connection decided as the popup closes", () => {
         });
     });
 
-    // The `isSite &&` half of that skip, which is what keeps it from reaching
-    // a tx or sign approval. The popup connects its port in show() before it
-    // knows the approval's type, and the background sets portConnected without
-    // looking at the type either, so a tx approval in the fallback window
-    // carries the flag too. Without the conjunct the window event would skip
-    // it, windowClosed would never be set, releaseApproval() would never settle
-    // it, and the page would hang — the #271 regression this guard is written
-    // around.
-    test("a tx window closed with the port connected still rejects", async () => {
+    // That skip must not reach a tx or sign approval. The popup connects its
+    // port in show() before it knows the approval's type, and the background
+    // sets portConnected without looking at the type either, so a tx approval
+    // carries the flag too. If the window event skipped it, windowClosed would
+    // never be set, and the toolbar popup would never open the approval.
+    test("a tx window closed with the port connected is still opened by the toolbar popup", async () => {
         const bg = loadBackground();
         const pending = bg.requestTx();
         await settle();
@@ -2600,9 +2761,10 @@ describe("a site connection decided as the popup closes", () => {
         await settle();
         bg.closeWindow(1);
         await settle();
-        expect(pending.result()).toEqual({
-            error: { code: 4001, message: "User rejected the request." },
-        });
+        expect(pending.result()).toBeNull();
+        expect(bg.actionPopups[bg.actionPopups.length - 1]).toBe(
+            "src/popup/index.html?approval=" + pending.id(),
+        );
     });
 });
 
@@ -2994,13 +3156,14 @@ describe("where an approval window opens", () => {
         expect(bg.created[1].left).toBeUndefined();
         expect(bg.created[1].top).toBeUndefined();
 
-        // The request waits on the second window rather than failing:
-        // closing that window is refusing the prompt.
+        // The request waits on the second window rather than failing. Once
+        // that window closes, the toolbar popup opens the request.
         expect(sign.result()).toBeNull();
         bg.closeWindow(2);
         await settle();
-        expect(sign.result()).toEqual({
-            error: { code: 4001, message: "User rejected the request." },
-        });
+        expect(sign.result()).toBeNull();
+        expect(bg.actionPopups[bg.actionPopups.length - 1]).toBe(
+            "src/popup/index.html?approval=" + sign.id(),
+        );
     });
 });

tests/e2e/firefox/run.js

@@ -1035,11 +1035,14 @@ step(
     },
 );
 
+// A closed approval window leaves its request pending, and the toolbar button
+// opens it (https://git.eeqj.de/sneak/AutistMask/issues/673). WebDriver cannot
+// see the toolbar popup, so its page is opened in a tab, at the URL the
+// extension put on the button.
 step(
-    "closing an approval window rejects the request with 4001",
+    "a closed approval window leaves the request pending until it is rejected",
     async (env) => {
         const d = env.driver;
-        const before = env.server.broadcast.length;
 
         await d.switchToWindow(env.dappWindow);
         await startRequest(d, "sign-closed", "personal_sign", [
@@ -1053,15 +1056,45 @@ step(
 
         // Closed, not rejected: this is the windows.onRemoved path, which can
         // only fire if windows.create() handed back a window id for the
-        // approval to be matched against — call site 4 in the issue, and the
-        // reason suppressing that write-back turns this step red.
+        // approval to be matched against. Without that id the toolbar button
+        // never opens the approval, and this step fails.
         await d.closeWindow(env.dappWindow);
-
-        await assertUserRejection(d, "sign-closed", "a closed approval window");
+        const outcome = await settleRequest(d, "sign-closed", 2000);
         assert(
-            env.server.broadcast.length === before,
-            "a closed approval window still put a transaction on the node",
+            outcome.settled === "pending",
+            "closing the approval window answered the page: " +
+                JSON.stringify(outcome),
         );
+
+        await d.switchToWindow(env.popupWindow);
+        const url = await d.executeAsync(
+            `const done = arguments[arguments.length - 1];
+             browser.browserAction.getPopup({}).then(done, (e) => done(String(e)));`,
+        );
+        assert(
+            url.includes("?approval="),
+            "the toolbar button does not open the pending approval: " + url,
+        );
+        const reopened = await d.newWindow("tab");
+        await d.switchToWindow(reopened);
+        await d.navigate(url);
+        await d.waitVisible("#view-approve-sign");
+        await d.click("#btn-reject-sign");
+
+        await d.switchToWindow(env.dappWindow);
+        await assertUserRejection(
+            d,
+            "sign-closed",
+            "the signature request rejected from the toolbar button",
+        );
+
+        // A page may close only a window a script opened, so the tab can
+        // still be open.
+        const left = await d.findWindow((u) => u.includes("?approval="));
+        if (left) {
+            await d.switchToWindow(left);
+            await d.closeWindow(env.dappWindow);
+        }
     },
 );
 

tests/e2e/run.js

@@ -4110,11 +4110,12 @@ async function waitForApprovalWindow(ctx, timeout = 30000) {
 // reservation exists.
 // How long the tab creation above is given to take effect before the request
 // that raises the next prompt is issued. Dismissing a browser-action popup
-// disconnects its port, and src/background/index.js calls resetPopupUrl() on
-// that disconnect; issued too early, the next request's setPopup() is undone
-// by the previous popup's teardown, chrome.action.openPopup() then rejects,
-// and the fallback window it opens dismisses the popup that was just raised —
-// which settles the fresh approval as a rejection before it can be seen.
+// disconnects its port, and src/background/index.js calls
+// updateToolbarPopup() on that disconnect; issued too early, the next
+// request's setPopup() is undone by the previous popup's teardown,
+// chrome.action.openPopup() then rejects, and the fallback window it opens
+// dismisses the popup that was just raised — which settles the fresh approval
+// as a rejection before it can be seen.
 // Every one of those steps was observed. 1.5s is comfortably past it.
 const APPROVAL_TAB_SETTLE_MS = 1500;
 
@@ -4140,6 +4141,17 @@ async function reserveApprovalTab(env) {
     return env.approvalTab;
 }
 
+// The URL the extension has put on the action: the page the toolbar button
+// opens.
+function toolbarPopupUrl(env) {
+    return env.page.evaluate(
+        () =>
+            new Promise((resolve) => {
+                chrome.action.getPopup({}, (u) => resolve(String(u)));
+            }),
+    );
+}
+
 // The site-connection prompt: the window the extension opened if it managed
 // to open one, and the reserved tab at the action's approval URL otherwise.
 async function openSiteApprovalPopup(env, timeout = 30000) {
@@ -4150,12 +4162,7 @@ async function openSiteApprovalPopup(env, timeout = 30000) {
             .find((p) => !p.isClosed() && p.url().includes("?approval="));
         if (existing) return existing;
 
-        const url = await env.page.evaluate(
-            () =>
-                new Promise((resolve) => {
-                    chrome.action.getPopup({}, (u) => resolve(String(u)));
-                }),
-        );
+        const url = await toolbarPopupUrl(env);
         if (url.includes("?approval=")) {
             await env.approvalTab.goto(url);
             return env.approvalTab;
@@ -5027,6 +5034,116 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
     );
 });
 
+// A transaction or signature approval stays pending when its window closes.
+// The browser can close that window when the user clicks away from it, for
+// example to copy the password from a password manager. The toolbar button then
+// opens the approval, and the user answers it there
+// (https://git.eeqj.de/sneak/AutistMask/issues/673).
+//
+// This closes the approval window of request `key` and checks that the page
+// has no answer. It then opens the approval again from the toolbar button.
+// Playwright cannot click that button, so its page is opened in a tab, at the
+// URL the extension put on the button.
+async function closeAndReopen(env, key, view) {
+    const approvalWindow = await waitForApprovalWindow(env.ctx);
+    await visible(approvalWindow, view);
+    await approvalWindow.close();
+
+    const outcome = await settleRequest(env.dapp, key, 2000);
+    assert(
+        outcome.settled === "pending",
+        "closing the approval window answered the page: " +
+            JSON.stringify(outcome),
+    );
+
+    const url = await toolbarPopupUrl(env);
+    assert(
+        url.includes("?approval="),
+        "the toolbar button does not open the pending approval: " + url,
+    );
+    const reopened = await env.ctx.newPage();
+    await reopened.goto(url);
+    await visible(reopened, view);
+    return reopened;
+}
+
+// The stub node answers every nonce request with 0, and the wallet refuses a
+// nonce it has already broadcast. A run can therefore approve one dApp
+// transaction only, so here the signature is approved and the transaction
+// rejected.
+test("a closed signature approval is signed from the toolbar button (#673)", async (env) => {
+    await startRequest(env.dapp, "sign-closed", "personal_sign", [
+        SIGN_HEX,
+        env.expectedAddress,
+    ]);
+    const popup = await closeAndReopen(
+        env,
+        "sign-closed",
+        "#view-approve-sign",
+    );
+    let outcome;
+    try {
+        await popup.fill("#approve-sign-password", PASSWORD);
+        await clickAndClose(popup, "#btn-approve-sign");
+        outcome = await settleRequest(env.dapp, "sign-closed");
+    } finally {
+        await closeApprovalPages(env.ctx);
+    }
+    assert(
+        outcome.settled === "resolved",
+        "personal_sign did not resolve: " + JSON.stringify(outcome),
+    );
+    const recovered = getAddress(
+        verifyMessage(getBytes(SIGN_HEX), outcome.result),
+    );
+    assert(
+        recovered === env.expectedAddress,
+        "the personal_sign signature recovers to " +
+            recovered +
+            ", not to the approved address " +
+            env.expectedAddress,
+    );
+
+    const url = await toolbarPopupUrl(env);
+    assert(
+        !url.includes("?approval="),
+        "the toolbar button still opens the answered approval: " + url,
+    );
+});
+
+test("a closed transaction approval is rejected from the toolbar button (#673)", async (env) => {
+    const before = env.routeOpts.broadcastTransactions.length;
+    await startRequest(env.dapp, "tx-closed", "eth_sendTransaction", [
+        {
+            from: env.expectedAddress,
+            to: STUB_COUNTERPARTY,
+            value: toQuantity(TX_VALUE_WEI),
+            data: TX_DATA,
+        },
+    ]);
+    const popup = await closeAndReopen(env, "tx-closed", "#view-approve-tx");
+    try {
+        await clickAndClose(popup, "#btn-reject-tx");
+        await assertUserRejection(
+            env.dapp,
+            "tx-closed",
+            "the transaction rejected from the toolbar button",
+        );
+    } finally {
+        await closeApprovalPages(env.ctx);
+    }
+    assert(
+        env.routeOpts.broadcastTransactions.length === before,
+        "a rejected transaction still reached the RPC",
+    );
+
+    const url = await toolbarPopupUrl(env);
+    assert(
+        !url.includes("?approval="),
+        "the toolbar button still opens the answered approval: " + url,
+    );
+});
+
 // The extension used to centre every approval window on the last focused
 // window. Centred on another approval window, the new one landed where the
 // browser refused to create it, and the request failed with no window at all
@@ -5070,8 +5187,8 @@ test("a prompt raised while another approval window has focus opens its own (#29
     });
     await visible(txWindow, "#view-approve-tx");
 
-    // Closing a window is refusing its prompt, so both answer 4001.
-    await closeApprovalPages(env.ctx);
+    await clickAndClose(txWindow, "#btn-reject-tx");
+    await clickAndClose(signWindow, "#btn-reject-sign");
     await assertUserRejection(env.dapp, "focus-tx", "the transaction prompt");
     await assertUserRejection(env.dapp, "focus-sign", "the sign prompt");
 });
@@ -5678,9 +5795,11 @@ async function main() {
         // for the new one. After a test that passed, its windows get five
         // seconds to close, and one still open then fails the test: it should
         // have closed itself, and closing it here unreported would hide that.
-        // After a test that already failed, they are closed unreported, so
-        // that a prompt it left unanswered does not refuse the next one from
-        // its site.
+        // After a test that already failed, they are closed unreported. That
+        // rejects a site-connection or network-switch prompt it left
+        // unanswered, so the prompt does not refuse the next one from its
+        // site. A transaction or signature approval stays pending, and can
+        // fail the tests after it.
         if (!failure) {
             const deadline = Date.now() + 5000;
             let open;