diff --git a/src/popup/styles/main.css b/src/popup/styles/main.css
index 30532d5..06a6394 100644
--- a/src/popup/styles/main.css
+++ b/src/popup/styles/main.css
@@ -44,3 +44,23 @@ body {
background-color 225ms ease-out,
color 225ms ease-out;
}
+
+/* An address is one atomic string, so it gets a row of its own and never
+ * breaks across lines. A wrapped address reads as two shorter strings, and
+ * two shorter strings are exactly what an address-poisoning attack needs
+ * the user to compare instead of the whole thing. Every view that shows an
+ * address puts it in one of these, alone: the colour dot, the wallet title,
+ * the ENS name and the explorer link all live on their own line above, so
+ * nothing competes with the 42 characters for width.
+ *
+ * overflow-x is the escape hatch, not the mechanism. The row is wide enough
+ * for a full address at every nesting depth the popup uses; if that ever
+ * stops being true — a font with wider glyphs, a browser zoom — the row
+ * scrolls and the user can still reach the last character, rather than the
+ * tail being clipped away by #app's overflow-x-hidden with nothing to say
+ * it happened. tests/e2e asserts the scroll is never actually needed. */
+.am-address {
+ display: block;
+ white-space: nowrap;
+ overflow-x: auto;
+}
diff --git a/src/popup/views/addressDetail.js b/src/popup/views/addressDetail.js
index 58a7da7..4ae35ea 100644
--- a/src/popup/views/addressDetail.js
+++ b/src/popup/views/addressDetail.js
@@ -7,7 +7,6 @@ const {
addressTitle,
escapeHtml,
displaySymbol,
- truncateMiddle,
renderAddressHtml,
attachCopyHandlers,
goBack,
@@ -229,10 +228,12 @@ function renderTransactions(txs) {
const amountStr = tx.value
? escapeHtml(tx.value + " " + sym)
: escapeHtml(sym);
- const maxAddr = Math.max(32, 36 - Math.max(0, amountStr.length - 10));
- const displayAddr =
- title || ensName || truncateMiddle(counterparty, maxAddr);
- const addrStr = escapeHtml(displayAddr);
+ // The counterparty used to be squeezed in beside the amount and
+ // truncated to whatever was left over. It gets its own row now and
+ // is shown whole; the title or ENS name, where there is one, names
+ // it on the line above rather than replacing it.
+ const nameStr = escapeHtml(title || ensName || "");
+ const addrStr = escapeHtml(counterparty);
const dot = addressDotHtml(counterparty);
const err = tx.isError ? " (failed)" : "";
const opacity = tx.isError ? " opacity:0.5;" : "";
@@ -240,7 +241,8 @@ function renderTransactions(txs) {
const iso = escapeHtml(isoDate(tx.timestamp));
html += `
`;
html += `
${ago}${dirLabel}${err}
`;
- html += `
${dot}${addrStr}${amountStr}
`;
+ html += `
${dot}${nameStr}${amountStr}
`;
+ html += `
${addrStr}
`;
html += `
`;
i++;
}
diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js
index 1431be4..35792ae 100644
--- a/src/popup/views/addressToken.js
+++ b/src/popup/views/addressToken.js
@@ -10,7 +10,6 @@ const {
addressTitle,
escapeHtml,
displaySymbol,
- truncateMiddle,
balanceLine,
unknownableAmount,
renderAddressHtml,
@@ -305,10 +304,12 @@ function renderTransactions(txs) {
const amountStr = tx.value
? escapeHtml(tx.value + " " + sym)
: escapeHtml(sym);
- const maxAddr = Math.max(32, 36 - Math.max(0, amountStr.length - 10));
- const displayAddr =
- title || ensName || truncateMiddle(counterparty, maxAddr);
- const addrStr = escapeHtml(displayAddr);
+ // The counterparty used to be squeezed in beside the amount and
+ // truncated to whatever was left over. It gets its own row now and
+ // is shown whole; the title or ENS name, where there is one, names
+ // it on the line above rather than replacing it.
+ const nameStr = escapeHtml(title || ensName || "");
+ const addrStr = escapeHtml(counterparty);
const dot = addressDotHtml(counterparty);
const err = tx.isError ? " (failed)" : "";
const opacity = tx.isError ? " opacity:0.5;" : "";
@@ -316,7 +317,8 @@ function renderTransactions(txs) {
const iso = escapeHtml(isoDate(tx.timestamp));
html += `
`;
html += `
${ago}${dirLabel}${err}
`;
- html += `
${dot}${addrStr}${amountStr}
`;
+ html += `
${dot}${nameStr}${amountStr}
`;
+ html += `
${addrStr}
`;
html += `
`;
i++;
}
diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js
index 4f17deb..057739f 100644
--- a/src/popup/views/helpers.js
+++ b/src/popup/views/helpers.js
@@ -331,6 +331,12 @@ function addressHoldsFunds(addr) {
return false;
}
+// The fewest characters of an address any caller may ask to display. The
+// 10-character cap inside truncateMiddle() is the other half of the same
+// guarantee; this is the half that used to be spelled out at each call
+// site, and is now enforced once in renderAddressHtml().
+const ADDRESS_MIN_DISPLAY_LEN = 32;
+
// Truncate the middle of a string, replacing removed characters with "…".
// Safety: refuses to truncate more than 10 characters, which is the maximum
// that still prevents address spoofing attacks (see Display Consistency in
@@ -518,17 +524,29 @@ function attachCopyHandlers(container) {
// Unified address rendering.
//
-// Produces consistent HTML for any Ethereum address:
-// • Color dot
-// • Optional title (e.g. "Wallet 1 — Address 2") shown bold above address
-// • Optional ENS name shown bold above address
-// • Full address (or truncated via maxLen) with dashed-underline click-to-copy
-// • Etherscan external link icon
+// Two stacked rows, in this order:
+// 1. Identity strip — colour dot, optional title (e.g. "Wallet 1 —
+// Address 2") and the explorer link icon. Optional ENS name below it.
+// 2. The address itself, alone on a full-width row that never wraps
+// (see .am-address in styles/main.css).
+//
+// The split is the point. Everything used to sit on one line: dot, address
+// and link together, with `break-all` to let the address fold when the line
+// ran out. In the wallet list, where the row also carried [info] and [x],
+// it ran out every time — the bug in #380 — and a folded address is a
+// spoofing hazard, not a cosmetic one. Nothing shares the address's row
+// now, so all 42 characters fit at every nesting depth the popup uses and
+// nothing has to be dropped or folded to make room.
//
// Options object:
// title — wallet title string (from addressTitle)
// ensName — ENS name string
-// maxLen — if set, truncate address display (min 32 chars enforced)
+// maxLen — if set, truncate address display. Floored at 32 characters
+// here rather than by the caller: no view passes it any more
+// (every address row is wide enough for all 42 characters),
+// so a floor that lived in the callers would have gone away
+// with them, and the "at least 32 characters" guarantee has
+// to survive having no current callers to be a guarantee.
// noLink — if true, omit etherscan link
//
// After inserting the returned HTML into the DOM, call
@@ -536,22 +554,22 @@ function attachCopyHandlers(container) {
function renderAddressHtml(address, opts) {
const { title, ensName, maxLen, noLink } = opts || {};
const dot = addressDotHtml(address);
- const displayAddr = maxLen ? truncateMiddle(address, maxLen) : address;
+ const displayAddr = maxLen
+ ? truncateMiddle(address, Math.max(ADDRESS_MIN_DISPLAY_LEN, maxLen))
+ : address;
const link = etherscanAddressUrl(address);
const extLink = noLink ? "" : etherscanLinkHtml(link);
let html = "";
+ html += `
${dot}`;
if (title) {
- html += `
${dot}${escapeHtml(title)}
`;
+ html += `
${escapeHtml(title)}`;
}
+ html += `${extLink}
`;
if (ensName) {
- html += `
${title ? "" : dot}${escapeHtml(ensName)}
`;
- }
- if (title || ensName) {
- html += `
${copyableHtml(displayAddr, "break-all")}${extLink}
`;
- } else {
- html += `
${dot}${copyableHtml(displayAddr, "break-all")}${extLink}
`;
+ html += `
${escapeHtml(ensName)}
`;
}
+ html += `
${copyableHtml(displayAddr)}
`;
return html;
}
diff --git a/src/popup/views/home.js b/src/popup/views/home.js
index 21aadaf..9c053f6 100644
--- a/src/popup/views/home.js
+++ b/src/popup/views/home.js
@@ -9,7 +9,6 @@ const {
addressTitle,
escapeHtml,
displaySymbol,
- truncateMiddle,
renderAddressHtml,
attachCopyHandlers,
pushCurrentView,
@@ -117,10 +116,13 @@ function renderHomeTxList(ctx) {
const amountStr = tx.value
? escapeHtml(tx.value + " " + sym)
: escapeHtml(sym);
+ // The counterparty used to be squeezed in beside the amount and
+ // truncated to whatever was left over. It gets its own row now and
+ // is shown whole; the title, when it is one of our own addresses,
+ // names it on the line above rather than replacing it.
const title = addressTitle(counterparty, state.wallets);
- const maxAddr = Math.max(32, 36 - Math.max(0, amountStr.length - 10));
- const displayAddr = title || truncateMiddle(counterparty, maxAddr);
- const addrStr = escapeHtml(displayAddr);
+ const titleStr = title ? escapeHtml(title) : "";
+ const addrStr = escapeHtml(counterparty);
const dot = addressDotHtml(counterparty);
const err = tx.isError ? " (failed)" : "";
const opacity = tx.isError ? " opacity:0.5;" : "";
@@ -128,7 +130,8 @@ function renderHomeTxList(ctx) {
const iso = escapeHtml(isoDate(tx.timestamp));
html += `
`;
html += `
${ago}${dirLabel}${err}
`;
- html += `
${dot}${addrStr}${amountStr}
`;
+ html += `
${dot}${titleStr}${amountStr}
`;
+ html += `
${addrStr}
`;
html += `
`;
i++;
}
@@ -252,17 +255,22 @@ function walletListHtml() {
: "";
const dot = addressDotHtml(addr.address);
const titleBold = isActive ? "font-bold" : "";
- html += `
Address ${ai + 1}
`;
+ // [info] and [x] ride on the "Address N" line, which was empty
+ // to its right, so the address below gets the row to itself.
+ // They used to sit beside the address and take about a third of
+ // the width off it, which is what made a 42-character address
+ // fold onto a second line here and nowhere else (#380).
+ html += `
`;
+ html += `${dot}Address ${ai + 1}`;
+ html += `${infoBtn}${removeBtn}`;
+ html += `
`;
if (addr.ensName) {
// An ENS reverse record is whatever the name owner set it
// to; renderAddressHtml() escapes its own copy of this and
// this list was the one that did not.
- html += `
${dot}${escapeHtml(addr.ensName)}
`;
+ html += `
${escapeHtml(addr.ensName)}
`;
}
- html += `
`;
- html += `${addr.ensName ? "" : dot}${escapeHtml(addr.address)}`;
- html += `${infoBtn}${removeBtn}`;
- html += `
`;
+ html += `
${escapeHtml(addr.address)}
`;
const addrTotal = formatAddressTotal(getAddressValue(addr));
html += `
${addrTotal || " "}
`;
html += balanceLinesForAddress(
diff --git a/src/popup/views/transactionDetail.js b/src/popup/views/transactionDetail.js
index 3831f27..f3df42a 100644
--- a/src/popup/views/transactionDetail.js
+++ b/src/popup/views/transactionDetail.js
@@ -137,10 +137,16 @@ function render() {
if (tx.contractAddress) {
const dot = addressDotHtml(tx.contractAddress);
const link = explorerUrl("token", tx.contractAddress);
+ // Hand-rolled rather than renderAddressHtml() because the
+ // link goes to the explorer's /token/ page, not /address/.
+ // Same two-row shape though: dot and link on the strip, the
+ // contract address alone on the row below it.
tokenContractEl.innerHTML =
`
${dot}` +
- copyableHtml(tx.contractAddress, "break-all") +
etherscanLinkHtml(link) +
+ `
` +
+ `
` +
+ copyableHtml(tx.contractAddress) +
`
`;
tokenContractSection.classList.remove("hidden");
} else {
diff --git a/tests/e2e/run.js b/tests/e2e/run.js
index f467a65..48eb11c 100644
--- a/tests/e2e/run.js
+++ b/tests/e2e/run.js
@@ -1525,6 +1525,7 @@ async function goToConfirm(page, { token, balance, amount }) {
await page.fill("#send-amount", amount);
await page.click("#btn-send-review");
await visible(page, "#view-confirm-tx");
+ await assertAddressesFit(page, "the confirmation screen");
}
// A balance as the main view renders it: balanceLinesForAddress() writes
@@ -3262,6 +3263,8 @@ test("eth_sendTransaction signs the approved transaction and broadcasts it (#183
JSON.stringify(screen.data),
);
+ await assertAddressesFit(popup, "the dApp transaction prompt");
+
const broadcastBefore = env.routeOpts.broadcastTransactions.length;
await popup.fill("#approve-tx-password", PASSWORD);
await popup.click("#btn-approve-tx");
@@ -3425,6 +3428,206 @@ test("the password never crossed either boundary in this section (#183)", async
await env.dapp.close();
});
+// ------------------------------------------- address layout (#380)
+//
+// "addresses should never wrap in the common views. this doesn't mean to
+// just change the css, but update the layout itself so the untruncated
+// addresses are shown in full and don't mess up the layout."
+//
+// Every one of these questions is about glyph advances and the width of
+// the box an address landed in, and nothing in the markup answers any of
+// them: a row can hold `white-space: nowrap` and still be too narrow, and
+// the popup's own `overflow-x-hidden` would then hide the evidence by
+// clipping the tail. So they are measured in a real Chromium, on the real
+// rendered views, one assertion per property #380 names:
+//
+// - the whole address is there (42 characters, no ellipsis)
+// - it occupies exactly one line box
+// - it fits its row, so the overflow-x escape hatch never engages
+// - its row ends inside the popup's content box
+// - and the document itself does not scroll sideways
+//
+// The narrowest containers the popup has are covered here — the
+// transaction detail wells (`bg-well p-3 mx-1`) and the token contract
+// well — so the wider ones cannot fail while these pass.
+
+// Everything on screen that carries an address, measured in one pass.
+// Views other than the current one are display:none and measure zero, so
+// filtering on width leaves exactly what a user can see right now.
+function addressRowReport(page) {
+ return page.evaluate(() => {
+ const app = document.getElementById("app");
+ const appRight = app.getBoundingClientRect().right;
+ const rows = [];
+ for (const el of document.querySelectorAll(".am-address")) {
+ const box = el.getBoundingClientRect();
+ if (box.width === 0) continue;
+ // Line boxes are counted off the inline content, because the
+ // element's own rect is one box whether the text inside it
+ // wrapped or not. A Range yields a rect per contained node as
+ // well as per line, so it is the distinct tops that count:
+ // a copyable span and the text inside it share one.
+ const range = document.createRange();
+ range.selectNodeContents(el);
+ const tops = new Set(
+ Array.from(range.getClientRects()).map((r) =>
+ Math.round(r.top),
+ ),
+ );
+ rows.push({
+ text: el.innerText.trim(),
+ lineBoxes: tops.size,
+ overflow: el.scrollWidth - el.clientWidth,
+ overhang: Math.round(box.right - appRight),
+ });
+ }
+ return {
+ rows,
+ pageOverflow:
+ document.documentElement.scrollWidth -
+ document.documentElement.clientWidth,
+ };
+ });
+}
+
+async function assertAddressesFit(page, where) {
+ const report = await addressRowReport(page);
+ assert(
+ report.rows.length > 0,
+ where + ": no address rows were rendered, so nothing was measured",
+ );
+ for (const row of report.rows) {
+ assert(
+ /^0x[0-9a-fA-F]{40}$/.test(row.text),
+ where +
+ ": the address is not shown whole: " +
+ JSON.stringify(row.text),
+ );
+ assert(
+ row.lineBoxes === 1,
+ where +
+ ": " +
+ row.text +
+ " wrapped onto " +
+ row.lineBoxes +
+ " lines",
+ );
+ assert(
+ row.overflow <= 1,
+ where +
+ ": " +
+ row.text +
+ " is " +
+ row.overflow +
+ "px wider than the row holding it",
+ );
+ assert(
+ row.overhang <= 1,
+ where +
+ ": " +
+ row.text +
+ " reaches " +
+ row.overhang +
+ "px past the popup's content box",
+ );
+ }
+ assert(
+ report.pageOverflow <= 0,
+ where + ": the popup scrolls sideways by " + report.pageOverflow + "px",
+ );
+ return report.rows.length;
+}
+
+// Back to Home from wherever the suite above finished, without assuming
+// which screen that was. Every screen the popup can rest on has a Back
+// button, and Home has none, so unwinding until Home shows is the one
+// route that does not depend on the order of the tests before this point.
+async function unwindToHome(page) {
+ for (let i = 0; i < 12; i++) {
+ if (await page.isVisible("#view-main")) return;
+ const back = page
+ .locator(".view:not(.hidden) button", { hasText: "Back" })
+ .first();
+ if ((await back.count()) === 0) break;
+ await back.click();
+ await page.waitForTimeout(150);
+ }
+ await visible(page, "#view-main");
+}
+
+// The reproduction from the issue: a wallet holding more than one address.
+// Every address in the list is a full 42 characters competing with the
+// [info] and [x] controls for one row's width, which is the state the
+// wallet view was reported wrapping in.
+test("a wallet with two addresses lists both in full, unwrapped (#380)", async (env) => {
+ await unwindToHome(env.page);
+
+ const before = await env.page
+ .locator("#wallet-list .btn-addr-info")
+ .count();
+ await env.page.locator("#wallet-list .btn-add-address").first().click();
+ await env.page.waitForFunction(
+ (n) =>
+ document.querySelectorAll("#wallet-list .btn-addr-info").length > n,
+ before,
+ { timeout: 60000 },
+ );
+
+ const shown = await assertAddressesFit(env.page, "the wallet list");
+ assert(
+ shown >= before + 1,
+ "the wallet list measured " +
+ shown +
+ " addresses, fewer than the " +
+ (before + 1) +
+ " it now holds",
+ );
+
+ // The [x] control only exists on a wallet holding more than one
+ // address, so its presence is also the proof the second one landed.
+ const removable = await env.page
+ .locator("#wallet-list .btn-remove-address")
+ .count();
+ assert(removable > 0, "the second address did not reach the wallet list");
+});
+
+test("every common view shows its addresses in full on one line (#380)", async (env) => {
+ await unwindToHome(env.page);
+ await assertAddressesFit(env.page, "Home");
+
+ await env.page.locator("#wallet-list .btn-addr-info").first().click();
+ await visible(env.page, "#view-address");
+ await visible(env.page, "#tx-list .tx-row");
+ await assertAddressesFit(env.page, "the address screen");
+
+ await env.page.click("#btn-receive");
+ await visible(env.page, "#view-receive");
+ await assertAddressesFit(env.page, "the receive screen");
+ await env.page.click("#btn-receive-back");
+ await visible(env.page, "#view-address");
+
+ await env.page.click("#btn-send");
+ await visible(env.page, "#view-send");
+ await assertAddressesFit(env.page, "the send screen");
+ await env.page.click("#btn-send-back");
+ await visible(env.page, "#view-address");
+
+ // The transaction detail screen carries the narrowest address rows in
+ // the popup: its fields sit inside a well that takes another 24px of
+ // padding and 8px of margin off the content width, and the token
+ // contract row there is narrower still.
+ await env.page.locator("#address-balances .balance-row").first().click();
+ await visible(env.page, "#view-address-token");
+ await assertAddressesFit(env.page, "the token screen");
+ await env.page.click("#btn-address-token-back");
+ await visible(env.page, "#view-address");
+
+ await env.page.locator("#tx-list .tx-row").first().click();
+ await visible(env.page, "#view-transaction");
+ await visible(env.page, "#tx-detail-token-contract-section");
+ await assertAddressesFit(env.page, "the transaction detail screen");
+});
+
// ---------------------------------------------------------------- runner
async function main() {