diff --git a/TODO.md b/TODO.md index 49525b6..f0c6731 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,17 @@ 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: Settings lists the sites connected without "Remember", and removing a site there disconnects it ([#406](https://git.eeqj.de/sneak/AutistMask/issues/406)). Such a connection 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]); + }); +});