fix: give every address a row of its own, so none wraps or is shortened (closes #380)
The wallet list was the reported case. An address there shared one row with the [info] and [x] controls, which took about a third of the width off it, so all 42 characters folded onto a second line. A folded address is not a cosmetic problem: it turns one string the user is meant to compare against a known value into two shorter ones, which is the shape an address-poisoning attack wants. The fix is the layout, not the CSS. renderAddressHtml() -- the single renderer behind every common view that shows an address -- now emits an identity strip (colour dot, wallet title, explorer link, with the ENS name below it) and then the address alone on a full-width row. The wallet list moves [info] and [x] up onto the "Address N" line, which was empty to its right. The transaction rows on Home, the address screen and the token screen carried a truncateMiddle()d counterparty squeezed in beside the amount; they now name it on the amount line where we know it, and carry the whole address on the row below. With the row to itself, an address fits at every nesting depth the popup uses, including the transaction detail wells, which are the narrowest containers it has. .am-address holds nowrap so it cannot fold again, and overflow-x so that if it ever does not fit -- wider glyphs, a zoom -- the user can still reach the last character rather than having it clipped away by #app's overflow-x-hidden with nothing to say it happened. No caller passes maxLen any more, so the 32-character floor that lived in those call sites moved into renderAddressHtml(). truncateMiddle() and its 10-character cap are unchanged: the guarantee has to outlive having no current callers. tests/e2e measures it in a real Chromium rather than asserting on markup: whether an address wrapped is a question about glyph advances and the width of the box it landed in, and nothing in the HTML answers it. Every rendered address is checked for being whole, occupying one line box, fitting its row and ending inside the popup's content box, with the document itself not scrolling sideways -- across Home with a two-address wallet, the address, token, receive, send and transaction detail screens, the confirmation screen and the dApp transaction prompt.
This commit is contained in:
203
tests/e2e/run.js
203
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() {
|
||||
|
||||
Reference in New Issue
Block a user