fix: one wording for a rejected password on every screen (closes #172)
All checks were successful
check / check (push) Successful in 37s
All checks were successful
check / check (push) Successful in 37s
The send confirmation and the delete-wallet confirmation rendered
"Wrong password." — a fragment, which README Language & Labeling and
RULES.md:120 both forbid — while the two reveal screens said "That
password is not correct." and the two dApp approval paths said "That
password is incorrect." Three wordings for one condition, on screens a
user can reach minutes apart.
All five decryptWithPassword call sites now show the wording the
approval paths introduced:
That password is incorrect. Please try again.
Strings only. Nothing about how a wrong password is handled changes: it
still fails closed on every screen, and the approval paths' settlement,
claim/release interlock and retry behaviour are untouched.
The new test scans the source for the call sites rather than driving
each view, because the invariant is about the set: a sixth screen that
decrypts the vault has to join it, and a per-view test cannot notice a
screen nobody wrote one for.
This commit is contained in:
79
tests/passwordMessages.test.js
Normal file
79
tests/passwordMessages.test.js
Normal file
@@ -0,0 +1,79 @@
|
||||
// One wording for one condition (issue #172).
|
||||
//
|
||||
// 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
|
||||
// confirmation and the delete-wallet confirmation said "Wrong password."
|
||||
// (a fragment, which RULES.md Language & Labeling forbids), the reveal
|
||||
// screens said "That password is not correct.", and the two dApp approval
|
||||
// paths said "That password is incorrect." A user hitting two of those
|
||||
// minutes apart had no way to tell whether the wallet meant the same
|
||||
// thing.
|
||||
//
|
||||
// This scans the source rather than driving six views, because the
|
||||
// invariant is about the set of call sites and not about any one of them:
|
||||
// a seventh screen that decrypts the vault has to join the set, and a
|
||||
// DOM test per view cannot notice one that was never written.
|
||||
|
||||
const fs = require("fs");
|
||||
const path = require("path");
|
||||
|
||||
const SRC = path.join(__dirname, "..", "src");
|
||||
|
||||
const CANONICAL = "That password is incorrect. Please try again.";
|
||||
|
||||
// Wordings this repo has actually shipped for the same condition. A
|
||||
// literal match, so a new variant is caught by the "only the canonical
|
||||
// sentence appears" assertion below rather than by guessing at phrasing.
|
||||
const SUPERSEDED = [
|
||||
"Wrong password.",
|
||||
"That password is not correct. Please try again.",
|
||||
];
|
||||
|
||||
function jsFilesUnder(dir) {
|
||||
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) return jsFilesUnder(full);
|
||||
return entry.name.endsWith(".js") ? [full] : [];
|
||||
});
|
||||
}
|
||||
|
||||
// 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
|
||||
// no longer names.
|
||||
function callSites() {
|
||||
return jsFilesUnder(SRC).filter((file) => {
|
||||
if (file === path.join(SRC, "shared", "vault.js")) return false;
|
||||
return fs.readFileSync(file, "utf8").includes("decryptWithPassword(");
|
||||
});
|
||||
}
|
||||
|
||||
describe("password failure messages", () => {
|
||||
const files = callSites();
|
||||
|
||||
test("the call sites are found where they are expected", () => {
|
||||
const found = files
|
||||
.map((f) => path.relative(SRC, f).split(path.sep).join("/"))
|
||||
.sort();
|
||||
expect(found).toEqual([
|
||||
"popup/views/approval.js",
|
||||
"popup/views/confirmTx.js",
|
||||
"popup/views/deleteWallet.js",
|
||||
"popup/views/exportPrivkey.js",
|
||||
"popup/views/showPhrase.js",
|
||||
]);
|
||||
});
|
||||
|
||||
test("the canonical message is a full sentence", () => {
|
||||
expect(CANONICAL).toMatch(/^[A-Z][^]*\.$/);
|
||||
});
|
||||
|
||||
test.each(files)("%s tells the user the canonical sentence", (f) => {
|
||||
expect(fs.readFileSync(f, "utf8")).toContain(CANONICAL);
|
||||
});
|
||||
|
||||
test.each(files)("%s carries no superseded wording", (f) => {
|
||||
const source = fs.readFileSync(f, "utf8");
|
||||
for (const old of SUPERSEDED) expect(source).not.toContain(old);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user