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
+12
View File
@@ -45,6 +45,18 @@ but the review is broader than any of them.
# Completed Steps # 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 - 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 code `4200` ([#279](https://git.eeqj.de/sneak/AutistMask/issues/279)). The
background's `Unsupported method: <method>` error carried no code, so a site 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 // that the user did not type — the same silent substitution the visible
// rejection message exists to end. // rejection message exists to end.
// Must render on ONE line of #flash-msg, whose reserved height // Must render on ONE line of #flash-msg; see showFlash() in
// (min-h-[1.25rem]) is exactly one line at text-xs. A string long enough to // src/popup/views/helpers.js for how long that is.
// wrap to two lines pushes the settings view down, which the No Layout Shift const DUST_THRESHOLD_MESSAGE = "Enter a whole number of gwei, zero or greater.";
// 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.";
// Returns the threshold in gwei, or null if the input is not one. // Returns the threshold in gwei, or null if the input is not one.
function parseDustThresholdGwei(raw) { function parseDustThresholdGwei(raw) {
+1 -1
View File
@@ -33,7 +33,7 @@
<!-- ============ FLASH MESSAGE AREA ============ --> <!-- ============ FLASH MESSAGE AREA ============ -->
<div <div
id="flash-msg" 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> ></div>
<!-- ============ WELCOME / FIRST USE ============ --> <!-- ============ WELCOME / FIRST USE ============ -->
+10 -5
View File
@@ -28,9 +28,7 @@ function init(ctx) {
$("btn-add-token-confirm").addEventListener("click", async () => { $("btn-add-token-confirm").addEventListener("click", async () => {
const contractAddr = $("add-token-address").value.trim(); const contractAddr = $("add-token-address").value.trim();
if (!contractAddr || !contractAddr.startsWith("0x")) { if (!contractAddr || !contractAddr.startsWith("0x")) {
showFlash( showFlash("Enter a valid contract address starting with 0x.");
"Please enter a valid contract address starting with 0x.",
);
return; return;
} }
const already = state.trackedTokens.find( const already = state.trackedTokens.find(
@@ -71,8 +69,15 @@ function init(ctx) {
require("./addressDetail").show(); require("./addressDetail").show();
} catch (e) { } catch (e) {
const detail = e.shortMessage || e.message || String(e); const detail = e.shortMessage || e.message || String(e);
log.errorf("Token lookup failed for", contractAddr, detail); log.errorf("Adding token failed for", contractAddr, detail);
showFlash(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.textContent = "";
infoEl.style.visibility = "hidden"; infoEl.style.visibility = "hidden";
} }
+10 -23
View File
@@ -142,15 +142,13 @@ function validatePassword() {
async function importMnemonic(ctx) { async function importMnemonic(ctx) {
const mnemonic = $("wallet-mnemonic").value.trim(); const mnemonic = $("wallet-mnemonic").value.trim();
if (!mnemonic) { if (!mnemonic) {
showFlash("Enter a recovery phrase or press the die to generate one."); showFlash("Enter a recovery phrase, or press the die.");
return; return;
} }
const words = mnemonic.split(/\s+/); const words = mnemonic.split(/\s+/);
if (words.length !== 12 && words.length !== 24) { if (words.length !== 12 && words.length !== 24) {
showFlash( showFlash(
"Recovery phrase must be 12 or 24 words. You entered " + "Recovery phrase must be 12 or 24 words, not " + words.length + ".",
words.length +
".",
); );
return; return;
} }
@@ -163,14 +161,12 @@ async function importMnemonic(ctx) {
const { xpub, firstAddress } = hdWalletFromMnemonic(mnemonic); const { xpub, firstAddress } = hdWalletFromMnemonic(mnemonic);
const xpubDup = findWalletByXpub(xpub); const xpubDup = findWalletByXpub(xpub);
if (xpubDup) { if (xpubDup) {
showFlash( showFlash("This recovery phrase is already added.");
"This recovery phrase is already added (" + xpubDup.name + ").",
);
return; return;
} }
const addrDup = findWalletByAddress(firstAddress); const addrDup = findWalletByAddress(firstAddress);
if (addrDup) { if (addrDup) {
showFlash("Address already exists in wallet (" + addrDup.name + ")."); showFlash("Address already exists in a wallet.");
return; return;
} }
const encrypted = await encryptWithPassword(mnemonic, pw); const encrypted = await encryptWithPassword(mnemonic, pw);
@@ -229,9 +225,7 @@ async function importPrivateKey(ctx) {
if (!pw) return; if (!pw) return;
const duplicate = findWalletByAddress(addr); const duplicate = findWalletByAddress(addr);
if (duplicate) { if (duplicate) {
showFlash( showFlash("This address already exists in a wallet.");
"This address already exists in wallet (" + duplicate.name + ").",
);
return; return;
} }
const encrypted = await encryptWithPassword(key, pw); const encrypted = await encryptWithPassword(key, pw);
@@ -258,36 +252,29 @@ async function importXprvKey(ctx) {
return; return;
} }
if (!isValidXprv(xprv)) { if (!isValidXprv(xprv)) {
showFlash( showFlash("That extended private key is not valid.");
"That extended private key is not valid. Please check it and try again.",
);
return; return;
} }
if (!isMasterExtendedKey(xprv)) { if (!isMasterExtendedKey(xprv)) {
showFlash( showFlash("Please paste the master key, not a child key.");
"That is an account-level or child key, which cannot be imported. " +
"Please paste the master extended private key for the wallet.",
);
return; return;
} }
let result; let result;
try { try {
result = hdWalletFromXprv(xprv); result = hdWalletFromXprv(xprv);
} catch { } catch {
showFlash( showFlash("That extended private key is not valid.");
"That extended private key is not valid. Please check it and try again.",
);
return; return;
} }
const { xpub, firstAddress } = result; const { xpub, firstAddress } = result;
const xpubDup = findWalletByXpub(xpub); const xpubDup = findWalletByXpub(xpub);
if (xpubDup) { if (xpubDup) {
showFlash("This key is already added (" + xpubDup.name + ")."); showFlash("This key is already added.");
return; return;
} }
const addrDup = findWalletByAddress(firstAddress); const addrDup = findWalletByAddress(firstAddress);
if (addrDup) { if (addrDup) {
showFlash("Address already exists in wallet (" + addrDup.name + ")."); showFlash("Address already exists in a wallet.");
return; return;
} }
const pw = validatePassword(); const pw = validatePassword();
+8 -4
View File
@@ -223,15 +223,19 @@ function clearFlash() {
flashTimer = null; flashTimer = null;
} }
$("flash-msg").textContent = ""; $("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) { function showFlash(msg, duration = 2000) {
clearFlash(); clearFlash();
$("flash-msg").textContent = msg; $("flash-msg").textContent = msg;
flashTimer = setTimeout(() => { $("flash-msg").title = msg;
$("flash-msg").textContent = ""; flashTimer = setTimeout(clearFlash, duration);
flashTimer = null;
}, duration);
} }
// A stored token balance as a number, or null when there is no number in it. // 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) { if (checksummed !== v) {
return { return {
valid: false, valid: false,
error: "Address checksum is invalid. Please double-check the address.", error: "Address checksum is invalid. Check the address.",
}; };
} }
} catch { } catch {
return { return {
valid: false, 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 provider = getProvider(state.rpcUrl, state.networkId);
const resolved = await provider.resolveName(to); const resolved = await provider.resolveName(to);
if (!resolved) { if (!resolved) {
showFlash("Could not resolve " + to); showFlash("That ENS name has no address.");
return; return;
} }
resolvedTo = resolved; resolvedTo = resolved;
+2 -8
View File
@@ -264,18 +264,12 @@ function init(ctx) {
const json = await resp.json(); const json = await resp.json();
if (json.error) { if (json.error) {
log.errorf("RPC validation error:", json.error); log.errorf("RPC validation error:", json.error);
showFlash("Endpoint returned error: " + json.error.message); showFlash("Endpoint returned an error.");
return; return;
} }
const net = currentNetwork(); const net = currentNetwork();
if (json.result !== net.chainId) { if (json.result !== net.chainId) {
showFlash( showFlash("Wrong network: expected " + net.name + ".");
"Wrong network (expected " +
net.name +
", got chain " +
json.result +
").",
);
return; return;
} }
} catch (e) { } catch (e) {
+10 -5
View File
@@ -115,9 +115,7 @@ function init(_ctx) {
$("btn-settings-addtoken-manual").addEventListener("click", async () => { $("btn-settings-addtoken-manual").addEventListener("click", async () => {
const addr = $("settings-addtoken-address").value.trim(); const addr = $("settings-addtoken-address").value.trim();
if (!addr || !addr.startsWith("0x")) { if (!addr || !addr.startsWith("0x")) {
showFlash( showFlash("Enter a valid contract address starting with 0x.");
"Please enter a valid contract address starting with 0x.",
);
return; return;
} }
if (isTracked(addr)) { if (isTracked(addr)) {
@@ -155,8 +153,15 @@ function init(_ctx) {
ctx.doRefreshAndRender(); ctx.doRefreshAndRender();
} catch (e) { } catch (e) {
const detail = e.shortMessage || e.message || String(e); const detail = e.shortMessage || e.message || String(e);
log.errorf("Token lookup failed for", addr, detail); log.errorf("Adding token failed for", addr, detail);
showFlash(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.textContent = "";
infoEl.style.visibility = "hidden"; infoEl.style.visibility = "hidden";
} }
+4 -6
View File
@@ -41,12 +41,10 @@ const DEFECTS = {
"changed or removed, and this wallet stays until you delete " + "changed or removed, and this wallet stays until you delete " +
"it yourself.", "it yourself.",
], ],
// One sentence for the places that have room for one: the flash on a // One line, for the flash on a blocked Send and the inline error on
// blocked Send, the inline error on the approval screens. // the approval screens. It must fit on the flash line; see showFlash()
shortMessage: // in src/popup/views/helpers.js.
"This wallet cannot sign, because it was imported from an " + shortMessage: "This wallet cannot sign. See the wallet list.",
"extended private key that is not a master key. The wallet list " +
"explains what happened.",
}, },
}; };
+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 // length, including one that wrapped to two lines and pushed the
// settings view down 12px. // settings view down 12px.
// //
// The assertion that actually measures — empty line vs. the message, // The line cuts a message too long for it with an ellipsis (see
// real Chromium, documented 360x600 popup — is // showFlash() in src/popup/views/helpers.js). The assertions that
// "a rejected dust threshold shifts no layout (#233)" in // measure that, in a real browser at the documented 360x600 popup, are
// tests/e2e/run.js, run by make test-e2e. It is not in make check // "a rejected dust threshold shifts no layout (#233)" and "an over-long
// because REPO_POLICIES.md caps make test at 20 seconds and a browser // flash message keeps to one line (#252)" in tests/e2e/run.js, run by
// suite does not fit; run it before changing the wording. // 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", () => { test("reserves its height in the markup", () => {
const flashLine = POPUP_HTML.match( const flashLine = POPUP_HTML.match(
/<div\s+id="flash-msg"\s+class="([^"]*)"/, /<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, // README, No Layout Shift: the rejection message goes into #flash-msg,
// whose min-h-[1.25rem] reserves exactly ONE line at text-xs. Reserving // whose min-h-[1.25rem] reserves exactly ONE line at text-xs, and which
// the space is not enough on its own — a message too long for one line // cuts a message too long for that line with an ellipsis rather than wrap
// wraps and pushes everything below it down anyway, which is what the // it. This shows the real message and measures that nothing moves; the
// first version of this change shipped: 75 characters, 32px, the settings // test after it does the same with a message several lines long. Both
// view and the threshold field 12px lower than with an empty line. // measure rather than inspect markup: the unit suite runs on the node
// // environment with no layout engine, where every height is zero (see the
// So this measures rather than inspects markup. It is the only assertion // note in tests/dustThreshold.test.js).
// 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.
test("a rejected dust threshold shifts no layout (#233)", async (env) => { test("a rejected dust threshold shifts no layout (#233)", async (env) => {
const page = await openPopup(env.ctx, env.popupUrl); const page = await openPopup(env.ctx, env.popupUrl);
try { try {
@@ -1377,11 +1373,11 @@ test("a rejected dust threshold shifts no layout (#233)", async (env) => {
); );
assert( assert(
after.flashHeight === before.flashHeight, after.flashHeight === before.flashHeight,
"the message does not fit the reserved line: " + "the message does not keep to the reserved line: " +
before.flashHeight + before.flashHeight +
"px empty vs " + "px empty vs " +
after.flashHeight + after.flashHeight +
"px with the message. Shorten DUST_THRESHOLD_MESSAGE", "px with the message",
); );
assert( assert(
after.settingsTop === before.settingsTop, 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) // --------------------------------------------- confirmation screen (#238)
// //
// The screen that decides what gets signed. The arithmetic underneath it // 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]);
});
});