diff --git a/README.md b/README.md index 975c2ce..718250d 100644 --- a/README.md +++ b/README.md @@ -479,6 +479,9 @@ papered over: forced by clicking Reveal and the settings gear in one page task, which a person cannot do; the case a person can hit, the first decrypt after the popup opens while libsodium is still loading, is not driven. +- Approval windows open at 1280x720 whatever size the extension asks for, so + whether an approval screen fits its window is checked in the Firefox suite + only. Any test that drives a failure path on purpose declares the `console.error` it is about to provoke, via `errors.expect()`. That is not a mute: the declaration @@ -573,7 +576,9 @@ suite), wallet creation through the UI, the Add Token screen, and the dApp round trips — `eth_requestAccounts`, `personal_sign`, `wallet_switchEthereumChain` rejected and then approved, `eth_sendTransaction`, and a closed approval window rejecting with EIP-1193 4001 — driven through the real content script, -background page and approval windows. +background page and approval windows. The `personal_sign` prompt is also +required to fit its window without scrolling sideways, with the extension's +pages at 170% zoom. The suite lives in `tests/e2e/firefox/`. Its WebDriver client (`driver.js`) has **no npm dependencies at all**: it is built on global `fetch` and @@ -947,8 +952,16 @@ discoverable. enough that vanilla JS with simple view switching is sufficient. A framework would add bundle size, build complexity, and attack surface for no benefit at this scale. -- **360x600 popup**: Standard browser extension popup dimensions. The UI is - designed for this fixed viewport. +- **396x600 popup**: The page is 396 pixels wide (`body` in + `src/popup/styles/main.css`), and 600 pixels is as tall as the browser lets + the toolbar popup grow. The UI is designed for this fixed viewport. The window + an approval opens in is given the same size. The browser counts the window's + frame in that width, and a screen taller than the window gives some of it to a + vertical scrollbar, so once the approval's screen is up the page widens its + window by however far it would scroll sideways, scaled by the browser's zoom. + Firefox draws some zoom steps a little larger than the zoom it reports, so the + page measures again once the window has grown, and widens it again while it + still falls short. #### No Layout Shift diff --git a/src/background/index.js b/src/background/index.js index 8b3fe77..3875339 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -453,7 +453,10 @@ function releaseApproval(approval) { // the result on the floor, so a rejection here would be unhandled. async function openApprovalWindow(id) { const popupUrl = runtime.getURL("src/popup/index.html?approval=" + id); - const popupWidth = 360; + // As wide as the page: body in src/popup/styles/main.css. The browser + // counts the window's frame in this width; the page itself adds back + // whatever the frame takes (src/popup/index.js). + const popupWidth = 396; const popupHeight = 600; let currentWin = null; @@ -473,7 +476,7 @@ async function openApprovalWindow(id) { // Centred on a browser window only. The last focused window can be // another approval window still open, and centring on one can give a // position the browser refuses ("Bounds must be at least 50% within - // visible screen space"): headless Chrome reports this 360x600 popup as + // visible screen space"): headless Chrome reports this 396x600 popup as // 1280x720. The request then failed with no window at all. Over a popup, // the browser picks the position. if (currentWin && currentWin.type === "normal") { diff --git a/src/popup/index.js b/src/popup/index.js index 6999cc9..0021a30 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -21,6 +21,12 @@ const { goBack, } = require("./views/helpers"); const { applyTheme } = require("./theme"); +const { + tabsGetCurrent, + tabsGetZoom, + windowsGetCurrent, + windowsUpdate, +} = require("../shared/browserApi"); // Renders a view the popup lands on without having navigated to it forward: // on restore here, and on Back. Only the views that can be fully re-rendered // from persisted state (RESTORABLE_VIEWS, src/shared/restorableViews.js) go @@ -163,6 +169,40 @@ function fallbackView() { showView("main"); } +// The background opens the approval window as wide as the page, but the +// browser counts the window's frame in that width, and a screen taller than +// the window takes a vertical scrollbar out of it too. So once the screen is +// up, the window is widened by however far the page would scroll sideways. +// That distance is in page pixels and the window's width in screen pixels: +// at 150% zoom one page pixel is 1.5 screen pixels. The zoom is read for this +// page's own tab, because asked without a tab id Firefox answers for the +// window focused last. Firefox draws some zoom steps a little larger than the +// zoom it reports, 170% among them, so once the page has its new width (its +// resize event) it is measured again, and the window widened again while the +// page still falls short, three times at most. A window that cannot grow sends +// no resize event, and the page stops there. The connection prompt can also +// show in the toolbar popup, whose window is the user's browser window; that +// one is left alone. +async function widenApprovalWindow() { + const page = document.documentElement; + if (page.scrollWidth <= page.clientWidth) return; + const win = await windowsGetCurrent(); + if (win.type !== "popup") return; + const tab = await tabsGetCurrent(); + const zoom = await tabsGetZoom(tab.id); + let width = win.width; + for (let tries = 0; tries < 3; tries++) { + const sideways = page.scrollWidth - page.clientWidth; + if (sideways <= 0) return; + width += Math.ceil(sideways * zoom); + const resized = new Promise((resolve) => + window.addEventListener("resize", resolve, { once: true }), + ); + await windowsUpdate(win.id, { width }); + await resized; + } +} + async function init() { // First, before anything can save: showView() saves on every navigation // without awaiting, so a save that fails from here on has somewhere to be @@ -239,8 +279,8 @@ async function init() { // the run on both harnesses (Playwright `pageerror`, and the Firefox // driver's console-service drain), so nothing is lost by leaving it // on that path. show() puts the approval's own screen up once the - // background has described it. - approval.show(approvalId); + // background has described it; the window is widened to fit it then. + approval.show(approvalId).then(widenApprovalWindow); return; } diff --git a/src/shared/browserApi.js b/src/shared/browserApi.js index 8584528..48064c9 100644 --- a/src/shared/browserApi.js +++ b/src/shared/browserApi.js @@ -243,6 +243,22 @@ function tabsSendMessage(tabId, message) { return invoke(tabsApi(), "sendMessage", tabId, message); } +/** + * @returns {Promise} the tab the calling page is in, or + * undefined for a page not in a tab, such as the toolbar popup. + */ +function tabsGetCurrent() { + return invoke(tabsApi(), "getCurrent"); +} + +/** + * @param {number} tabId + * @returns {Promise} that tab's zoom: 1.5 at 150%. + */ +function tabsGetZoom(tabId) { + return invoke(tabsApi(), "getZoom", tabId); +} + /** * @param {Object} createData * @returns {Promise} the created window. @@ -258,6 +274,22 @@ function windowsGetLastFocused() { return invoke(windowsApi(), "getLastFocused"); } +/** + * @returns {Promise} the window the calling page is in. + */ +function windowsGetCurrent() { + return invoke(windowsApi(), "getCurrent"); +} + +/** + * @param {number} windowId + * @param {Object} updateInfo + * @returns {Promise} the updated window. + */ +function windowsUpdate(windowId, updateInfo) { + return invoke(windowsApi(), "update", windowId, updateInfo); +} + /** * @param {number} windowId * @returns {Promise} @@ -280,10 +312,14 @@ module.exports = { storageRemove, storageSet, tabsApi, + tabsGetCurrent, + tabsGetZoom, tabsQuery, tabsSendMessage, windowsApi, windowsCreate, + windowsGetCurrent, windowsGetLastFocused, windowsRemove, + windowsUpdate, }; diff --git a/tests/approvalWindow.test.js b/tests/approvalWindow.test.js index 6f81442..ff68313 100644 --- a/tests/approvalWindow.test.js +++ b/tests/approvalWindow.test.js @@ -45,3 +45,132 @@ test("the approval window shows no screen until the background describes the app expect(env.visibleViews()).toEqual(["approve-network"]); expect(env.pageErrors).toEqual([]); }); + +// The background opens the approval window as wide as the page, but the +// browser counts the window's frame in that width, and a screen taller than +// the window takes a vertical scrollbar out of it too +// (https://git.eeqj.de/sneak/AutistMask/issues/518). The browser suites run +// without frames, and only the Firefox suite's sign prompt at a zoom other +// than 100%, so the widening is checked here too. +// +// A window 396 screen pixels wide whose page is drawn at `drawn` screen pixels +// to the page pixel. Widening it gives the page its new width, less the 15 +// screen pixels of the scrollbar the approval screen has, and then calls the +// page's resize listeners, each of which asked to be called once. +function windowsStub(type, drawn) { + const updates = []; + return { + updates, + getCurrent: (done) => done({ id: 7, type, width: 396 }), + update: (id, info, done) => { + updates.push({ id, info }); + done({}); + globalThis.document.documentElement.clientWidth = Math.floor( + (info.width - 15) / drawn, + ); + const resized = globalThis.window.listeners.resize || []; + globalThis.window.listeners.resize = []; + for (const fn of resized) fn(); + }, + }; +} + +// The approval page's own tab is at `zoom`. Asked without a tab id, Firefox +// answers for the window focused last, here a site's tab at 100%. +function tabsStub(zoom) { + return { + getCurrent: (done) => done({ id: 11 }), + getZoom: (...args) => { + const done = args.pop(); + done(args[0] === 11 ? zoom : 1); + }, + }; +} + +// An approval window at `zoom`, `clientWidth` page pixels wide before its +// screen is up, whose background holds its answer until the test gives it. +async function bootZoomedApprovalWindow(windows, zoom, clientWidth) { + let answer = null; + const env = await bootPopup(unversionedValidProfile(), { + search: "?approval=approval-1", + runtime: { + connect: () => ({ postMessage: () => {} }), + sendMessage: (msg, reply) => { + if (msg.type === "AUTISTMASK_GET_APPROVAL") answer = reply; + }, + }, + windows, + tabs: tabsStub(zoom), + documentElement: { scrollWidth: 396, clientWidth }, + }); + return { env, answer }; +} + +test("at 150% zoom the approval window widens by what its screen lacks, scrollbar included", async () => { + const windows = windowsStub("popup", 1.5); + // At 150% a window 396 screen pixels wide shows 264 page pixels. + const { env, answer } = await bootZoomedApprovalWindow(windows, 1.5, 264); + + // Not before there is a screen to fit. + expect(windows.updates).toEqual([]); + + // The screen is taller than the window, so once it is up a vertical + // scrollbar takes 15 screen pixels, 10 page pixels at 150%. + env.document.documentElement.clientWidth = 254; + answer({ + type: "network", + origin: "https://dapp.example", + currentNetworkId: "mainnet", + requestedNetworkId: "sepolia", + isPhishingDomain: false, + }); + await settle(); + + // 396 - 254 = 142 page pixels short, which at 150% is 213 screen pixels. + // At the 100% of the site's tab it would be 142, and the screen would + // still not fit. + expect(env.visibleViews()).toEqual(["approve-network"]); + expect(windows.updates).toEqual([{ id: 7, info: { width: 609 } }]); + expect(env.document.documentElement.clientWidth).toBe(396); + expect(env.pageErrors).toEqual([]); +}); + +test("at 170% the approval window is widened again while its screen still falls short", async () => { + // Firefox reports 170% as 1.7 but draws a page pixel about 1.714 screen + // pixels wide, so a window 396 wide shows 231 page pixels. + const windows = windowsStub("popup", 1.714); + const { env, answer } = await bootZoomedApprovalWindow(windows, 1.7, 231); + + // Less the scrollbar once the screen is up. + env.document.documentElement.clientWidth = 222; + answer({ + type: "network", + origin: "https://dapp.example", + currentNetworkId: "mainnet", + requestedNetworkId: "sepolia", + isPhishingDomain: false, + }); + await settle(); + + // 174 page pixels short, so 296 screen pixels wider at 1.7. That shows 394 + // page pixels, still 2 short, so 4 screen pixels wider again. + expect(windows.updates).toEqual([ + { id: 7, info: { width: 692 } }, + { id: 7, info: { width: 696 } }, + ]); + const page = env.document.documentElement; + expect(page.scrollWidth).toBeLessThanOrEqual(page.clientWidth); + expect(env.pageErrors).toEqual([]); +}); + +test("a connection prompt in the toolbar popup leaves the browser window alone", async () => { + const windows = windowsStub("normal", 1.5); + const { env, answer } = await bootZoomedApprovalWindow(windows, 1.5, 264); + + answer({ origin: "https://dapp.example", isPhishingDomain: false }); + await settle(); + + expect(env.visibleViews()).toEqual(["approve-site"]); + expect(windows.updates).toEqual([]); + expect(env.pageErrors).toEqual([]); +}); diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index c86202a..a89a477 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -2930,7 +2930,7 @@ describe("where an approval window opens", () => { await settle(); expect(bg.created).toHaveLength(1); - expect(bg.created[0]).toMatchObject({ left: 460, top: 60 }); + expect(bg.created[0]).toMatchObject({ left: 442, top: 60 }); }); test("not centred on an approval window the user was last in", async () => { @@ -2947,7 +2947,7 @@ describe("where an approval window opens", () => { bg.requestSign(); await settle(); - // Centred, it would be at left 900, the position the browser refused. + // Centred, it would be at left 882, the position the browser refused. expect(bg.created).toHaveLength(1); expect(bg.created[0].left).toBeUndefined(); expect(bg.created[0].top).toBeUndefined(); @@ -2969,7 +2969,7 @@ describe("where an approval window opens", () => { await settle(); expect(bg.created).toHaveLength(2); - expect(bg.created[0]).toMatchObject({ left: 1520, top: 750 }); + expect(bg.created[0]).toMatchObject({ left: 1502, top: 750 }); expect(bg.created[1].left).toBeUndefined(); expect(bg.created[1].top).toBeUndefined(); diff --git a/tests/e2e/firefox/run.js b/tests/e2e/firefox/run.js index 8aed5c7..b6f1fc7 100644 --- a/tests/e2e/firefox/run.js +++ b/tests/e2e/firefox/run.js @@ -394,6 +394,26 @@ async function waitForApprovalWindow(d, timeout = 30000) { } } +// Firefox keeps one zoom per site, so a zoom set on the popup's tab holds for +// every page of the extension, the approval windows included. +async function setExtensionZoom(env, zoom) { + const d = env.driver; + await d.switchToWindow(env.popupWindow); + const outcome = await d.executeAsync( + `const done = arguments[arguments.length - 1]; + const zoom = arguments[0]; + browser.tabs + .getCurrent() + .then((tab) => browser.tabs.setZoom(tab.id, zoom)) + .then(() => done({}), (e) => done({ error: String(e) }));`, + [zoom], + ); + assert( + outcome && !outcome.error, + "could not set the extension's zoom: " + (outcome && outcome.error), + ); +} + function startRequest(d, key, method, params) { return d.execute( "window.__dapp.start(arguments[0], arguments[1], arguments[2]);" + @@ -633,6 +653,14 @@ step( "personal_sign returns a signature that recovers to the address", async (env) => { const d = env.driver; + // The window is opened as wide as the page, and widened once the + // screen is up if it still scrolls sideways + // (https://git.eeqj.de/sneak/AutistMask/issues/518). This suite is + // where that is checked: the Chrome suite's approval windows open at + // 1280x720 whatever width the extension asks for. At 170%, one of + // Firefox's zoom steps, the page is drawn a little larger than the + // 1.7 Firefox reports, so a single widening leaves it a pixel short. + await setExtensionZoom(env, 1.7); await d.switchToWindow(env.dappWindow); await startRequest(d, "sign", "personal_sign", [SIGN_HEX, env.address]); @@ -640,6 +668,12 @@ step( await d.switchToWindow(popup); await d.waitVisible("#view-approve-sign"); + await d.waitFor( + "the sign prompt to fit its window without scrolling sideways", + "return document.documentElement.scrollWidth <=" + + " document.documentElement.clientWidth;", + ); + const screen = await d.execute( `return { origin: document.getElementById("approve-sign-origin").textContent, @@ -674,6 +708,7 @@ step( await d.switchToWindow(env.dappWindow); const outcome = await settleRequest(d, "sign"); + await setExtensionZoom(env, 1); assert( outcome.settled === "resolved", "personal_sign did not resolve: " + JSON.stringify(outcome), diff --git a/tests/support/popupBoot.js b/tests/support/popupBoot.js index 308d0d4..dce68f4 100644 --- a/tests/support/popupBoot.js +++ b/tests/support/popupBoot.js @@ -242,6 +242,13 @@ async function settle() { * "?approval=" and an id for the popup opened as an approval window. * @param {object} [options.runtime] members of chrome.runtime that replace the * stub's own, for a test that has to answer the background's messages. + * @param {object} [options.windows] chrome.windows, for a test of the approval + * window resizing itself. + * @param {object} [options.tabs] chrome.tabs, for the same test: the page's + * tab and its zoom. + * @param {object} [options.documentElement] members of document.documentElement + * that replace the stub's own: scrollWidth and clientWidth, which are equal + * unless a test says otherwise, so the page does not scroll sideways. * @returns {Promise} handles onto the booted page. */ async function bootPopup(stored, options) { @@ -277,7 +284,13 @@ async function bootPopup(stored, options) { (options && options.storage) || makeStorageStub(stored === undefined ? {} : { autistmask: stored }); const document = makeDocument(POPUP_HTML); + Object.assign( + document.documentElement, + { scrollWidth: 396, clientWidth: 396 }, + options && options.documentElement, + ); const reloads = []; + const windowListeners = {}; globalThis.chrome = { storage: { local: storage.local }, @@ -287,6 +300,8 @@ async function bootPopup(stored, options) { onMessage: { addListener: () => {} }, ...(options && options.runtime), }, + windows: options && options.windows, + tabs: options && options.tabs, }; globalThis.document = document; globalThis.window = { @@ -300,7 +315,13 @@ async function bootPopup(stored, options) { addEventListener: () => {}, removeEventListener: () => {}, }), - addEventListener: () => {}, + // Recorded, as the document's are, so a test that widens the approval + // window can call the page's resize listeners. + listeners: windowListeners, + addEventListener: (name, fn) => { + windowListeners[name] = windowListeners[name] || []; + windowListeners[name].push(fn); + }, }; // The ten-second refresh init() starts, and a transaction wait's timers, // would outlive the test. So every interval is recorded rather than