diff --git a/TODO.md b/TODO.md index e53423b..8df4a10 100644 --- a/TODO.md +++ b/TODO.md @@ -132,6 +132,17 @@ but the review is broader than any of them. constant rather than `isDebug()`, so it survives only in a debug build; a testnet or the runtime debug toggle still raises the banner but without the view id. + +- 2026-09-21: The Confirm Delete button on the delete-wallet screen no longer + stays dead after a successful delete + ([#335](https://git.eeqj.de/sneak/AutistMask/issues/335)). The password route + disabled the button before the decrypt and never re-enabled it, so a second + delete in the same popup session needed a reopen; the lost-password route + re-enabled its own button in its leave hook, so the two screens behaved + differently. Both now reset through the shared `finishDelete()`, the one path + both routes take, so they behave the same and the button is live for the next + delete. + - 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The wallet list was the reported case: the address shared one row with the diff --git a/src/popup/views/deleteWallet.js b/src/popup/views/deleteWallet.js index e4c6609..896d7fe 100644 --- a/src/popup/views/deleteWallet.js +++ b/src/popup/views/deleteWallet.js @@ -51,16 +51,12 @@ function clear() { // The lost-password screen holds no secret — a wallet name is not one — // but it is wiped on leave for the neighbouring reason: a typed // confirmation left standing in a hidden view is one click away from -// destroying a wallet the user has since navigated off. The button is -// re-enabled here too, so a screen left mid-delete is usable on re-entry. +// destroying a wallet the user has since navigated off. function clearLostPassword() { lostPasswordIndex = null; $("delete-wallet-lost-name-input").value = ""; $("delete-wallet-lost-flash").textContent = ""; $("delete-wallet-lost-flash").style.visibility = "hidden"; - const btn = $("btn-delete-wallet-lost-confirm"); - btn.disabled = false; - btn.classList.remove("text-muted"); } function show(walletIdx) { @@ -98,6 +94,17 @@ function showLostPassword() { // cleanup and the accountsChanged broadcast cannot drift apart between // them. async function finishDelete(walletIdx) { + // Each route's confirm button was disabled by its own click handler + // before the delete ran. Re-enable both here, on the one path they + // share, so the two routes reset the same way and a second delete in + // the same popup session finds a live button instead of a dead one. + const passwordBtn = $("btn-delete-wallet-confirm"); + passwordBtn.disabled = false; + passwordBtn.classList.remove("text-muted"); + const lostPasswordBtn = $("btn-delete-wallet-lost-confirm"); + lostPasswordBtn.disabled = false; + lostPasswordBtn.classList.remove("text-muted"); + const { activeAddressChanged } = removeWalletFromState(state, walletIdx); deleteWalletIndex = null; @@ -187,8 +194,8 @@ function init(_ctx) { btn.disabled = true; btn.classList.add("text-muted"); - // finishDelete() navigates, and the leave hook re-enables the - // button and wipes the typed name on the way out. + // finishDelete() re-enables the button; navigating away then runs + // the leave hook that wipes the typed name. await finishDelete(lostPasswordIndex); }); diff --git a/tests/deleteWalletLostPassword.test.js b/tests/deleteWalletLostPassword.test.js index 88cc05f..6f2915a 100644 --- a/tests/deleteWalletLostPassword.test.js +++ b/tests/deleteWalletLostPassword.test.js @@ -172,6 +172,15 @@ async function openLostPassword(deleteWallet, walletIdx) { await click("btn-delete-wallet-lost-password"); } +// Delete a wallet through the password route: open its confirm screen, +// enter the password, and confirm. The vault is mocked, so the password +// text itself is irrelevant — decryptWithPassword decides pass or fail. +async function deleteWithPassword(deleteWallet, walletIdx) { + deleteWallet.show(walletIdx); + node("delete-wallet-password").value = "any password"; + await click("btn-delete-wallet-confirm"); +} + // ------------------------------------------------------------ tests // The stub is what every persistence assertion below rests on, so its one @@ -456,15 +465,21 @@ describe("what the screen leaves behind", () => { ); }); - // Left mid-delete, the screen has to come back usable. - test("the confirm button is re-enabled on the way out", async () => { - const { helpers, deleteWallet } = load(); + // Both routes now re-enable through finishDelete(), not their leave + // hooks, so the button comes back live once a delete completes. + test("the confirm button is re-enabled after a delete", async () => { + const { deleteWallet } = load(); await openLostPassword(deleteWallet, 1); - node("btn-delete-wallet-lost-confirm").disabled = true; - helpers.showView("settings"); + node("delete-wallet-lost-name-input").value = "Wallet 2"; + await click("btn-delete-wallet-lost-confirm"); expect(node("btn-delete-wallet-lost-confirm").disabled).toBe(false); + expect( + node("btn-delete-wallet-lost-confirm").classList.contains( + "text-muted", + ), + ).toBe(false); }); // A wallet name is not a secret, so the screen is excluded for the @@ -475,3 +490,48 @@ describe("what the screen leaves behind", () => { expect(RESTORABLE_VIEWS.has("delete-wallet-confirm")).toBe(false); }); }); + +// The password route is the pre-existing bug this file's fix addresses: +// its Confirm Delete button was disabled before the decrypt and never +// re-enabled on success, so a second delete in the same popup session +// found a dead button. Now both routes re-enable through finishDelete(). +// +// Against head these tests fail: with the re-enable absent, the button +// stays disabled after the first delete, so the disabled assertions read +// true where they expect false. +describe("the password route's confirm button", () => { + test("is re-enabled after a successful delete", async () => { + const { deleteWallet, vault } = load(); + vault.decryptWithPassword.mockResolvedValue(); + + await deleteWithPassword(deleteWallet, 1); + + expect(node("btn-delete-wallet-confirm").disabled).toBe(false); + expect( + node("btn-delete-wallet-confirm").classList.contains("text-muted"), + ).toBe(false); + }); + + // The reported symptom: delete one wallet, then open Delete Wallet for + // a second one without reopening the popup. The button must be live on + // that second visit, and the second delete must actually persist. + test("a second delete works in the same popup session", async () => { + const { deleteWallet, vault, storage } = load(); + vault.decryptWithPassword.mockResolvedValue(); + + await deleteWithPassword(deleteWallet, 1); + + // Wallet 2 is gone; the list is now [Wallet 1, Wallet 3]. Opening + // the confirm screen for the wallet now at index 1 (Wallet 3) must + // find its button live, not the dead one the first delete left. + deleteWallet.show(1); + expect(node("btn-delete-wallet-confirm").disabled).toBe(false); + + node("delete-wallet-password").value = "any password"; + await click("btn-delete-wallet-confirm"); + + expect((await persistedWallets(storage)).map((w) => w.name)).toEqual([ + "Wallet 1", + ]); + }); +});