fix: keep the flash line to its one line at any message length #420

Merged
clawbot merged 1 commits from issue-252-flash-one-line into next 2026-10-04 10:22:52 +02:00
13 changed files with 262 additions and 81 deletions
Showing only changes of commit d47ad7ab43 - Show all commits
+12
View File
@@ -45,6 +45,18 @@ but the review is broader than any of them.
# Completed Steps
- 2026-10-04: The flash line keeps to the one line it reserves at any message
length ([#252](https://git.eeqj.de/sneak/AutistMask/issues/252)). A message
that wrapped pushed the whole screen below it down. `#flash-msg` no longer
wraps: text too long for the line is cut with an ellipsis, and `showFlash()`
puts the whole message in the line's title. Every message is also reworded to
at most 50 characters so none is cut; none carries a wallet name or text from
a server, and the add-token screens flash a fixed line for any error other
than a contract that is not a token. A new test in `tests/e2e/run.js` puts a
message several lines long on the line and fails if the line or the screen
below it moves. The two approval-screen error boxes are left to
[#297](https://git.eeqj.de/sneak/AutistMask/issues/297).
- 2026-10-04: A method the wallet does not implement is refused with EIP-1193
code `4200` ([#279](https://git.eeqj.de/sneak/AutistMask/issues/279)). The
background's `Unsupported method: <method>` error carried no code, so a site
+3 -7
View File
@@ -19,13 +19,9 @@
// that the user did not type — the same silent substitution the visible
// rejection message exists to end.
// Must render on ONE line of #flash-msg, whose reserved height
// (min-h-[1.25rem]) is exactly one line at text-xs. A string long enough to
// wrap to two lines pushes the settings view down, which the No Layout Shift
// policy forbids. Do not lengthen this without re-running the layout test in
// tests/e2e/run.js, which measures the flash line and goes red on a shift.
const DUST_THRESHOLD_MESSAGE =
"Please enter a whole number of gwei, zero or greater.";
// Must render on ONE line of #flash-msg; see showFlash() in
// src/popup/views/helpers.js for how long that is.
const DUST_THRESHOLD_MESSAGE = "Enter a whole number of gwei, zero or greater.";
// Returns the threshold in gwei, or null if the input is not one.
function parseDustThresholdGwei(raw) {
+1 -1
View File
@@ -33,7 +33,7 @@
<!-- ============ FLASH MESSAGE AREA ============ -->
<div
id="flash-msg"
class="text-xs text-muted min-h-[1.25rem] mb-1"
class="text-xs text-muted min-h-[1.25rem] mb-1 truncate"
></div>
<!-- ============ WELCOME / FIRST USE ============ -->
+10 -5
View File
@@ -28,9 +28,7 @@ function init(ctx) {
$("btn-add-token-confirm").addEventListener("click", async () => {
const contractAddr = $("add-token-address").value.trim();
if (!contractAddr || !contractAddr.startsWith("0x")) {
showFlash(
"Please enter a valid contract address starting with 0x.",
);
showFlash("Enter a valid contract address starting with 0x.");
return;
}
const already = state.trackedTokens.find(
@@ -71,8 +69,15 @@ function init(ctx) {
require("./addressDetail").show();
} catch (e) {
const detail = e.shortMessage || e.message || String(e);
log.errorf("Token lookup failed for", contractAddr, detail);
showFlash(detail);
log.errorf("Adding token failed for", contractAddr, detail);
// lookupTokenInfo() rejects a contract with a one-line message
// starting "Not a valid ERC-20 token". Any other error, such as a
// failed save, can be far longer, so it is only logged.
showFlash(
detail.startsWith("Not a valid ERC-20 token")
? detail
: "Could not add the token.",
);
infoEl.textContent = "";
infoEl.style.visibility = "hidden";
}
+10 -23
View File
@@ -142,15 +142,13 @@ function validatePassword() {
async function importMnemonic(ctx) {
const mnemonic = $("wallet-mnemonic").value.trim();
if (!mnemonic) {
showFlash("Enter a recovery phrase or press the die to generate one.");
showFlash("Enter a recovery phrase, or press the die.");
return;
}
const words = mnemonic.split(/\s+/);
if (words.length !== 12 && words.length !== 24) {
showFlash(
"Recovery phrase must be 12 or 24 words. You entered " +
words.length +
".",
"Recovery phrase must be 12 or 24 words, not " + words.length + ".",
);
return;
}
@@ -163,14 +161,12 @@ async function importMnemonic(ctx) {
const { xpub, firstAddress } = hdWalletFromMnemonic(mnemonic);
const xpubDup = findWalletByXpub(xpub);
if (xpubDup) {
showFlash(
"This recovery phrase is already added (" + xpubDup.name + ").",
);
showFlash("This recovery phrase is already added.");
return;
}
const addrDup = findWalletByAddress(firstAddress);
if (addrDup) {
showFlash("Address already exists in wallet (" + addrDup.name + ").");
showFlash("Address already exists in a wallet.");
return;
}
const encrypted = await encryptWithPassword(mnemonic, pw);
@@ -229,9 +225,7 @@ async function importPrivateKey(ctx) {
if (!pw) return;
const duplicate = findWalletByAddress(addr);
if (duplicate) {
showFlash(
"This address already exists in wallet (" + duplicate.name + ").",
);
showFlash("This address already exists in a wallet.");
return;
}
const encrypted = await encryptWithPassword(key, pw);
@@ -258,36 +252,29 @@ async function importXprvKey(ctx) {
return;
}
if (!isValidXprv(xprv)) {
showFlash(
"That extended private key is not valid. Please check it and try again.",
);
showFlash("That extended private key is not valid.");
return;
}
if (!isMasterExtendedKey(xprv)) {
showFlash(
"That is an account-level or child key, which cannot be imported. " +
"Please paste the master extended private key for the wallet.",
);
showFlash("Please paste the master key, not a child key.");
return;
}
let result;
try {
result = hdWalletFromXprv(xprv);
} catch {
showFlash(
"That extended private key is not valid. Please check it and try again.",
);
showFlash("That extended private key is not valid.");
return;
}
const { xpub, firstAddress } = result;
const xpubDup = findWalletByXpub(xpub);
if (xpubDup) {
showFlash("This key is already added (" + xpubDup.name + ").");
showFlash("This key is already added.");
return;
}
const addrDup = findWalletByAddress(firstAddress);
if (addrDup) {
showFlash("Address already exists in wallet (" + addrDup.name + ").");
showFlash("Address already exists in a wallet.");
return;
}
const pw = validatePassword();
+8 -4
View File
@@ -223,15 +223,19 @@ function clearFlash() {
flashTimer = null;
}
$("flash-msg").textContent = "";
$("flash-msg").title = "";
}
// The flash line reserves exactly one line, and a message that wrapped would
// push the screen below it down (README, No Layout Shift). So #flash-msg never
// wraps: text too long for the line is cut with an ellipsis, and the whole
// message is also put in the line's title. Write messages to fit, at most 50
// characters, so none is cut.
function showFlash(msg, duration = 2000) {
clearFlash();
$("flash-msg").textContent = msg;
flashTimer = setTimeout(() => {
$("flash-msg").textContent = "";
flashTimer = null;
}, duration);
$("flash-msg").title = msg;
flashTimer = setTimeout(clearFlash, duration);
}
// A stored token balance as a number, or null when there is no number in it.
+3 -3
View File
@@ -63,13 +63,13 @@ function validateToAddress(value) {
if (checksummed !== v) {
return {
valid: false,
error: "Address checksum is invalid. Please double-check the address.",
error: "Address checksum is invalid. Check the address.",
};
}
} catch {
return {
valid: false,
error: "Address checksum is invalid. Please double-check the address.",
error: "Address checksum is invalid. Check the address.",
};
}
}
@@ -211,7 +211,7 @@ function init(_ctx) {
const provider = getProvider(state.rpcUrl, state.networkId);
const resolved = await provider.resolveName(to);
if (!resolved) {
showFlash("Could not resolve " + to);
showFlash("That ENS name has no address.");
return;
}
resolvedTo = resolved;
+2 -8
View File
@@ -264,18 +264,12 @@ function init(ctx) {
const json = await resp.json();
if (json.error) {
log.errorf("RPC validation error:", json.error);
showFlash("Endpoint returned error: " + json.error.message);
showFlash("Endpoint returned an error.");
return;
}
const net = currentNetwork();
if (json.result !== net.chainId) {
showFlash(
"Wrong network (expected " +
net.name +
", got chain " +
json.result +
").",
);
showFlash("Wrong network: expected " + net.name + ".");
return;
}
} catch (e) {
+10 -5
View File
@@ -115,9 +115,7 @@ function init(_ctx) {
$("btn-settings-addtoken-manual").addEventListener("click", async () => {
const addr = $("settings-addtoken-address").value.trim();
if (!addr || !addr.startsWith("0x")) {
showFlash(
"Please enter a valid contract address starting with 0x.",
);
showFlash("Enter a valid contract address starting with 0x.");
return;
}
if (isTracked(addr)) {
@@ -155,8 +153,15 @@ function init(_ctx) {
ctx.doRefreshAndRender();
} catch (e) {
const detail = e.shortMessage || e.message || String(e);
log.errorf("Token lookup failed for", addr, detail);
showFlash(detail);
log.errorf("Adding token failed for", addr, detail);
// lookupTokenInfo() rejects a contract with a one-line message
// starting "Not a valid ERC-20 token". Any other error, such as a
// failed save, can be far longer, so it is only logged.
showFlash(
detail.startsWith("Not a valid ERC-20 token")
? detail
: "Could not add the token.",
);
infoEl.textContent = "";
infoEl.style.visibility = "hidden";
}
+4 -6
View File
@@ -41,12 +41,10 @@ const DEFECTS = {
"changed or removed, and this wallet stays until you delete " +
"it yourself.",
],
// One sentence for the places that have room for one: the flash on a
// blocked Send, the inline error on the approval screens.
shortMessage:
"This wallet cannot sign, because it was imported from an " +
"extended private key that is not a master key. The wallet list " +
"explains what happened.",
// One line, for the flash on a blocked Send and the inline error on
// the approval screens. It must fit on the flash line; see showFlash()
// in src/popup/views/helpers.js.
shortMessage: "This wallet cannot sign. See the wallet list.",
},
};
+7 -6
View File
@@ -99,12 +99,13 @@ describe("the flash line the message is shown in", () => {
// length, including one that wrapped to two lines and pushed the
// settings view down 12px.
//
// The assertion that actually measures — empty line vs. the message,
// real Chromium, documented 360x600 popup — is
// "a rejected dust threshold shifts no layout (#233)" in
// tests/e2e/run.js, run by make test-e2e. It is not in make check
// because REPO_POLICIES.md caps make test at 20 seconds and a browser
// suite does not fit; run it before changing the wording.
// The line cuts a message too long for it with an ellipsis (see
// showFlash() in src/popup/views/helpers.js). The assertions that
// measure that, in a real browser at the documented 360x600 popup, are
// "a rejected dust threshold shifts no layout (#233)" and "an over-long
// flash message keeps to one line (#252)" in tests/e2e/run.js, run by
// make test-e2e. They are not in make check because REPO_POLICIES.md
// caps make test at 20 seconds and a browser suite does not fit.
test("reserves its height in the markup", () => {
const flashLine = POPUP_HTML.match(
/<div\s+id="flash-msg"\s+class="([^"]*)"/,
+62 -13
View File
@@ -1320,17 +1320,13 @@ async function waitForFilledFlashLine(page) {
}
// README, No Layout Shift: the rejection message goes into #flash-msg,
// whose min-h-[1.25rem] reserves exactly ONE line at text-xs. Reserving
// the space is not enough on its own — a message too long for one line
// wraps and pushes everything below it down anyway, which is what the
// first version of this change shipped: 75 characters, 32px, the settings
// view and the threshold field 12px lower than with an empty line.
//
// So this measures rather than inspects markup. It is the only assertion
// in the repo that can see the wording grow: the unit suite runs on the
// node environment with no layout engine, where every height is zero (see
// the note in tests/dustThreshold.test.js). Lengthen
// DUST_THRESHOLD_MESSAGE past one line and this test goes red.
// whose min-h-[1.25rem] reserves exactly ONE line at text-xs, and which
// cuts a message too long for that line with an ellipsis rather than wrap
// it. This shows the real message and measures that nothing moves; the
// test after it does the same with a message several lines long. Both
// measure rather than inspect markup: the unit suite runs on the node
// environment with no layout engine, where every height is zero (see the
// note in tests/dustThreshold.test.js).
test("a rejected dust threshold shifts no layout (#233)", async (env) => {
const page = await openPopup(env.ctx, env.popupUrl);
try {
@@ -1377,11 +1373,11 @@ test("a rejected dust threshold shifts no layout (#233)", async (env) => {
);
assert(
after.flashHeight === before.flashHeight,
"the message does not fit the reserved line: " +
"the message does not keep to the reserved line: " +
before.flashHeight +
"px empty vs " +
after.flashHeight +
"px with the message. Shorten DUST_THRESHOLD_MESSAGE",
"px with the message",
);
assert(
after.settingsTop === before.settingsTop,
@@ -1400,6 +1396,59 @@ test("a rejected dust threshold shifts no layout (#233)", async (env) => {
}
});
// ------------------------------------------------ the flash line (#252)
// #flash-msg never wraps: a message too long for its one line is cut with an
// ellipsis (see showFlash() in src/popup/views/helpers.js). This puts a
// message several lines long into it and measures that the line and the
// screen below it stay where they were.
test("an over-long flash message keeps to one line (#252)", async (env) => {
const page = await openPopup(env.ctx, env.popupUrl);
try {
await page.setViewportSize(POPUP_VIEWPORT);
await openSettings(page);
const before = await page.evaluate(measureFlashLine);
const overflows = await page.evaluate(() => {
const line = document.getElementById("flash-msg");
line.textContent =
"This message is far too long for one line. ".repeat(5);
return line.scrollWidth > line.clientWidth;
});
const after = await page.evaluate(measureFlashLine);
assert(
after.flashHeight === before.flashHeight,
"the flash line is " +
before.flashHeight +
"px before and " +
after.flashHeight +
"px with an over-long message, so it wraps",
);
assert(
after.settingsTop === before.settingsTop,
"the settings view moved " +
(after.settingsTop - before.settingsTop) +
"px when the message appeared",
);
assert(
after.fieldTop === before.fieldTop,
"the dust threshold field moved " +
(after.fieldTop - before.fieldTop) +
"px when the message appeared",
);
// Checked last: a line that wraps does not run past its right edge,
// so this only shows the message really was cut once nothing moved.
assert(
overflows,
"the message fits on the line, so it proves nothing: " +
JSON.stringify(after.text),
);
} finally {
await page.close();
}
});
// --------------------------------------------- confirmation screen (#238)
//
// The screen that decides what gets signed. The arithmetic underneath it
+130
View File
@@ -0,0 +1,130 @@
// The flash line (#252). #flash-msg reserves one line and cuts a message too
// long for it with an ellipsis; that is measured in a real browser by
// tests/e2e/run.js. Here: showFlash() keeps the whole message readable in the
// line's title, and the two add-token screens flash a fixed line, not the text
// of whatever error adding the token threw.
const ADDRESS = "0x1111111111111111111111111111111111111111";
let elements;
function fakeElement() {
return {
value: "",
textContent: "",
title: "",
style: {},
listeners: {},
addEventListener(event, handler) {
this.listeners[event] = handler;
},
};
}
// Stands in for document.getElementById(): one fake element per id.
function element(id) {
return (elements[id] ||= fakeElement());
}
beforeEach(() => {
jest.resetModules();
elements = {};
globalThis.document = { getElementById: element };
// state.js reads chrome.storage.local at load.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
});
afterEach(() => {
jest.dontMock("../src/popup/views/helpers");
jest.dontMock("../src/shared/state");
jest.dontMock("../src/shared/balances");
jest.restoreAllMocks();
jest.useRealTimers();
delete globalThis.document;
delete globalThis.chrome;
});
test("showFlash() puts the whole message in the title, and clears both", () => {
jest.useFakeTimers();
const { showFlash } = require("../src/popup/views/helpers");
showFlash("Saved.");
expect(element("flash-msg").textContent).toBe("Saved.");
expect(element("flash-msg").title).toBe("Saved.");
jest.advanceTimersByTime(2000);
expect(element("flash-msg").textContent).toBe("");
expect(element("flash-msg").title).toBe("");
});
describe.each([
["addToken", "add-token-address", "btn-add-token-confirm"],
[
"settingsAddToken",
"settings-addtoken-address",
"btn-settings-addtoken-manual",
],
])("adding a token on %s", (view, field, button) => {
let flashes;
let errors;
// Clicks the screen's add button with lookupTokenInfo() and saveState()
// replaced by the given functions.
async function add(lookupTokenInfo, saveState) {
flashes = [];
errors = jest.spyOn(console, "error").mockImplementation(() => {});
jest.spyOn(console, "log").mockImplementation(() => {});
jest.doMock("../src/shared/balances", () => ({ lookupTokenInfo }));
jest.doMock("../src/shared/state", () => ({
state: { trackedTokens: [] },
saveState,
}));
jest.doMock("../src/popup/views/helpers", () => ({
$: element,
showView: () => {},
showFlash: (msg) => flashes.push(msg),
escapeHtml: (s) => s,
goBack: () => {},
}));
require("../src/popup/views/" + view).init({
doRefreshAndRender: () => {},
});
element(field).value = ADDRESS;
await element(button).listeners.click();
}
test("a failed save flashes a fixed line and logs the error", async () => {
const detail = "A sentence about the stored record. ".repeat(4);
await add(
async () => ({ symbol: "TKN", decimals: 18, name: "Token" }),
async () => {
throw new Error(detail);
},
);
expect(flashes).toEqual(["Could not add the token."]);
expect(errors).toHaveBeenCalledWith(
"[AutistMask]",
"Adding token failed for",
ADDRESS,
detail,
);
});
test("a contract that is not a token flashes the lookup message", async () => {
const detail = "Not a valid ERC-20 token (symbol() failed).";
await add(
async () => {
throw new Error(detail);
},
async () => {},
);
expect(flashes).toEqual([detail]);
});
});