fix: a saved SuccessTx in another shape reopens as itself, and long amounts wrap (closes #740)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run

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
This commit was merged in pull request #742.
This commit is contained in:
2026-11-03 11:38:35 +01:00
parent 9c691dfa6b
commit 1d40e2d7d5
6 changed files with 97 additions and 39 deletions
+4 -1
View File
@@ -833,7 +833,10 @@
<div id="success-tx-decoded" class="mb-3 hidden text-xs"></div> <div id="success-tx-decoded" class="mb-3 hidden text-xs"></div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">Amount</div> <div class="text-xs text-muted mb-1">Amount</div>
<div id="success-tx-summary" class="font-bold"></div> <div
id="success-tx-summary"
class="font-bold break-words"
></div>
</div> </div>
<div class="mb-3"> <div class="mb-3">
<div class="text-xs text-muted mb-1">To</div> <div class="text-xs text-muted mb-1">To</div>
-16
View File
@@ -116,21 +116,6 @@ function isAddressText(value) {
return isText(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 // The pending transaction confirmTx.show() renders: `token` reaches
// renderAddressHtml() when it is not "ETH", `from`/`to` reach // renderAddressHtml() when it is not "ETH", `from`/`to` reach
// addressTitle(), makeBlockie() and getLocalWarnings(), and `amount` reaches // addressTitle(), makeBlockie() and getLocalWarnings(), and `amount` reaches
@@ -209,7 +194,6 @@ function renderView(view, state, views) {
case "success-tx": case "success-tx":
if (!data.hash) return false; if (!data.hash) return false;
if (!isAddressText(data.to)) return false; if (!isAddressText(data.to)) return false;
if (!isRenderableDecoded(data.decoded)) return false;
// `chainId` picks the network whose explorer the links open, so // `chainId` picks the network whose explorer the links open, so
// when it is there it has to name one. Likewise on error-tx. // when it is there it has to name one. Likewise on error-tx.
if (data.chainId && !SUPPORTED_CHAIN_IDS.has(data.chainId)) { if (data.chainId && !SUPPORTED_CHAIN_IDS.has(data.chainId)) {
+4 -4
View File
@@ -91,7 +91,7 @@ function decodedAddressHtml(d, chainId) {
if (!d.isToken) return toAddressHtml(d.address, chainId); if (!d.isToken) return toAddressHtml(d.address, chainId);
const sym = tokenLabel(d.address, chainId) || "Unknown token"; const sym = tokenLabel(d.address, chainId) || "Unknown token";
return ( return (
`<div class="font-bold">${escapeHtml(sym)}</div>` + `<div class="font-bold break-words">${escapeHtml(sym)}</div>` +
blockscoutOffTokenNoteHtml(d.address, chainId) + blockscoutOffTokenNoteHtml(d.address, chainId) +
toAddressHtml(d.address, chainId) toAddressHtml(d.address, chainId)
); );
@@ -329,11 +329,11 @@ function decodedDetailsHtml(decoded, chainId) {
let html = `<div class="border border-border border-dashed p-2 mb-3">`; let html = `<div class="border border-border border-dashed p-2 mb-3">`;
if (decoded.name && typeof decoded.name === "string") { if (decoded.name && typeof decoded.name === "string") {
html += `<div class="mb-2"><div class="text-xs text-muted mb-1">Action</div>`; html += `<div class="mb-2"><div class="text-xs text-muted mb-1">Action</div>`;
html += `<div class="font-bold">${escapeHtml(decoded.name)}</div></div>`; html += `<div class="font-bold break-words">${escapeHtml(decoded.name)}</div></div>`;
} }
if (decoded.description && typeof decoded.description === "string") { if (decoded.description && typeof decoded.description === "string") {
html += `<div class="mb-2"><div class="text-xs text-muted mb-1">Description</div>`; html += `<div class="mb-2"><div class="text-xs text-muted mb-1">Description</div>`;
html += `<div>${escapeHtml(decoded.description)}</div></div>`; html += `<div class="break-words">${escapeHtml(decoded.description)}</div></div>`;
} }
for (const d of lines) { for (const d of lines) {
html += `<div class="mb-2">`; html += `<div class="mb-2">`;
@@ -341,7 +341,7 @@ function decodedDetailsHtml(decoded, chainId) {
if (d.address) { if (d.address) {
html += decodedAddressHtml(d, chainId); html += decodedAddressHtml(d, chainId);
} else { } else {
html += `<div class="font-bold">${escapeHtml(d.value)}</div>`; html += `<div class="font-bold break-words">${escapeHtml(d.value)}</div>`;
} }
html += `</div>`; html += `</div>`;
} }
+48 -14
View File
@@ -416,20 +416,6 @@ const CONTRACT = [
views: ["success-tx"], views: ["success-tx"],
restored: false, 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. // error-tx passes on `data.message`, same dereference.
{ {
value: { message: "boom" }, 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",
);
});
});
+1 -1
View File
@@ -459,7 +459,7 @@ describe("a swap that gives another spender an allowance", () => {
expect(html).toContain("Permit2 allowance"); expect(html).toContain("Permit2 allowance");
expect(html).toContain("Unlimited"); expect(html).toContain("Unlimited");
expect(html).toContain("Allowance token"); expect(html).toContain("Allowance token");
expect(html).toContain('<div class="font-bold">USDC</div>'); expect(html).toContain('<div class="font-bold break-words">USDC</div>');
expect(html).toContain(`data-copy="${USDC}"`); expect(html).toContain(`data-copy="${USDC}"`);
} }
+40 -3
View File
@@ -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 () => { test("SuccessTx's decoded action names the token from the tokens tracked on Sepolia", async () => {
function expectTokenNamed() { function expectTokenNamed() {
expect(getElement("success-tx-decoded").innerHTML).toContain( expect(getElement("success-tx-decoded").innerHTML).toContain(
'<div class="font-bold">TDOL</div>', '<div class="font-bold break-words">TDOL</div>',
); );
} }
state.networkTrackedTokens = { state.networkTrackedTokens = {
@@ -810,7 +810,7 @@ describe("the Permit2 allowance lines of a swap on WaitTx and ErrorTx", () => {
expect(html.indexOf("Unlimited")).toBeLessThan( expect(html.indexOf("Unlimited")).toBeLessThan(
html.indexOf("Allowance token"), html.indexOf("Allowance token"),
); );
expect(html).toContain('<div class="font-bold">USDC</div>'); expect(html).toContain('<div class="font-bold break-words">USDC</div>');
expect(html).toContain(`data-copy="${USDC}"`); expect(html).toContain(`data-copy="${USDC}"`);
expect(html).toContain(`href="https://etherscan.io/address/${USDC}"`); expect(html).toContain(`href="https://etherscan.io/address/${USDC}"`);
expect(html).toMatch(/<div class="text-xs">.*class="am-address"/); expect(html).toMatch(/<div class="text-xs">.*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 // 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 // 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. // 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", () => { test("each amount under the Amount figure wraps", () => {
const decoded = { const decoded = {
details: [ details: [
@@ -896,6 +896,43 @@ describe("a long amount on WaitTx and ErrorTx", () => {
expect(html).toMatch( expect(html).toMatch(
/id="error-tx-summary"\s+class="font-bold break-words"/, /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(
'<div class="font-bold break-words">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(
'<div class="font-bold break-words">Transfer LONGSYMBOL<',
);
expect(html).toContain(
'<div class="break-words">Transfer LONGSYMBOL to a recipient<',
);
}); });
}); });