fix: one wording for an empty password field on every screen (closes #265)
All checks were successful
check / check (push) Successful in 29s
e2e / e2e-chrome (push) Successful in 48s
e2e / e2e-firefox (push) Successful in 18s

This commit was merged in pull request #296.
This commit is contained in:
2026-08-17 08:59:59 +02:00
parent 743b1962a5
commit ab1c1846a7
3 changed files with 102 additions and 3 deletions

View File

@@ -1,4 +1,4 @@
// One wording for one condition (issue #172).
// One wording for one condition (issues #172 and #265).
//
// Every screen that asks for the password decrypts the vault itself, and
// each one used to write its own sentence for the same failure: the send
@@ -21,6 +21,15 @@
// call site is read back to its own catch handler and the prose that
// handler shows the user must be the canonical sentence and nothing else
// — which fails on a novel wording, not only on a known-superseded one.
//
// The empty-password condition (#265) is pinned the same way and off the
// same call sites: the private key export screen said "Password is
// required." where the other five said "Please enter your password." Each
// decrypt's password variable is walked back to the guard that rejects it
// when blank, and the prose that guard shows must be the canonical
// sentence. Anchoring on the decrypt keeps the wallet-creation screen out
// of the set: an empty field there is a password being CHOSEN, a
// different condition with its own wording.
const fs = require("fs");
const path = require("path");
@@ -28,6 +37,7 @@ const path = require("path");
const SRC = path.join(__dirname, "..", "src");
const CANONICAL = "That password is incorrect. Please try again.";
const CANONICAL_EMPTY = "Please enter your password.";
// Wordings this repo has actually shipped for the same condition. This is
// a secondary, whole-file sweep for stragglers outside a decrypt handler;
@@ -36,6 +46,7 @@ const CANONICAL = "That password is incorrect. Please try again.";
const SUPERSEDED = [
"Wrong password.",
"That password is not correct. Please try again.",
"Password is required.",
];
function jsFilesUnder(dir) {
@@ -149,6 +160,71 @@ function handlerMessages(file, callOffset, label) {
.filter((v) => v.includes(" "));
}
// The identifier a decrypt call passes as its password, which is what the
// empty-field guard for that screen tests.
function passwordArg(masked, callOffset, label) {
const open = callOffset + "decryptWithPassword".length;
const args = [];
let depth = 0;
let start = open + 1;
for (let i = open; i < masked.length; i++) {
const c = masked[i];
if (c === "(" || c === "[" || c === "{") depth += 1;
else if (c === ")" || c === "]" || c === "}") {
depth -= 1;
if (depth === 0) {
args.push(masked.slice(start, i));
break;
}
} else if (c === "," && depth === 1) {
args.push(masked.slice(start, i));
start = i + 1;
}
}
const arg = (args[1] ?? "").trim();
if (!/^[A-Za-z_$][\w$]*$/.test(arg))
throw new Error(`${label}: password argument is not a name: ${arg}`);
return arg;
}
// Innermost block enclosing the decrypt that also declares its password
// variable — the handler the screen's submit button runs, which is where
// the empty-field guard lives.
function declaringBlock(masked, callOffset, ident, label) {
const declared = new RegExp(`\\b(?:const|let|var)\\s+${ident}\\s*=`);
let at = callOffset;
for (;;) {
const open = enclosingBlockStart(masked, at);
if (open === -1) throw new Error(`${label}: nothing declares ${ident}`);
const end = blockEnd(masked, open);
if (declared.test(masked.slice(open, end))) return [open, end];
at = open - 1;
}
}
// The prose the empty-field guard puts in front of the user. Exactly one
// guard per handler is required: two would mean the condition is answered
// in more than one place and this would be pinning only one of them.
function emptyGuardMessages(file, callOffset, label) {
const { masked, strings } = scan(fs.readFileSync(file, "utf8"));
const ident = passwordArg(masked, callOffset, label);
const [from, to] = declaringBlock(masked, callOffset, ident, label);
const guard = new RegExp(`if\\s*\\(\\s*!\\s*${ident}\\s*\\)\\s*\\{`, "g");
const opens = [];
let m;
while ((m = guard.exec(masked.slice(from, to))) !== null)
opens.push(from + m.index + m[0].length - 1);
if (opens.length !== 1)
throw new Error(
`${label}: expected one empty-${ident} guard, found ${opens.length}`,
);
const close = blockEnd(masked, opens[0]);
return strings
.filter((s) => s.offset >= opens[0] && s.offset < close)
.map((s) => s.value)
.filter((v) => v.includes(" "));
}
// The call sites are found, not listed: the file layout moves (the private
// key export was in addressDetail.js when #172 was filed and is its own
// view now), and a hardcoded list would quietly stop covering a screen it
@@ -187,8 +263,9 @@ describe("password failure messages", () => {
});
});
test("the canonical message is a full sentence", () => {
test("the canonical messages are full sentences", () => {
expect(CANONICAL).toMatch(/^[A-Z][^]*\.$/);
expect(CANONICAL_EMPTY).toMatch(/^[A-Z][^]*\.$/);
});
// Exact equality, per call site: a message that is merely different
@@ -203,6 +280,15 @@ describe("password failure messages", () => {
},
);
test.each(sites.map((s) => [s.label, s]))(
"%s answers an empty password field with the canonical sentence",
(label, site) => {
expect(emptyGuardMessages(site.file, site.offset, label)).toEqual([
CANONICAL_EMPTY,
]);
},
);
test.each(files.map((f) => [path.relative(SRC, f), f]))(
"%s carries no superseded wording",
(_rel, file) => {