From 3f3bfcb010085e1ac3e221cdfa2a6bb092d5a004 Mon Sep 17 00:00:00 2001 From: clawbot Date: Mon, 17 Aug 2026 06:32:57 +0000 Subject: [PATCH] fix: one wording for an empty password field on every screen (closes #265) The private key export screen answered an empty password field with "Password is required." where the other five screens that ask for the password say "Please enter your password." Same condition, two sentences, and a user hitting both had no reason to think they meant the same thing. The majority wording wins; the export screen now says it too. Strings only. Every screen still refuses to proceed on an empty field exactly as before. tests/passwordMessages.test.js already pinned the rejected-password sentence per decryptWithPassword call site; it now pins the empty-field sentence the same way, walking each decrypt's password variable back to the guard that rejects it blank and requiring the prose there to be the canonical sentence and nothing else. Anchoring on the decrypt keeps the wallet-creation screen out of the set, where an empty field means a password being chosen rather than entered -- a different condition with its own wording. Every error container measured at a 360px viewport in the pinned Playwright container against the built popup. The export screen's container holds at 20px with the section below it at the same offset for the empty reserved state, the old string and the new one. --- 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) => {