From 5bf8b5ff1f8ede50ecb079da1ffdc64ed8888246 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 10:22:51 +0200 Subject: [PATCH] fix: keep the flash line to its one line at any message length (closes #252) The flash line reserves one line, so a message that wrapped pushed the screen below it down. #flash-msg no longer wraps: text too long for it is cut with an ellipsis, and showFlash() puts the whole message in its title. Every message is also reworded to at most 50 characters so none is cut, and the add-token screens flash a fixed line for any error other than the two lookup messages, logging the detail. A new end-to-end test writes a message several lines long into the line and fails if the line or the screen below it moves. Model: opus-5-5 --- TODO.md | 12 +++ src/popup/dustThreshold.js | 10 +-- src/popup/index.html | 2 +- src/popup/views/addToken.js | 15 ++-- src/popup/views/addWallet.js | 33 +++---- src/popup/views/helpers.js | 12 ++- src/popup/views/send.js | 6 +- src/popup/views/settings.js | 10 +-- src/popup/views/settingsAddToken.js | 15 ++-- src/shared/walletDefects.js | 10 +-- tests/dustThreshold.test.js | 13 +-- tests/e2e/run.js | 75 +++++++++++++--- tests/flashLine.test.js | 130 ++++++++++++++++++++++++++++ 13 files changed, 262 insertions(+), 81 deletions(-) create mode 100644 tests/flashLine.test.js diff --git a/TODO.md b/TODO.md index 7004d95..843c842 100644 --- a/TODO.md +++ b/TODO.md @@ -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: ` error carried no code, so a site diff --git a/src/popup/dustThreshold.js b/src/popup/dustThreshold.js index 7b64c60..e735fb8 100644 --- a/src/popup/dustThreshold.js +++ b/src/popup/dustThreshold.js @@ -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) { diff --git a/src/popup/index.html b/src/popup/index.html index e645e3f..741bc1f 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -33,7 +33,7 @@
diff --git a/src/popup/views/addToken.js b/src/popup/views/addToken.js index a408d9b..a2114bb 100644 --- a/src/popup/views/addToken.js +++ b/src/popup/views/addToken.js @@ -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"; } diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js index 1eaf55e..c9b7087 100644 --- a/src/popup/views/addWallet.js +++ b/src/popup/views/addWallet.js @@ -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(); diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index f1e5f9f..374513f 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -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. diff --git a/src/popup/views/send.js b/src/popup/views/send.js index fee2895..1600d14 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -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; diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index 4049d55..caf5800 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -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) { diff --git a/src/popup/views/settingsAddToken.js b/src/popup/views/settingsAddToken.js index 205cb38..94f479b 100644 --- a/src/popup/views/settingsAddToken.js +++ b/src/popup/views/settingsAddToken.js @@ -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"; } diff --git a/src/shared/walletDefects.js b/src/shared/walletDefects.js index afa069b..8ed89e2 100644 --- a/src/shared/walletDefects.js +++ b/src/shared/walletDefects.js @@ -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.", }, }; diff --git a/tests/dustThreshold.test.js b/tests/dustThreshold.test.js index 8a858d7..52eb48e 100644 --- a/tests/dustThreshold.test.js +++ b/tests/dustThreshold.test.js @@ -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( / { 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 diff --git a/tests/flashLine.test.js b/tests/flashLine.test.js new file mode 100644 index 0000000..8ff4a9f --- /dev/null +++ b/tests/flashLine.test.js @@ -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]); + }); +});