diff --git a/README.md b/README.md index 98485d7..e540937 100644 --- a/README.md +++ b/README.md @@ -464,6 +464,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 @@ -536,7 +539,8 @@ 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. 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 @@ -893,8 +897,11 @@ 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 its width, so the page widens the window by whatever the frame takes. #### No Layout Shift diff --git a/src/background/index.js b/src/background/index.js index 00fa439..c589f92 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -423,7 +423,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; @@ -443,7 +446,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..df72634 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -21,6 +21,7 @@ const { goBack, } = require("./views/helpers"); const { applyTheme } = require("./theme"); +const { 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 @@ -233,6 +234,22 @@ async function init() { const params = new URLSearchParams(window.location.search); const approvalId = params.get("approval"); if (approvalId) { + // The background opens this window as wide as the page, but the + // browser counts the window's frame in that width, and how wide the + // frame is depends on the browser and the system. The window is + // widened by however far the page would scroll sideways. The + // connection prompt can also show in the toolbar popup, whose window + // is the user's browser window; that one is left alone. + const sideways = + document.documentElement.scrollWidth - + document.documentElement.clientWidth; + if (sideways > 0) { + windowsGetCurrent().then((win) => { + if (win.type === "popup") { + windowsUpdate(win.id, { width: win.width + sideways }); + } + }); + } // Deliberately not awaited, and deliberately not .catch()ed. show() // is async, so a throw past its first await surfaces as an unhandled // rejection rather than an uncaught error — measured as still failing diff --git a/src/shared/browserApi.js b/src/shared/browserApi.js index c512286..1122afe 100644 --- a/src/shared/browserApi.js +++ b/src/shared/browserApi.js @@ -250,6 +250,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} @@ -275,6 +291,8 @@ module.exports = { tabsSendMessage, windowsApi, windowsCreate, + windowsGetCurrent, windowsGetLastFocused, windowsRemove, + windowsUpdate, }; diff --git a/tests/approvalWindow.test.js b/tests/approvalWindow.test.js index 6f81442..58522f0 100644 --- a/tests/approvalWindow.test.js +++ b/tests/approvalWindow.test.js @@ -45,3 +45,50 @@ 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 +// (https://git.eeqj.de/sneak/AutistMask/issues/518). The browser suites run +// without frames, so the widening is checked here. +function windowsStub(type) { + const updates = []; + return { + updates, + getCurrent: (done) => done({ id: 7, type, width: 396 }), + update: (id, info, done) => { + updates.push({ id, info }); + done({}); + }, + }; +} + +const approvalRuntime = { + connect: () => ({ postMessage: () => {} }), + sendMessage: () => {}, +}; + +test("the approval window widens itself by whatever its frame takes", async () => { + const windows = windowsStub("popup"); + const env = await bootPopup(unversionedValidProfile(), { + search: "?approval=approval-1", + runtime: approvalRuntime, + windows, + documentElement: { scrollWidth: 396, clientWidth: 380 }, + }); + + expect(windows.updates).toEqual([{ id: 7, info: { width: 412 } }]); + expect(env.pageErrors).toEqual([]); +}); + +test("a connection prompt in the toolbar popup leaves the browser window alone", async () => { + const windows = windowsStub("normal"); + const env = await bootPopup(unversionedValidProfile(), { + search: "?approval=approval-1", + runtime: approvalRuntime, + windows, + documentElement: { scrollWidth: 396, clientWidth: 380 }, + }); + + expect(windows.updates).toEqual([]); + expect(env.pageErrors).toEqual([]); +}); diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index f0ad395..9a4cc3d 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -2805,7 +2805,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 () => { @@ -2822,7 +2822,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(); @@ -2844,7 +2844,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 14d6f31..e6a4df2 100644 --- a/tests/e2e/firefox/run.js +++ b/tests/e2e/firefox/run.js @@ -640,6 +640,19 @@ step( await d.switchToWindow(popup); await d.waitVisible("#view-approve-sign"); + // The window is opened as wide as the page + // (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. + const sideways = await d.execute( + "return document.documentElement.scrollWidth -" + + " document.documentElement.clientWidth;", + ); + assert( + sideways <= 0, + "the sign prompt scrolls sideways by " + sideways + "px", + ); + const screen = await d.execute( `return { origin: document.getElementById("approve-sign-origin").textContent, diff --git a/tests/support/popupBoot.js b/tests/support/popupBoot.js index 308d0d4..93d0dcc 100644 --- a/tests/support/popupBoot.js +++ b/tests/support/popupBoot.js @@ -242,6 +242,11 @@ 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.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,6 +282,11 @@ 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 = []; globalThis.chrome = { @@ -287,6 +297,7 @@ async function bootPopup(stored, options) { onMessage: { addListener: () => {} }, ...(options && options.runtime), }, + windows: options && options.windows, }; globalThis.document = document; globalThis.window = {