From d5d48d35f8aa094e5e67a6219d9ad9f49d961bac Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 7 Oct 2026 06:21:21 +0000 Subject: [PATCH] fix: show every password error in its own line below the field (closes #493) The add wallet screen reported a missing, short or mismatched password in the flash line at the top of the popup, and the private key export, recovery phrase and delete wallet screens each wrote to a line of their own above the password field. All four now use showError() and hideError() with a fixed-height error line below the field, as the send confirmation and approval screens do. The line clears when the screen is shown again and when the password is tried again. Other add wallet messages stay in the flash line. Model: opus-5-5 --- README.md | 17 +-- TODO.md | 12 ++ src/popup/index.html | 32 ++--- src/popup/views/addWallet.js | 16 ++- src/popup/views/deleteWallet.js | 27 +++-- src/popup/views/exportPrivkey.js | 25 ++-- src/popup/views/showPhrase.js | 27 +++-- tests/addressScanCancelled.test.js | 2 + tests/deleteWalletLostPassword.test.js | 2 +- tests/e2e/run.js | 12 +- tests/exportPrivkey.test.js | 10 +- tests/passwordErrorLines.test.js | 159 +++++++++++++++++++++++++ 12 files changed, 270 insertions(+), 71 deletions(-) create mode 100644 tests/passwordErrorLines.test.js diff --git a/README.md b/README.md index 317d142..6e549d6 100644 --- a/README.md +++ b/README.md @@ -1444,14 +1444,17 @@ view would leave a wallet one click from deletion. without it, the lost-password route on DeleteWallet is the first they would hear of it. The hint line reserves its height, so switching tabs cannot move the password fields under the pointer. + - Error line - "Import" button - **Transitions**: - "Import" with a valid entry and a matching password of at least 12 characters → creates the wallet, clears the navigation stack, and → **Home**. The phrase and xprv modes then scan for further used addresses and report the count as a flash message. - - "Import" with an invalid entry, a duplicate wallet or address, or a short - or mismatched password → flash message, no screen change + - "Import" with a missing, short or mismatched password → full-sentence + error on the error line, no screen change + - "Import" with an invalid entry or a duplicate wallet or address → flash + message, no screen change - "Back" → previous screen (Welcome, Home, or Settings) #### AddressDetail (`address`) @@ -1490,8 +1493,8 @@ view would leave a wallet one click from deletion. copy) - Warning that anyone holding the private key can transfer all funds from the address - - Error line - - Password input and "Reveal" button, shown until the key is revealed + - Password input, error line and "Reveal" button, shown until the key is + revealed - The private key on a highlighted background, tap to copy, shown only after the password has been accepted - **Transitions**: @@ -1829,8 +1832,8 @@ view would leave a wallet one click from deletion. - Wallet name - Warning box stating that anyone holding these words can take everything in the wallet, from any device, without the password - - Error line - - Password input + "Reveal" button, shown until the password is accepted + - Password input, error line and "Reveal" button, shown until the password + is accepted - The recovery phrase itself, in full and click-to-copy, shown only after a correct password and in place of the password prompt - **Transitions**: @@ -1859,8 +1862,8 @@ view would leave a wallet one click from deletion. - "Back" button, "Delete Wallet" heading - Warning naming the wallet and stating that deletion is permanent and any funds are unrecoverable without the recovery phrase - - Error line - Password input + - Error line - "Confirm Delete" button - An underlined "I have lost my password" control - **Transitions**: diff --git a/TODO.md b/TODO.md index d33ad3b..52ed40f 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,18 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-07: Every password error in the popup is shown the same way + ([#493](https://git.eeqj.de/sneak/AutistMask/issues/493)): with `showError()` + and `hideError()` in a fixed-height error line below the password field, as + the send confirmation and approval screens already did. The add wallet screen + showed a missing, short or mismatched password in the flash line, and the + private key export, recovery phrase and delete wallet screens each had a line + of their own above the field. Each line clears when the screen is shown again + and when the password is tried again. The add wallet screen's other messages, + such as an invalid recovery phrase, a duplicate wallet and the address scan, + stay in the flash line. `tests/passwordErrorLines.test.js` drives all four + screens in the popup. + - 2026-10-07: The README says that the wallet never clears the clipboard after the private key or the recovery phrase is copied, and why ([#492](https://git.eeqj.de/sneak/AutistMask/issues/492)): the clipboard is diff --git a/src/popup/index.html b/src/popup/index.html index f92c9c5..6d3dddd 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -207,6 +207,10 @@ class="border border-border p-1 w-full font-mono text-sm bg-bg text-fg" /> + @@ -1116,10 +1120,6 @@ is permanent. Any funds will be unrecoverable without your recovery phrase.

-
+ diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js index 97f7f9a..b6e0c3e 100644 --- a/src/popup/views/addWallet.js +++ b/src/popup/views/addWallet.js @@ -2,6 +2,8 @@ const { $, showView, showFlash, + showError, + hideError, goBack, clearViewStack, onViewLeave, @@ -98,6 +100,7 @@ function clear() { $("add-wallet-password").value = ""; $("add-wallet-password-confirm").value = ""; $("add-wallet-phrase-warning").style.visibility = "hidden"; + hideError("add-wallet-password-error"); } // Each wallet has its own password (its own encryptedSecret), so adding a @@ -125,15 +128,18 @@ function validatePassword() { const pw = $("add-wallet-password").value; const pw2 = $("add-wallet-password-confirm").value; if (!pw) { - showFlash("Please choose a password."); + showError("add-wallet-password-error", "Please choose a password."); return null; } if (pw.length < 12) { - showFlash("Password must be at least 12 characters."); + showError( + "add-wallet-password-error", + "Password must be at least 12 characters.", + ); return null; } if (pw !== pw2) { - showFlash("Passwords do not match."); + showError("add-wallet-password-error", "Passwords do not match."); return null; } return pw; @@ -342,8 +348,10 @@ function init(ctx) { $("add-wallet-phrase-warning").style.visibility = "visible"; }); - // Import / confirm + // Import / confirm. Each press starts with no password error on screen: + // validatePassword() puts it back if the password is still wrong. $("btn-add-wallet-confirm").addEventListener("click", async () => { + hideError("add-wallet-password-error"); if (currentMode === "mnemonic") { await importMnemonic(ctx); } else if (currentMode === "privkey") { diff --git a/src/popup/views/deleteWallet.js b/src/popup/views/deleteWallet.js index e7bbb89..5808fcb 100644 --- a/src/popup/views/deleteWallet.js +++ b/src/popup/views/deleteWallet.js @@ -2,6 +2,8 @@ const { $, showView, showFlash, + showError, + hideError, goBack, clearViewStack, onViewLeave, @@ -55,8 +57,7 @@ function confirmKey(name) { function clear() { deleteWalletIndex = null; $("delete-wallet-password").value = ""; - $("delete-wallet-flash").textContent = ""; - $("delete-wallet-flash").style.visibility = "hidden"; + hideError("delete-wallet-password-error"); } // The lost-password screen holds no secret — a wallet name is not one — @@ -232,19 +233,22 @@ function init(_ctx) { $("btn-delete-wallet-confirm").addEventListener("click", async () => { const pw = $("delete-wallet-password").value; if (!pw) { - $("delete-wallet-flash").textContent = - "Please enter your password."; - $("delete-wallet-flash").style.visibility = "visible"; + showError( + "delete-wallet-password-error", + "Please enter your password.", + ); return; } if (deleteWalletIndex === null) { - $("delete-wallet-flash").textContent = - "No wallet selected for deletion."; - $("delete-wallet-flash").style.visibility = "visible"; + showError( + "delete-wallet-password-error", + "No wallet selected for deletion.", + ); return; } + hideError("delete-wallet-password-error"); const btn = $("btn-delete-wallet-confirm"); btn.disabled = true; btn.classList.add("text-muted"); @@ -256,9 +260,10 @@ function init(_ctx) { try { await decryptWithPassword(wallet.encryptedSecret, pw); } catch { - $("delete-wallet-flash").textContent = - "That password is incorrect. Please try again."; - $("delete-wallet-flash").style.visibility = "visible"; + showError( + "delete-wallet-password-error", + "That password is incorrect. Please try again.", + ); btn.disabled = false; btn.classList.remove("text-muted"); return; diff --git a/src/popup/views/exportPrivkey.js b/src/popup/views/exportPrivkey.js index b027c13..3860387 100644 --- a/src/popup/views/exportPrivkey.js +++ b/src/popup/views/exportPrivkey.js @@ -20,6 +20,8 @@ const { $, showView, showFlash, + showError, + hideError, flashCopyFeedback, goBack, onViewLeave, @@ -56,11 +58,6 @@ function isCurrentReveal(generation) { ); } -function fail(message) { - $("export-privkey-flash").textContent = message; - $("export-privkey-flash").style.visibility = "visible"; -} - // Wipe every trace of the key and drop the address selection. Safe to call // when nothing was ever revealed, and safe to call twice. function clear() { @@ -71,8 +68,7 @@ function clear() { $("export-privkey-password").value = ""; $("export-privkey-result").classList.add("hidden"); $("export-privkey-password-section").classList.remove("hidden"); - $("export-privkey-flash").textContent = ""; - $("export-privkey-flash").style.visibility = "hidden"; + hideError("export-privkey-password-error"); } function show(walletIdx, addrIdx) { @@ -112,15 +108,19 @@ function show(walletIdx, addrIdx) { async function reveal() { const password = $("export-privkey-password").value; if (!password) { - fail("Please enter your password."); + showError( + "export-privkey-password-error", + "Please enter your password.", + ); return; } if (walletIndex === null) { - fail("No address is selected."); + showError("export-privkey-password-error", "No address is selected."); return; } const wallet = state.wallets[walletIndex]; + hideError("export-privkey-password-error"); const btn = $("btn-export-privkey-confirm"); btn.disabled = true; btn.classList.add("text-muted"); @@ -140,11 +140,12 @@ async function reveal() { $("export-privkey-password-section").classList.add("hidden"); $("export-privkey-value").textContent = signer.privateKey; $("export-privkey-result").classList.remove("hidden"); - $("export-privkey-flash").textContent = ""; - $("export-privkey-flash").style.visibility = "hidden"; } catch { if (!isCurrentReveal(generation)) return; - fail("That password is incorrect. Please try again."); + showError( + "export-privkey-password-error", + "That password is incorrect. Please try again.", + ); } finally { btn.disabled = false; btn.classList.remove("text-muted"); diff --git a/src/popup/views/showPhrase.js b/src/popup/views/showPhrase.js index 839cf67..482dbe9 100644 --- a/src/popup/views/showPhrase.js +++ b/src/popup/views/showPhrase.js @@ -21,6 +21,8 @@ const { $, showView, showFlash, + showError, + hideError, flashCopyFeedback, goBack, onViewLeave, @@ -52,11 +54,6 @@ function isCurrentReveal(generation) { ); } -function fail(message) { - $("show-phrase-flash").textContent = message; - $("show-phrase-flash").style.visibility = "visible"; -} - // Wipe every trace of the phrase and drop the wallet selection. Safe to // call when nothing was ever revealed, and safe to call twice. function clear() { @@ -66,8 +63,7 @@ function clear() { $("show-phrase-password").value = ""; $("show-phrase-result").classList.add("hidden"); $("show-phrase-password-section").classList.remove("hidden"); - $("show-phrase-flash").textContent = ""; - $("show-phrase-flash").style.visibility = "hidden"; + hideError("show-phrase-password-error"); } function show(walletIdx) { @@ -90,19 +86,23 @@ function show(walletIdx) { async function reveal() { const password = $("show-phrase-password").value; if (!password) { - fail("Please enter your password."); + showError("show-phrase-password-error", "Please enter your password."); return; } if (walletIndex === null) { - fail("No wallet is selected."); + showError("show-phrase-password-error", "No wallet is selected."); return; } const wallet = state.wallets[walletIndex]; if (!walletHasRecoveryPhrase(wallet)) { - fail("This wallet does not have a recovery phrase."); + showError( + "show-phrase-password-error", + "This wallet does not have a recovery phrase.", + ); return; } + hideError("show-phrase-password-error"); const btn = $("btn-show-phrase-reveal"); btn.disabled = true; btn.classList.add("text-muted"); @@ -120,13 +120,14 @@ async function reveal() { $("show-phrase-password-section").classList.add("hidden"); $("show-phrase-value").textContent = phrase; $("show-phrase-result").classList.remove("hidden"); - $("show-phrase-flash").textContent = ""; - $("show-phrase-flash").style.visibility = "hidden"; } catch { if (!isCurrentReveal(generation)) return; // Deliberately not the caught error: the message is fixed so that // nothing derived from the ciphertext or the attempt can surface. - fail("That password is incorrect. Please try again."); + showError( + "show-phrase-password-error", + "That password is incorrect. Please try again.", + ); } finally { btn.disabled = false; btn.classList.remove("text-muted"); diff --git a/tests/addressScanCancelled.test.js b/tests/addressScanCancelled.test.js index 984a51d..febfd63 100644 --- a/tests/addressScanCancelled.test.js +++ b/tests/addressScanCancelled.test.js @@ -39,6 +39,8 @@ jest.doMock("../src/popup/views/helpers", () => ({ $: element, showView: () => {}, showFlash: () => {}, + showError: () => {}, + hideError: () => {}, goBack: () => {}, clearViewStack: () => {}, onViewLeave: () => {}, diff --git a/tests/deleteWalletLostPassword.test.js b/tests/deleteWalletLostPassword.test.js index 4cc8877..0ba4df4 100644 --- a/tests/deleteWalletLostPassword.test.js +++ b/tests/deleteWalletLostPassword.test.js @@ -253,7 +253,7 @@ describe("reaching the screen", () => { const { decryptWithPassword } = require("../src/shared/vault"); decryptWithPassword.mockRejectedValue(new Error("nope")); await click("btn-delete-wallet-confirm"); - expect(node("delete-wallet-flash").textContent).toBe( + expect(node("delete-wallet-password-error").textContent).toBe( "That password is incorrect. Please try again.", ); }); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 8723f03..caba3b3 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -809,7 +809,7 @@ async function secretScreenState(page, view) { return page.evaluate( (v) => ({ value: document.getElementById(v + "-value").textContent, - error: document.getElementById(v + "-flash").textContent, + error: document.getElementById(v + "-password-error").textContent, html: document.getElementById("view-" + v).innerHTML, resultHidden: document .getElementById(v + "-result") @@ -906,7 +906,8 @@ test("a wrong password reveals nothing (#161)", async (env) => { await env.page.click("#btn-show-phrase-reveal"); await env.page.waitForFunction( () => - document.getElementById("show-phrase-flash").textContent.length > 0, + document.getElementById("show-phrase-password-error").textContent + .length > 0, null, { timeout: 60000 }, ); @@ -1997,9 +1998,10 @@ test("an over-long flash message keeps to one line (#252)", async (env) => { const PASSWORD_ERROR_CONTAINERS = [ "approve-tx-error", "approve-sign-error", - "export-privkey-flash", - "show-phrase-flash", - "delete-wallet-flash", + "add-wallet-password-error", + "export-privkey-password-error", + "show-phrase-password-error", + "delete-wallet-password-error", "confirm-tx-password-error", ]; diff --git a/tests/exportPrivkey.test.js b/tests/exportPrivkey.test.js index 8629431..75f8081 100644 --- a/tests/exportPrivkey.test.js +++ b/tests/exportPrivkey.test.js @@ -207,7 +207,7 @@ describe("a decrypt still running when the screen is left", () => { }); // Same hole on the failure path: a wrong-password error written after - // the wipe would restore the flash line on a screen the user has left. + // the wipe would restore the error line on a screen the user has left. test("never writes the failure message either", async () => { const { helpers, vault, exportPrivkey } = load(); exportPrivkey.show(0, 0); @@ -217,8 +217,10 @@ describe("a decrypt still running when the screen is left", () => { reveal.reject(new Error("decryption failed")); await reveal.pending; - expect(node("export-privkey-flash").textContent).toBe(""); - expect(node("export-privkey-flash").style.visibility).toBe("hidden"); + expect(node("export-privkey-password-error").textContent).toBe(""); + expect(node("export-privkey-password-error").style.visibility).toBe( + "hidden", + ); }); }); @@ -260,7 +262,7 @@ describe("a reveal that is not interrupted", () => { await reveal.pending; expect(node("export-privkey-value").textContent).toBe(""); - expect(node("export-privkey-flash").textContent).toBe( + expect(node("export-privkey-password-error").textContent).toBe( "That password is incorrect. Please try again.", ); }); diff --git a/tests/passwordErrorLines.test.js b/tests/passwordErrorLines.test.js new file mode 100644 index 0000000..bbc09f5 --- /dev/null +++ b/tests/passwordErrorLines.test.js @@ -0,0 +1,159 @@ +// Every screen that asks for a password shows a password error the same way: +// through showError() and hideError() in src/popup/views/helpers.js, in a +// fixed-height error line below the password field, and never in the flash +// line at the top of the popup +// (https://git.eeqj.de/sneak/AutistMask/issues/493). These boot the real popup +// over src/popup/index.html, so each error line has to exist in the markup, +// and check that the error appears in it and clears again. + +jest.mock("../src/shared/vault", () => ({ + decryptWithPassword: jest.fn(), + encryptWithPassword: jest.fn(), +})); + +const { + bootPopup, + cleanupPopup, + unversionedValidProfile, +} = require("./support/popupBoot"); + +const PASSWORD = "correct horse battery staple"; +const WRONG_PASSWORD = "That password is incorrect. Please try again."; + +afterEach(() => { + cleanupPopup(); +}); + +// The error line as the user sees it. +function errorLine(page, id) { + return { + inMarkup: page.document.authoredIds.has(id), + text: page.text(id), + visibility: page.node(id).style.visibility, + }; +} + +function shown(text) { + return { inMarkup: true, text, visibility: "visible" }; +} + +const cleared = { inMarkup: true, text: "", visibility: "hidden" }; + +describe("the add wallet screen", () => { + const ERROR = "add-wallet-password-error"; + + // First run: Welcome, "Add wallet", then the die for a valid phrase. + async function openAddWallet() { + const page = await bootPopup(undefined); + await page.click("btn-welcome-add"); + await page.click("btn-generate-phrase"); + return page; + } + + function setPasswords(page, password, confirm) { + page.node("add-wallet-password").value = password; + page.node("add-wallet-password-confirm").value = confirm; + } + + test("shows a password problem below the password fields, and clears it on the next press", async () => { + const page = await openAddWallet(); + setPasswords(page, "short", "short"); + await page.click("btn-add-wallet-confirm"); + expect(errorLine(page, ERROR)).toEqual( + shown("Password must be at least 12 characters."), + ); + expect(page.text("flash-msg")).toBe(""); + + // The password is fixed and the phrase emptied: the password error + // goes, and the phrase problem is still reported in the flash line. + setPasswords(page, PASSWORD, PASSWORD); + page.node("wallet-mnemonic").value = ""; + await page.click("btn-add-wallet-confirm"); + expect(errorLine(page, ERROR)).toEqual(cleared); + expect(page.text("flash-msg")).toBe( + "Enter a recovery phrase, or press the die.", + ); + + // Leaving clears the flash line and stops its timer, which would + // otherwise fire after this page is gone. + await page.click("btn-add-wallet-back"); + }); + + test("clears the error when the screen is shown again", async () => { + const page = await openAddWallet(); + setPasswords(page, PASSWORD, PASSWORD + " typo"); + await page.click("btn-add-wallet-confirm"); + expect(errorLine(page, ERROR)).toEqual( + shown("Passwords do not match."), + ); + + await page.click("btn-add-wallet-back"); + await page.click("btn-welcome-add"); + expect(errorLine(page, ERROR)).toEqual(cleared); + }); +}); + +describe.each([ + { + screen: "the private key export screen", + open: () => require("../src/popup/views/exportPrivkey").show(0, 0), + field: "export-privkey-password", + button: "btn-export-privkey-confirm", + error: "export-privkey-password-error", + }, + { + screen: "the recovery phrase screen", + open: () => require("../src/popup/views/showPhrase").show(0), + field: "show-phrase-password", + button: "btn-show-phrase-reveal", + error: "show-phrase-password-error", + }, + { + screen: "the delete wallet screen", + open: () => require("../src/popup/views/deleteWallet").show(0), + field: "delete-wallet-password", + button: "btn-delete-wallet-confirm", + error: "delete-wallet-password-error", + }, +])("$screen", ({ open, field, button, error }) => { + // Opens the screen and enters a password the vault rejects. + async function failedAttempt() { + const page = await bootPopup(unversionedValidProfile()); + open(); + const { decryptWithPassword } = require("../src/shared/vault"); + decryptWithPassword.mockRejectedValue(new Error("wrong password")); + page.node(field).value = "not the password"; + await page.click(button); + return { page, decryptWithPassword }; + } + + test("shows a wrong password below the password field", async () => { + const { page } = await failedAttempt(); + expect(errorLine(page, error)).toEqual(shown(WRONG_PASSWORD)); + }); + + test("clears the error while the next password is checked", async () => { + const { page, decryptWithPassword } = await failedAttempt(); + let rejectDecrypt; + decryptWithPassword.mockReturnValue( + new Promise((resolve, reject) => { + rejectDecrypt = reject; + }), + ); + + page.node(field).value = "another guess"; + const pressed = page.click(button); + await page.settle(); + expect(errorLine(page, error)).toEqual(cleared); + + rejectDecrypt(new Error("wrong password")); + await pressed; + expect(errorLine(page, error)).toEqual(shown(WRONG_PASSWORD)); + }); + + test("clears the error when the screen is shown again", async () => { + const { page } = await failedAttempt(); + open(); + expect(errorLine(page, error)).toEqual(cleared); + }); +});