Compare commits

..

1 Commits

Author SHA1 Message Date
5d8d18f9cd test: drive the Settings screen in a browser and guard every popup element id (closes #229)
Some checks failed
e2e / e2e-chrome (push) Has been cancelled
e2e / e2e-firefox (push) Has been cancelled
check / check (push) Has been cancelled
Nothing exercised the Settings view in a browser, and jest runs in the
node environment with no DOM, so the densest run of $("...") lookups in
the codebase was unverified at runtime. A wrong id is valid JavaScript
naming a defined function: $() returns null and the next property access
throws, which inside a view's init() aborts the rest of the popup's
init() and leaves every screen blank.

Two halves, because they catch different things.

The e2e suite (tests/e2e/run.js) gains seven cases between the address
removal and dust threshold sections. They assert the About well and the
wallet list were actually written — show() populates those near its end,
only the debug well and the debug-mode checkbox follow, so reading them
back proves show() ran through to there rather than just far enough to
unhide the section — that the four Token Spam Protection controls are
real input[type=checkbox] elements defaulted on, and that the theme and
network selectors offer exactly the choices src/shared/networks.js and
index.html define.

What the selectors persist is asserted by a round trip through
NON-DEFAULT values: they are driven to dark and sepolia, the popup is
closed and reopened, both are read back, and both are then restored the
same way and reasserted after a second reopen. Neither value is the
first <option> of its <select>, which is the point — the first option is
what the DOM reports with no JavaScript having run at all, so asserting
it would pass just as happily against a Settings screen that assigned
nothing. One spam filter is likewise toggled off and back on across a
reopen each way. Those round trips run the change handler, saveState(),
loadState() and the assignments show() and init() make, rather than only
looking at the screen. Each group records a coverage key and a final
case demands the exact set, so a section that silently stopped running
reddens the suite instead of shrinking it.

show() no longer wraps its settings-network lookup in if (networkSelect),
and neither does init(): a null there was silently skipped, which is
exactly the failure this change exists to make loud.

tests/popupElementIds.test.js is the general half and needs no browser,
so jest picks it up and it runs in make check: every literal id reached
through $(), document.getElementById(), showError()/hideError() and
showView() must exist in src/popup/index.html, no id in index.html may
be defined twice, and the scan asserts it found the code and the markup
so it cannot pass by covering nothing. Only literal arguments are
resolvable statically; $(containerId) and a lookup naming the wrong
existing element are the browser suites' job, and README says so.

Demonstrated against four deliberate breaks. A typo'd id in settings.js
reddens both halves, the e2e run reporting "pageerror: Cannot set
properties of null (setting 'checked')" against its first test. A
handler bound to the wrong but existing element passes the static guard
and reddens only the new functional case. A typo in a view no browser
suite opens reddens only the static guard. Deleting either persisted
value assignment in settings.js — the theme one in init(), the network
one in show() — reddens the selector round trip and nothing else, each
one on its own.
2026-08-17 06:58:41 +00:00
3 changed files with 3 additions and 102 deletions

13
TODO.md
View File

@@ -74,19 +74,6 @@ undefined identifiers, which is how
the deletion of both persisted-value assignments in `settings.js` (only the
selector round-trip case red)
([#229](https://git.eeqj.de/sneak/AutistMask/issues/229)).
- 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

View File

@@ -112,7 +112,7 @@ function show(walletIdx, addrIdx) {
async function reveal() {
const password = $("export-privkey-password").value;
if (!password) {
fail("Please enter your password.");
fail("Password is required.");
return;
}
if (walletIndex === null) {

View File

@@ -1,4 +1,4 @@
// One wording for one condition (issues #172 and #265).
// 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
@@ -21,15 +21,6 @@
// 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");
@@ -37,7 +28,6 @@ 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;
@@ -46,7 +36,6 @@ const CANONICAL_EMPTY = "Please enter your password.";
const SUPERSEDED = [
"Wrong password.",
"That password is not correct. Please try again.",
"Password is required.",
];
function jsFilesUnder(dir) {
@@ -160,71 +149,6 @@ 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
@@ -263,9 +187,8 @@ describe("password failure messages", () => {
});
});
test("the canonical messages are full sentences", () => {
test("the canonical message is a full sentence", () => {
expect(CANONICAL).toMatch(/^[A-Z][^]*\.$/);
expect(CANONICAL_EMPTY).toMatch(/^[A-Z][^]*\.$/);
});
// Exact equality, per call site: a message that is merely different
@@ -280,15 +203,6 @@ 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) => {