fix: show every password error in its own line below the field (closes #493)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run

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. On the add wallet screen the line sits beside the
Import button, which keeps its place at 360x600. 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
This commit is contained in:
2026-10-07 07:49:59 +00:00
parent 29ba54d5b6
commit e4116ecd4a
12 changed files with 386 additions and 79 deletions
+2
View File
@@ -39,6 +39,8 @@ jest.doMock("../src/popup/views/helpers", () => ({
$: element,
showView: () => {},
showFlash: () => {},
showError: () => {},
hideError: () => {},
goBack: () => {},
clearViewStack: () => {},
onViewLeave: () => {},
+1 -1
View File
@@ -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.",
);
});
+109 -6
View File
@@ -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 },
);
@@ -1993,13 +1994,14 @@ test("an over-long flash message keeps to one line (#252)", async (env) => {
// Every screen that asks for a password reserves room for one line of error.
// The two on the dApp approval screens also have a border and padding, which
// that reserved height has to cover too.
// that reserved height has to cover too. The add wallet screen's line sits
// beside its button rather than above it, and has its own test below.
const PASSWORD_ERROR_CONTAINERS = [
"approve-tx-error",
"approve-sign-error",
"export-privkey-flash",
"show-phrase-flash",
"delete-wallet-flash",
"export-privkey-password-error",
"show-phrase-password-error",
"delete-wallet-password-error",
"confirm-tx-password-error",
];
@@ -2064,6 +2066,107 @@ test("a password error moves nothing on any screen (#297)", async (env) => {
}
});
// ------------------------------------------ add wallet Import button (#493)
// At 360x600 the add wallet screen's Import button already starts near the
// bottom of the popup, so its password error line sits beside the button
// rather than above it. Measures the button and the line empty and again
// filled with the longest error addWallet.js puts there. Runs in the page.
function measureImportButton() {
const button = document.getElementById("btn-add-wallet-confirm");
const line = document.getElementById("add-wallet-password-error");
const measure = () => {
const b = button.getBoundingClientRect();
const l = line.getBoundingClientRect();
return {
buttonTop: b.top + window.scrollY,
buttonBottom: b.bottom + window.scrollY,
lineTop: l.top + window.scrollY,
lineBottom: l.bottom + window.scrollY,
};
};
const empty = measure();
line.textContent = "Password must be at least 12 characters.";
line.style.visibility = "visible";
const filled = measure();
line.textContent = "";
line.style.visibility = "hidden";
return { empty, filled };
}
test("the add wallet password error leaves Import where it was (#493)", async (env) => {
const page = await openPopup(env.ctx, env.popupUrl);
try {
await page.setViewportSize(POPUP_VIEWPORT);
// Brought up by toggling classes, as in the test above. The note
// addWallet.js shows once a wallet exists is the only part of the
// screen that differs between the first wallet and a later one.
await page.evaluate(() => {
const screen = document.getElementById("view-add-wallet");
for (const view of document.querySelectorAll(".view")) {
view.classList.toggle("hidden", view !== screen);
}
});
for (const walletExists of [false, true]) {
await page.evaluate(
(shown) =>
document
.getElementById("add-wallet-separate-password-note")
.classList.toggle("hidden", !shown),
walletExists,
);
for (const tab of ["tab-mnemonic", "tab-privkey", "tab-xprv"]) {
await page.click("#" + tab);
const { empty, filled } =
await page.evaluate(measureImportButton);
const where =
"#" +
tab +
(walletExists
? " with a wallet already added"
: " for the first wallet");
// Printed pass or fail, as the dust threshold test does.
console.log(
"# add wallet Import top, " +
where +
": " +
empty.buttonTop +
"px",
);
for (const m of [empty, filled]) {
assert(
m.lineTop >= m.buttonTop &&
m.lineBottom <= m.buttonBottom,
"the error line on " +
where +
" does not fit beside Import, so it adds height: " +
JSON.stringify(m),
);
}
assert(
filled.buttonTop === empty.buttonTop,
"Import moved " +
(filled.buttonTop - empty.buttonTop) +
"px when the error appeared on " +
where,
);
if (!walletExists) {
assert(
empty.buttonTop < POPUP_VIEWPORT.height,
"Import starts at " +
empty.buttonTop +
"px on " +
where +
", below the fold",
);
}
}
}
} finally {
await page.close();
}
});
// --------------------------------------------- confirmation screen (#238)
//
// The screen that decides what gets signed. The arithmetic underneath it
+6 -4
View File
@@ -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.",
);
});
+159
View File
@@ -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);
});
});