fix: re-enable Confirm Delete after a delete, so a second one needs no reopen (closes #335)
check / check (push) Failing after 0s
e2e / e2e-firefox (push) Failing after 0s
e2e / e2e-chrome (push) Successful in 1m56s

The password route disabled its Confirm Delete button before the decrypt
and never re-enabled it on success, so a second delete in the same popup
session found a dead button until the popup was closed and reopened. The
lost-password route re-enabled its own button in its leave hook, so the
two screens on the one screen behaved differently.

Both routes now reset the button through the shared finishDelete(), the
one path they both take, and the lost-password leave hook no longer
handles it separately. Tests drive a password-route delete and a second
delete in the same session; they fail against the prior head, where the
button stays disabled after the first delete.

Model: opus-4-8
This commit is contained in:
2026-09-21 23:03:15 +00:00
parent ae61792aee
commit cac9b71709
3 changed files with 90 additions and 12 deletions
+11
View File
@@ -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 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 testnet or the runtime debug toggle still raises the banner but without the
view id. 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 - 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 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 wallet list was the reported case: the address shared one row with the
+14 -7
View File
@@ -51,16 +51,12 @@ function clear() {
// The lost-password screen holds no secret — a wallet name is not one — // 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 // but it is wiped on leave for the neighbouring reason: a typed
// confirmation left standing in a hidden view is one click away from // confirmation left standing in a hidden view is one click away from
// destroying a wallet the user has since navigated off. The button is // destroying a wallet the user has since navigated off.
// re-enabled here too, so a screen left mid-delete is usable on re-entry.
function clearLostPassword() { function clearLostPassword() {
lostPasswordIndex = null; lostPasswordIndex = null;
$("delete-wallet-lost-name-input").value = ""; $("delete-wallet-lost-name-input").value = "";
$("delete-wallet-lost-flash").textContent = ""; $("delete-wallet-lost-flash").textContent = "";
$("delete-wallet-lost-flash").style.visibility = "hidden"; $("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) { function show(walletIdx) {
@@ -98,6 +94,17 @@ function showLostPassword() {
// cleanup and the accountsChanged broadcast cannot drift apart between // cleanup and the accountsChanged broadcast cannot drift apart between
// them. // them.
async function finishDelete(walletIdx) { 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); const { activeAddressChanged } = removeWalletFromState(state, walletIdx);
deleteWalletIndex = null; deleteWalletIndex = null;
@@ -187,8 +194,8 @@ function init(_ctx) {
btn.disabled = true; btn.disabled = true;
btn.classList.add("text-muted"); btn.classList.add("text-muted");
// finishDelete() navigates, and the leave hook re-enables the // finishDelete() re-enables the button; navigating away then runs
// button and wipes the typed name on the way out. // the leave hook that wipes the typed name.
await finishDelete(lostPasswordIndex); await finishDelete(lostPasswordIndex);
}); });
+65 -5
View File
@@ -172,6 +172,15 @@ async function openLostPassword(deleteWallet, walletIdx) {
await click("btn-delete-wallet-lost-password"); 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 // ------------------------------------------------------------ tests
// The stub is what every persistence assertion below rests on, so its one // 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. // Both routes now re-enable through finishDelete(), not their leave
test("the confirm button is re-enabled on the way out", async () => { // hooks, so the button comes back live once a delete completes.
const { helpers, deleteWallet } = load(); test("the confirm button is re-enabled after a delete", async () => {
const { deleteWallet } = load();
await openLostPassword(deleteWallet, 1); await openLostPassword(deleteWallet, 1);
node("btn-delete-wallet-lost-confirm").disabled = true; node("delete-wallet-lost-name-input").value = "Wallet 2";
helpers.showView("settings"); await click("btn-delete-wallet-lost-confirm");
expect(node("btn-delete-wallet-lost-confirm").disabled).toBe(false); 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 // 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); 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",
]);
});
});