From ab1c1846a7300a6d5ba936726dce4208ffe65605 Mon Sep 17 00:00:00 2001 From: clawbot Date: Mon, 17 Aug 2026 08:59:59 +0200 Subject: [PATCH] fix: one wording for an empty password field on every screen (closes #265) --- TODO.md | 13 +++++ src/popup/views/exportPrivkey.js | 2 +- tests/passwordMessages.test.js | 90 +++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/TODO.md b/TODO.md index 430c99d..9cd8c86 100644 --- a/TODO.md +++ b/TODO.md @@ -46,6 +46,19 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-17: One wording for an empty password field on every screen that asks + for one. The private key export screen said "Password is required." where the + other five say "Please enter your password.", the same one-condition-two- + wordings split that [#172](https://git.eeqj.de/sneak/AutistMask/issues/172) + closed for a rejected password. Strings only, no behaviour change. + `tests/passwordMessages.test.js` now pins the empty-field guard per call site + as well as the decrypt handler, anchored on the `decryptWithPassword` sites so + the wallet-creation screen — where an empty field means a password being + chosen, a different condition — stays out of the set. Every error container + measured at a 360px viewport in the pinned Playwright container: the export + screen's container holds at 20px with the following section at the same offset + for the old string, the new string and the empty reserved state + ([#265](https://git.eeqj.de/sneak/AutistMask/issues/265)). - 2026-08-17: An address total no longer reports `$0.00` for holdings it cannot price. Prices exist for the top 25 tokens only, so the priced-only sum was printed as the total and an address holding nothing but unpriced ERC-20s was diff --git a/src/popup/views/exportPrivkey.js b/src/popup/views/exportPrivkey.js index dd60f51..a3113ae 100644 --- a/src/popup/views/exportPrivkey.js +++ b/src/popup/views/exportPrivkey.js @@ -112,7 +112,7 @@ function show(walletIdx, addrIdx) { async function reveal() { const password = $("export-privkey-password").value; if (!password) { - fail("Password is required."); + fail("Please enter your password."); return; } if (walletIndex === null) { diff --git a/tests/passwordMessages.test.js b/tests/passwordMessages.test.js index 494891c..d96d922 100644 --- a/tests/passwordMessages.test.js +++ b/tests/passwordMessages.test.js @@ -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) => {