From 64ec571a179d02c82407144a6543f6fd49adde0f Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 3 Nov 2026 12:27:20 +0100 Subject: [PATCH] fix: long symbols wrap on TxApproval and TransactionDetail, and SuccessTx keeps its amount and recipient (closes #743) TxApproval and TransactionDetail cut off a long symbol, amount or description at the edge of the screen. These lines now wrap. SuccessTx hid its Amount and To lines whenever it had a decoded action, even one that showed neither. Each line is now hidden only when a readable decoded line shows the same thing. A swap with no Amount line now shows its amount on SuccessTx, as WaitTx and ErrorTx do. Model: opus-5-5 --- README.md | 4 +- src/popup/index.html | 9 ++- src/popup/views/approval.js | 8 +-- src/popup/views/transactionDetail.js | 13 ++-- src/popup/views/txStatus.js | 38 +++++++----- tests/approvalBundledListSepolia.test.js | 10 ++-- tests/approvalTokenNetwork.test.js | 50 ++++++++++++++-- tests/persistedFieldContract.test.js | 8 ++- tests/swapStatusAmount.test.js | 46 +++++++++++---- tests/tokenTransferDetail.test.js | 63 ++++++++++++++++++-- tests/txStatus.test.js | 75 ++++++++++++++++++++++++ 11 files changed, 269 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 504bd81..3e0d78a 100644 --- a/README.md +++ b/README.md @@ -812,7 +812,9 @@ For a swap whose amount could not be decoded, the amount reads Shown when the receipt arrives. The screen holds: - the heading "Transaction Confirmed"; -- the amount and the recipient, or the decoded action when there is one; +- the decoded action, when there is one; +- the amount, unless a line of the decoded action shows it; +- the recipient, unless a line of the decoded action shows it; - the block number, with an explorer link; - the transaction hash, with an explorer link; - a "Done" button. diff --git a/src/popup/index.html b/src/popup/index.html index d50f879..1e3e773 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -1646,7 +1646,7 @@
Action
Action
@@ -1736,7 +1736,10 @@
Value
-
+
Network fee (max)
diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 4d001e0..7423b82 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -361,7 +361,7 @@ function showTxApproval(details) { if (toAddr) { let toHtml = ""; if (symbol) { - toHtml += `
${escapeHtml(symbol)}
`; + toHtml += `
${escapeHtml(symbol)}
`; } toHtml += approvalAddressHtml(toAddr, approvedTx.chainId); $("approve-tx-to").innerHTML = toHtml; @@ -394,14 +394,14 @@ function showTxApproval(details) { $("approve-tx-action").textContent = decoded.name; let detailsHtml = ""; if (decoded.description) { - detailsHtml += `
${escapeHtml(decoded.description)}
`; + detailsHtml += `
${escapeHtml(decoded.description)}
`; } for (const d of decoded.details) { detailsHtml += `
`; detailsHtml += `
${escapeHtml(d.label)}
`; if (d.address) { if (d.isToken) { - detailsHtml += `
${escapeHtml(tokenLabel(d.address, approvedTx.chainId) || "Unknown token")}
`; + detailsHtml += `
${escapeHtml(tokenLabel(d.address, approvedTx.chainId) || "Unknown token")}
`; detailsHtml += blockscoutOffTokenNoteHtml( d.address, approvedTx.chainId, @@ -412,7 +412,7 @@ function showTxApproval(details) { approvedTx.chainId, ); } else { - detailsHtml += `
${escapeHtml(d.value)}
`; + detailsHtml += `
${escapeHtml(d.value)}
`; } detailsHtml += `
`; } diff --git a/src/popup/views/transactionDetail.js b/src/popup/views/transactionDetail.js index 0d40756..0d2d1c1 100644 --- a/src/popup/views/transactionDetail.js +++ b/src/popup/views/transactionDetail.js @@ -126,13 +126,16 @@ function render() { const exactStr = tx.exactValue ? tx.exactValue + " " + detailSym : tx.directionLabel + " " + detailSym; - $("tx-detail-value").innerHTML = copyableHtml(exactStr, "font-bold"); + $("tx-detail-value").innerHTML = copyableHtml( + exactStr, + "font-bold break-words", + ); // Native quantity (raw integer, copyable) const nativeEl = $("tx-detail-native"); if (tx.rawAmount && tx.rawUnit) { const nativeStr = tx.rawAmount + " " + tx.rawUnit; - nativeEl.innerHTML = copyableHtml(nativeStr, ""); + nativeEl.innerHTML = copyableHtml(nativeStr, "break-words"); nativeEl.parentElement.classList.remove("hidden"); } else { nativeEl.innerHTML = ""; @@ -368,7 +371,7 @@ async function loadFullTxDetails(txHash, chainId) { actionEl.textContent = decoded.name; let detailsHtml = ""; if (decoded.description) { - detailsHtml += `
${escapeHtml(decoded.description)}
`; + detailsHtml += `
${escapeHtml(decoded.description)}
`; } for (const d of decoded.details || []) { detailsHtml += `
`; @@ -377,13 +380,13 @@ async function loadFullTxDetails(txHash, chainId) { // Token entry: show symbol on its own line, then address via shared renderer const tokenSymbol = d.value.match(/^(\S+)\s*\(/)?.[1]; if (tokenSymbol) { - detailsHtml += `
${escapeHtml(displaySymbol(tokenSymbol))}
`; + detailsHtml += `
${escapeHtml(displaySymbol(tokenSymbol))}
`; } detailsHtml += renderAddressHtml(d.address, { chainId }); } else if (d.address) { detailsHtml += renderAddressHtml(d.address, { chainId }); } else { - detailsHtml += `
${escapeHtml(d.value)}
`; + detailsHtml += `
${escapeHtml(d.value)}
`; } detailsHtml += `
`; } diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index 22319d0..1eac740 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -321,10 +321,14 @@ function showSuccess(txInfo, txHash, blockNumber) { ctx.doRefreshAndRender(); } +function readableLines(decoded) { + if (!decoded || !Array.isArray(decoded.details)) return []; + return decoded.details.filter(isReadableLine); +} + // The decoded action, or "" when no line of it can be read. function decodedDetailsHtml(decoded, chainId) { - if (!decoded || !Array.isArray(decoded.details)) return ""; - const lines = decoded.details.filter(isReadableLine); + const lines = readableLines(decoded); if (lines.length === 0) return ""; let html = `
`; if (decoded.name && typeof decoded.name === "string") { @@ -355,20 +359,22 @@ function renderSuccess() { const decodedHtml = decodedDetailsHtml(d.decoded, d.chainId); - // When decoded details are present, the Amount and To are already - // shown inside the decoded well — hide the top-level duplicates. - const summarySection = $("success-tx-summary").parentElement; - const toSection = $("success-tx-to").parentElement; - if (decodedHtml) { - summarySection.classList.add("hidden"); - toSection.classList.add("hidden"); - } else { - summarySection.classList.remove("hidden"); - toSection.classList.remove("hidden"); - $("success-tx-summary").textContent = - d.amount + (d.symbol ? " " + d.symbol : ""); - $("success-tx-to").innerHTML = toAddressHtml(d.to, d.chainId); - } + // Amount and To are hidden only when a readable decoded line shows the same + // thing: an `Amount` line, or a line with the address in `to`. Some swaps + // have no `Amount` line, and a saved record can have unreadable lines. An + // empty `to` is a contract creation, which no decoded line shows. + const lines = readableLines(d.decoded); + $("success-tx-summary").parentElement.classList.toggle( + "hidden", + lines.some((line) => line.label === "Amount"), + ); + $("success-tx-to").parentElement.classList.toggle( + "hidden", + lines.some((line) => line.address && line.address === d.to), + ); + $("success-tx-summary").textContent = + d.amount + (d.symbol ? " " + d.symbol : ""); + $("success-tx-to").innerHTML = toAddressHtml(d.to, d.chainId); $("success-tx-block").innerHTML = blockNumberHtml(d.blockNumber, d.chainId); $("success-tx-hash").innerHTML = txHashHtml(d.hash, d.chainId); diff --git a/tests/approvalBundledListSepolia.test.js b/tests/approvalBundledListSepolia.test.js index 5f332c4..8fc7d90 100644 --- a/tests/approvalBundledListSepolia.test.js +++ b/tests/approvalBundledListSepolia.test.js @@ -151,10 +151,10 @@ test("names no token, and states the amount in base units", async () => { expect(state.currentView).toBe("approve-tx"); expect(node("approve-tx-to").innerHTML).not.toContain("USDC"); expect(decodedLine("Token")).toContain( - '
Unknown token
', + '
Unknown token
', ); expect(decodedLine("Amount")).toContain( - '
1234500000 base units (decimals unknown)
', + '
1234500000 base units (decimals unknown)
', ); }); @@ -162,8 +162,10 @@ test("names no token, and states the amount in base units", async () => { test("a mainnet transfer of USDC is still named and scaled as USDC", async () => { await showTransfer(NETWORKS.mainnet.chainId); expect(state.currentView).toBe("approve-tx"); - expect(decodedLine("Token")).toContain('
USDC
'); + expect(decodedLine("Token")).toContain( + '
USDC
', + ); expect(decodedLine("Amount")).toContain( - '
1234.5000 USDC
', + '
1234.5000 USDC
', ); }); diff --git a/tests/approvalTokenNetwork.test.js b/tests/approvalTokenNetwork.test.js index 914433f..8c34939 100644 --- a/tests/approvalTokenNetwork.test.js +++ b/tests/approvalTokenNetwork.test.js @@ -24,6 +24,8 @@ globalThis.chrome = { storage: { local: { get: async () => ({}), set: async () => {} } }, }; +const fs = require("fs"); +const path = require("path"); const { Interface } = require("ethers"); const { state } = require("../src/shared/state"); const { NETWORKS } = require("../src/shared/networks"); @@ -193,13 +195,13 @@ describe("a token tracked on Sepolia only", () => { test("is named on the Contract line and the decoded Token line, with its amount at its scale", async () => { await showTokenTransfer(); expect(node("approve-tx-to").innerHTML).toContain( - '
TDOL
', + '
TDOL
', ); expect(decodedLine("Token")).toContain( - '
TDOL
', + '
TDOL
', ); expect(decodedLine("Amount")).toContain( - '
1234.5000 TDOL
', + '
1234.5000 TDOL
', ); }); @@ -220,13 +222,51 @@ test("a token tracked and held on mainnet only is not named, and its amount is i ]); expect(node("approve-tx-to").innerHTML).not.toContain("MDOL"); expect(decodedLine("Token")).toContain( - '
Unknown token
', + '
Unknown token
', ); expect(decodedLine("Amount")).toContain( - '
1234500000 base units (decimals unknown)
', + '
1234500000 base units (decimals unknown)
', ); }); +// Nothing limits the length of a token's symbol. Without the break-words class +// a long one runs past the right edge of the screen and is cut off. +describe("a long token symbol", () => { + const LONG = "L".repeat(60); + + test("wraps on every decoded line and on the Contract line", async () => { + state.networkTrackedTokens = { + sepolia: [{ address: TOKEN, symbol: LONG, decimals: 6 }], + }; + await showTokenTransfer(); + expect(node("approve-tx-to").innerHTML).toContain( + `
${LONG}
`, + ); + expect(node("approve-tx-decoded-details").innerHTML).toContain( + `
Transfer ${LONG}
`, + ); + expect(decodedLine("Token")).toContain( + `
${LONG}
`, + ); + expect(decodedLine("Amount")).toContain( + `
1234.5000 ${LONG}
`, + ); + }); + + test("wraps on the Action line, and a long amount wraps on the Value line", () => { + const html = fs.readFileSync( + path.join(__dirname, "..", "src", "popup", "index.html"), + "utf8", + ); + expect(html).toMatch( + /id="approve-tx-action"\s+class="text-xs font-bold mb-2 break-words"/, + ); + expect(html).toMatch( + /id="approve-tx-value"\s+class="text-xs font-bold break-words"/, + ); + }); +}); + // The From and Contract lines, and the decoded Token and Recipient lines. test("every address links to the Sepolia explorer", async () => { await showTokenTransfer(); diff --git a/tests/persistedFieldContract.test.js b/tests/persistedFieldContract.test.js index a402575..45ddf12 100644 --- a/tests/persistedFieldContract.test.js +++ b/tests/persistedFieldContract.test.js @@ -1103,7 +1103,7 @@ describe("a popup reopened on a SuccessTx with a decoded record in another shape expect(page.node("success-tx-to").innerHTML).toContain(ADDRESS); }); - test("a line it can read is shown", async () => { + test("a line it can read is shown, with the amount and the recipient", async () => { const page = await bootPopup( savedSuccess({ details: [null, { label: "Steps", value: "V3_SWAP_EXACT_IN" }], @@ -1114,5 +1114,11 @@ describe("a popup reopened on a SuccessTx with a decoded record in another shape expect(page.node("success-tx-decoded").innerHTML).toContain( "V3_SWAP_EXACT_IN", ); + const summary = page.node("success-tx-summary"); + expect(summary.parentElement.classList.contains("hidden")).toBe(false); + expect(summary.textContent).toBe("1.0000 ETH"); + const to = page.node("success-tx-to"); + expect(to.parentElement.classList.contains("hidden")).toBe(false); + expect(to.innerHTML).toContain(ADDRESS); }); }); diff --git a/tests/swapStatusAmount.test.js b/tests/swapStatusAmount.test.js index 33a6b3c..13628d1 100644 --- a/tests/swapStatusAmount.test.js +++ b/tests/swapStatusAmount.test.js @@ -231,6 +231,15 @@ async function approveSwap(data, response, value = 0n) { await click("btn-approve-tx"); } +// A later popup resumes the wait, and the receipt is there. +async function receiptArrives() { + mockReceipt = { blockNumber: 1 }; + txStatus.restoreWait(); + for (let i = 0; i < 10; i++) { + await new Promise((r) => setTimeout(r, 0)); + } +} + afterEach(() => { txStatus.endWait(); mockReceipt = null; @@ -287,20 +296,27 @@ describe.each([ expect(node("error-tx-summary").textContent).toBe(expected); }); - // The success screen shows the approval screen's decoded lines in place - // of the amount, so the ETH value is not on it either. - test("the success screen has no amount line of its own", async () => { + // The success screen has an Amount line of its own only when the approval + // screen's decoded lines have none. The ETH value is on neither. + test("the success screen's amount", async () => { await approveSwap(v4SwapData(actions), { txHash: TX_HASH }); - mockReceipt = { blockNumber: 1 }; - // A later popup resumes the wait, and the receipt is there. - txStatus.restoreWait(); - for (let i = 0; i < 10; i++) { - await new Promise((r) => setTimeout(r, 0)); - } - const summary = node("success-tx-summary").parentElement; - expect(summary.classList.contains("hidden")).toBe(true); + await receiptArrives(); + const summary = node("success-tx-summary"); + expect(summary.parentElement.classList.contains("hidden")).toBe( + !expected.startsWith("Unknown"), + ); + expect(summary.textContent).toBe(expected); expect(node("success-tx-decoded").innerHTML).not.toContain("0.0000"); }); + + // The Protocol line of the decoded lines shows the router. + test("the success screen has no To line of its own", async () => { + await approveSwap(v4SwapData(actions), { txHash: TX_HASH }); + await receiptArrives(); + const to = node("success-tx-to").parentElement; + expect(to.classList.contains("hidden")).toBe(true); + expect(node("success-tx-decoded").innerHTML).toContain(ROUTER); + }); }); // A swap paid in native ETH has a Token In of "ETH (native)" and no Amount @@ -339,6 +355,14 @@ describe.each([ ); expect(node("error-tx-summary").textContent).toBe("1.0000 ETH"); }); + + test("the success screen shows the ETH the transaction sends", async () => { + await approveSwap(v4SwapData(actions), { txHash: TX_HASH }, ONE_ETH); + await receiptArrives(); + const summary = node("success-tx-summary"); + expect(summary.parentElement.classList.contains("hidden")).toBe(false); + expect(summary.textContent).toBe("1.0000 ETH"); + }); }); // https://git.eeqj.de/sneak/AutistMask/issues/722: a WRAP_ETH of 1 ETH into the diff --git a/tests/tokenTransferDetail.test.js b/tests/tokenTransferDetail.test.js index 4006155..e4331e2 100644 --- a/tests/tokenTransferDetail.test.js +++ b/tests/tokenTransferDetail.test.js @@ -21,6 +21,8 @@ // calls. Driven against a minimal DOM stub in the shape // tests/contractCreation.test.js uses. +const fs = require("fs"); +const path = require("path"); const { Interface } = require("ethers"); jest.mock("../src/shared/log", () => ({ @@ -51,6 +53,7 @@ globalThis.chrome = { const { state } = require("../src/shared/state"); const { NETWORKS } = require("../src/shared/networks"); const { fetchRecentTransactions } = require("../src/shared/transactions"); +const { displaySymbol } = require("../src/shared/symbolDisplay"); const transactionDetail = require("../src/popup/views/transactionDetail"); const EXPLORER = "https://explorer.example/api/v2"; @@ -197,15 +200,63 @@ test("a token transfer's decoded details name the token contract and the amount "Token Transfer", ); const token = decodedLine("Token"); - expect(token).toContain('
TDOL
'); + expect(token).toContain('
TDOL
'); expect(token).toContain(TOKEN); expect(token).not.toContain(RECIPIENT); expect(decodedLine("Recipient")).toContain(RECIPIENT); expect(decodedLine("Amount")).toContain( - '
1234.5000 TDOL
', + '
1234.5000 TDOL
', ); }); +// Nothing limits the length of a symbol the explorer reports. Without the +// break-words class a long one runs past the right edge of the screen and is +// cut off. +describe("a long token symbol", () => { + const LONG = "L".repeat(60); + + test("wraps on the amount, native quantity and decoded lines", async () => { + state.trackedTokens = [{ address: TOKEN, symbol: LONG, decimals: 6 }]; + const [row] = await fetchRecentTransactions( + HOLDER, + EXPLORER, + NETWORKS.mainnet.chainId, + ); + transactionDetail.show({ + ...row, + symbol: LONG, + rawUnit: LONG + " base units (10^-6)", + }); + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); + + expect(node("tx-detail-value").innerHTML).toContain( + `font-bold break-words" data-copy="1234.5 ${displaySymbol(LONG)}"`, + ); + expect(node("tx-detail-native").innerHTML).toContain( + `break-words" data-copy="1234500000 ${LONG} base units (10^-6)"`, + ); + expect(node("tx-detail-calldata-details").innerHTML).toContain( + `
Transfer ${LONG}
`, + ); + expect(decodedLine("Token")).toContain( + `
${displaySymbol(LONG)}
`, + ); + expect(decodedLine("Amount")).toContain( + `
1234.5000 ${LONG}
`, + ); + }); + + test("wraps on the Action line", () => { + const html = fs.readFileSync( + path.join(__dirname, "..", "src", "popup", "index.html"), + "utf8", + ); + expect(html).toMatch( + /id="tx-detail-calldata-action"\s+class="text-xs font-bold mb-2 break-words"/, + ); + }); +}); + // Mainnet is active, and its Blockscout knows nothing of the transaction. The // row was read from the Blockscout Settings remembers for Sepolia, and the // screen reads the transaction from there too, so every link on it is drawn. @@ -278,9 +329,11 @@ test("with mainnet active, a Sepolia token transfer's decoded details name the t }; await showSepoliaTransferWithMainnetActive(); - expect(decodedLine("Token")).toContain('
TDOL
'); + expect(decodedLine("Token")).toContain( + '
TDOL
', + ); expect(decodedLine("Amount")).toContain( - '
1234.5000 TDOL
', + '
1234.5000 TDOL
', ); }); @@ -295,6 +348,6 @@ test("with mainnet active, a Sepolia token transfer's decoded details take nothi expect(decodedLine("Token")).not.toContain("MDOL"); expect(decodedLine("Amount")).toContain( - '
1234500000 base units (decimals unknown)
', + '
1234500000 base units (decimals unknown)
', ); }); diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index 29cae35..07127b7 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -118,6 +118,7 @@ const { state } = require("../src/shared/state"); const { NETWORKS } = require("../src/shared/networks"); const { RESTORABLE_VIEWS } = require("../src/shared/restorableViews"); const { log } = require("../src/shared/log"); +const { CONTRACT_CREATION_TEXT } = require("../src/popup/views/helpers"); const TX_HASH = "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84"; @@ -1009,6 +1010,80 @@ describe("a saved wait in another shape when its receipt arrives", () => { }); }); +// The decoded action stands in for the Amount and To lines only where it shows +// the same thing. A saved record with unreadable lines can leave out either. +describe("SuccessTx's Amount and To lines beside a decoded action", () => { + const OTHER = "0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238"; + const STEPS = { label: "Steps", value: "V3_SWAP_EXACT_IN" }; + const AMOUNT = { label: "Amount", value: "0.0050 ETH" }; + const RECIPIENT_LINE = { + label: "Recipient", + value: RECIPIENT, + address: RECIPIENT, + }; + const SPENDER = { label: "Spender", value: OTHER, address: OTHER }; + + function renderSuccessWith(details, to) { + state.viewData = { + amount: "0.0050", + symbol: "ETH", + to, + hash: TX_HASH, + chainId: TX_INFO.chainId, + decoded: { details }, + }; + txStatus.renderSuccess(); + } + + function lineHidden(id) { + return getElement(id + "-parent").classList.contains("hidden"); + } + + test.each([ + ["no line for either", [STEPS], false, false], + ["a line of another address", [AMOUNT, SPENDER], true, false], + ["an Amount line only", [AMOUNT], true, false], + ["a Recipient line only", [RECIPIENT_LINE], false, true], + [ + "an unreadable Amount line", + [{ label: "Amount" }, RECIPIENT_LINE], + false, + true, + ], + [ + "an unreadable Recipient line", + [AMOUNT, { address: RECIPIENT }], + true, + false, + ], + ["a line for each", [AMOUNT, RECIPIENT_LINE], true, true], + ])("%s", (_name, details, amountHidden, toHidden) => { + renderSuccessWith(details, RECIPIENT); + expect(state.currentView).toBe("success-tx"); + expect(lineHidden("success-tx-summary")).toBe(amountHidden); + expect(lineHidden("success-tx-to")).toBe(toHidden); + }); + + test("the lines read the amount and the recipient", () => { + renderSuccessWith([STEPS], RECIPIENT); + expect(getElement("success-tx-summary").textContent).toBe("0.0050 ETH"); + expect(getElement("success-tx-to").innerHTML).toContain(RECIPIENT); + }); + + // A contract creation's data can decode as an ERC-20 call, with an empty + // token address. + test("a contract creation keeps its To line", () => { + renderSuccessWith( + [{ label: "Token", value: "", address: "", isToken: true }], + "", + ); + expect(lineHidden("success-tx-to")).toBe(false); + expect(getElement("success-tx-to").innerHTML).toBe( + CONTRACT_CREATION_TEXT, + ); + }); +}); + describe("wait-tx is a view the popup may reopen onto", () => { // The resume feature is wired through RESTORABLE_VIEWS: restoreView() // refuses any view not in the set, so dropping "wait-tx" from it kills