From 1d40e2d7d589033d43bd41e7b0b88602d2391015 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 3 Nov 2026 11:38:35 +0100 Subject: [PATCH] fix: a saved SuccessTx in another shape reopens as itself, and long amounts wrap (closes #740) The popup refused a saved SuccessTx whose decoded record was in a shape it did not expect, and showed Home. SuccessTx already shows only the lines it can read, so the popup no longer checks the record first. A long amount or token symbol was cut off at the edge of the screen. These now wrap: - the Amount figure - each decoded amount - the token symbol - the name and description of SuccessTx's decoded action WaitTx and ErrorTx share the line that shows the symbol, so they gain this too. Model: opus-5-5 --- src/popup/index.html | 5 ++- src/popup/viewRouter.js | 16 ------- src/popup/views/txStatus.js | 8 ++-- tests/persistedFieldContract.test.js | 62 +++++++++++++++++++++------- tests/swapStatusAmount.test.js | 2 +- tests/txStatus.test.js | 43 +++++++++++++++++-- 6 files changed, 97 insertions(+), 39 deletions(-) diff --git a/src/popup/index.html b/src/popup/index.html index 56cdef6..d50f879 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -833,7 +833,10 @@
Amount
-
+
To
diff --git a/src/popup/viewRouter.js b/src/popup/viewRouter.js index 31270da..dbd7406 100644 --- a/src/popup/viewRouter.js +++ b/src/popup/viewRouter.js @@ -116,21 +116,6 @@ function isAddressText(value) { return isText(value); } -// Decoded calldata, as decodedDetailsHtml() (src/popup/views/txStatus.js) -// walks it: `for (const d of decoded.details)` needs an iterable, and each -// entry's `address` reaches toAddressHtml(). Absent or falsy is the ordinary -// case and short-circuits before either. -function isRenderableDecoded(value) { - if (!value) return true; - if (!isRecord(value)) return false; - if (!value.details) return true; - if (!Array.isArray(value.details)) return false; - return value.details.every( - (entry) => - isRecord(entry) && (!entry.address || isAddressText(entry.address)), - ); -} - // The pending transaction confirmTx.show() renders: `token` reaches // renderAddressHtml() when it is not "ETH", `from`/`to` reach // addressTitle(), makeBlockie() and getLocalWarnings(), and `amount` reaches @@ -209,7 +194,6 @@ function renderView(view, state, views) { case "success-tx": if (!data.hash) return false; if (!isAddressText(data.to)) return false; - if (!isRenderableDecoded(data.decoded)) return false; // `chainId` picks the network whose explorer the links open, so // when it is there it has to name one. Likewise on error-tx. if (data.chainId && !SUPPORTED_CHAIN_IDS.has(data.chainId)) { diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index 43af5a4..22319d0 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -91,7 +91,7 @@ function decodedAddressHtml(d, chainId) { if (!d.isToken) return toAddressHtml(d.address, chainId); const sym = tokenLabel(d.address, chainId) || "Unknown token"; return ( - `
${escapeHtml(sym)}
` + + `
${escapeHtml(sym)}
` + blockscoutOffTokenNoteHtml(d.address, chainId) + toAddressHtml(d.address, chainId) ); @@ -329,11 +329,11 @@ function decodedDetailsHtml(decoded, chainId) { let html = `
`; if (decoded.name && typeof decoded.name === "string") { html += `
Action
`; - html += `
${escapeHtml(decoded.name)}
`; + html += `
${escapeHtml(decoded.name)}
`; } if (decoded.description && typeof decoded.description === "string") { html += `
Description
`; - html += `
${escapeHtml(decoded.description)}
`; + html += `
${escapeHtml(decoded.description)}
`; } for (const d of lines) { html += `
`; @@ -341,7 +341,7 @@ function decodedDetailsHtml(decoded, chainId) { if (d.address) { html += decodedAddressHtml(d, chainId); } else { - html += `
${escapeHtml(d.value)}
`; + html += `
${escapeHtml(d.value)}
`; } html += `
`; } diff --git a/tests/persistedFieldContract.test.js b/tests/persistedFieldContract.test.js index d1e6c94..a402575 100644 --- a/tests/persistedFieldContract.test.js +++ b/tests/persistedFieldContract.test.js @@ -416,20 +416,6 @@ const CONTRACT = [ views: ["success-tx"], restored: false, }, - { - value: { hash: "0x1", to: ADDRESS, decoded: { details: 7 } }, - views: ["success-tx"], - restored: false, - }, - { - value: { - hash: "0x1", - to: ADDRESS, - decoded: { details: [{ address: 42 }] }, - }, - views: ["success-tx"], - restored: false, - }, // error-tx passes on `data.message`, same dereference. { value: { message: "boom" }, @@ -1082,3 +1068,51 @@ describe("a restore-only hostile value onto", () => { } } }); + +// A saved decoded record comes back from storage, in whatever shape it is in. +// SuccessTx reopens as itself and shows only the lines it can read. +describe("a popup reopened on a SuccessTx with a decoded record in another shape", () => { + function savedSuccess(decoded) { + return restoringOnto("success-tx", { + viewData: { + amount: "1.0000", + symbol: "ETH", + to: ADDRESS, + hash: "0x1", + chainId: "0x1", + decoded, + }, + }); + } + + test.each([ + 7, + "text", + [1, 2], + { details: 7 }, + { details: [null] }, + { details: [7] }, + { details: [{ address: 42 }] }, + { details: [{ label: "Steps", value: "text", address: 42 }] }, + ])("%j shows the amount and the recipient", async (decoded) => { + const page = await bootPopup(savedSuccess(decoded)); + expect(page.pageErrors).toEqual([]); + expect(page.visibleViews()).toEqual(["success-tx"]); + expect(page.hidden("success-tx-decoded")).toBe(true); + expect(page.text("success-tx-summary")).toBe("1.0000 ETH"); + expect(page.node("success-tx-to").innerHTML).toContain(ADDRESS); + }); + + test("a line it can read is shown", async () => { + const page = await bootPopup( + savedSuccess({ + details: [null, { label: "Steps", value: "V3_SWAP_EXACT_IN" }], + }), + ); + expect(page.pageErrors).toEqual([]); + expect(page.visibleViews()).toEqual(["success-tx"]); + expect(page.node("success-tx-decoded").innerHTML).toContain( + "V3_SWAP_EXACT_IN", + ); + }); +}); diff --git a/tests/swapStatusAmount.test.js b/tests/swapStatusAmount.test.js index 10a3159..33a6b3c 100644 --- a/tests/swapStatusAmount.test.js +++ b/tests/swapStatusAmount.test.js @@ -459,7 +459,7 @@ describe("a swap that gives another spender an allowance", () => { expect(html).toContain("Permit2 allowance"); expect(html).toContain("Unlimited"); expect(html).toContain("Allowance token"); - expect(html).toContain('
USDC
'); + expect(html).toContain('
USDC
'); expect(html).toContain(`data-copy="${USDC}"`); } diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index 3d3e4c8..29cae35 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -636,7 +636,7 @@ describe("SuccessTx and ErrorTx for a Sepolia transaction with mainnet active", test("SuccessTx's decoded action names the token from the tokens tracked on Sepolia", async () => { function expectTokenNamed() { expect(getElement("success-tx-decoded").innerHTML).toContain( - '
TDOL
', + '
TDOL
', ); } state.networkTrackedTokens = { @@ -810,7 +810,7 @@ describe("the Permit2 allowance lines of a swap on WaitTx and ErrorTx", () => { expect(html.indexOf("Unlimited")).toBeLessThan( html.indexOf("Allowance token"), ); - expect(html).toContain('
USDC
'); + expect(html).toContain('
USDC
'); expect(html).toContain(`data-copy="${USDC}"`); expect(html).toContain(`href="https://etherscan.io/address/${USDC}"`); expect(html).toMatch(/
.*class="am-address"/); @@ -869,7 +869,7 @@ describe("the Permit2 allowance lines of a swap on WaitTx and ErrorTx", () => { // An amount of about 47 digits or more is wider than the screen. Without the // break-words class its last digits are cut off at the edge. That class keeps // the words of a line such as `(decimals unknown)` whole. -describe("a long amount on WaitTx and ErrorTx", () => { +describe("a long amount on WaitTx, ErrorTx and SuccessTx", () => { test("each amount under the Amount figure wraps", () => { const decoded = { details: [ @@ -896,6 +896,43 @@ describe("a long amount on WaitTx and ErrorTx", () => { expect(html).toMatch( /id="error-tx-summary"\s+class="font-bold break-words"/, ); + expect(html).toMatch( + /id="success-tx-summary"\s+class="font-bold break-words"/, + ); + }); + + test("each amount of SuccessTx's decoded action wraps", () => { + state.viewData = { + hash: TX_HASH, + to: RECIPIENT, + chainId: TX_INFO.chainId, + decoded: { details: [{ label: "Amount", value: "1.0000 ETH" }] }, + }; + txStatus.renderSuccess(); + expect(getElement("success-tx-decoded").innerHTML).toContain( + '
1.0000 ETH<', + ); + }); + + test("the name and description of SuccessTx's decoded action wrap", () => { + state.viewData = { + hash: TX_HASH, + to: RECIPIENT, + chainId: TX_INFO.chainId, + decoded: { + name: "Transfer LONGSYMBOL", + description: "Transfer LONGSYMBOL to a recipient", + details: [{ label: "Amount", value: "1.0000 ETH" }], + }, + }; + txStatus.renderSuccess(); + const html = getElement("success-tx-decoded").innerHTML; + expect(html).toContain( + '
Transfer LONGSYMBOL<', + ); + expect(html).toContain( + '
Transfer LONGSYMBOL to a recipient<', + ); }); });