fix: approval windows open as wide as the page (closes #518) #555
@@ -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
|
||||
@@ -551,7 +554,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
|
||||
@@ -908,8 +912,13 @@ 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.
|
||||
|
||||
#### No Layout Shift
|
||||
|
||||
|
||||
@@ -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") {
|
||||
|
||||
+28
-2
@@ -21,6 +21,11 @@ const {
|
||||
goBack,
|
||||
} = require("./views/helpers");
|
||||
const { applyTheme } = require("./theme");
|
||||
const {
|
||||
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 +168,27 @@ 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 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 sideways =
|
||||
document.documentElement.scrollWidth -
|
||||
document.documentElement.clientWidth;
|
||||
if (sideways <= 0) return;
|
||||
const win = await windowsGetCurrent();
|
||||
if (win.type !== "popup") return;
|
||||
const zoom = await tabsGetZoom();
|
||||
await windowsUpdate(win.id, {
|
||||
width: win.width + Math.ceil(sideways * zoom),
|
||||
});
|
||||
}
|
||||
|
||||
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 +265,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;
|
||||
}
|
||||
|
||||
|
||||
@@ -235,6 +235,14 @@ function tabsSendMessage(tabId, message) {
|
||||
return invoke(tabsApi(), "sendMessage", tabId, message);
|
||||
}
|
||||
|
||||
/**
|
||||
* @returns {Promise<number>} the zoom of the active tab in the calling page's
|
||||
* window: 1.5 at 150%.
|
||||
*/
|
||||
function tabsGetZoom() {
|
||||
return invoke(tabsApi(), "getZoom");
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {Object} createData
|
||||
* @returns {Promise<Object>} the created window.
|
||||
@@ -250,6 +258,22 @@ function windowsGetLastFocused() {
|
||||
return invoke(windowsApi(), "getLastFocused");
|
||||
}
|
||||
|
||||
/**
|
||||
* @returns {Promise<Object>} the window the calling page is in.
|
||||
*/
|
||||
function windowsGetCurrent() {
|
||||
return invoke(windowsApi(), "getCurrent");
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {number} windowId
|
||||
* @param {Object} updateInfo
|
||||
* @returns {Promise<Object>} the updated window.
|
||||
*/
|
||||
function windowsUpdate(windowId, updateInfo) {
|
||||
return invoke(windowsApi(), "update", windowId, updateInfo);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {number} windowId
|
||||
* @returns {Promise<void>}
|
||||
@@ -271,10 +295,13 @@ module.exports = {
|
||||
storageRemove,
|
||||
storageSet,
|
||||
tabsApi,
|
||||
tabsGetZoom,
|
||||
tabsQuery,
|
||||
tabsSendMessage,
|
||||
windowsApi,
|
||||
windowsCreate,
|
||||
windowsGetCurrent,
|
||||
windowsGetLastFocused,
|
||||
windowsRemove,
|
||||
windowsUpdate,
|
||||
};
|
||||
|
||||
@@ -45,3 +45,77 @@ 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 at 100% zoom, 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({});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// An approval window at 150% zoom, whose background holds its answer until
|
||||
// the test gives it.
|
||||
async function bootZoomedApprovalWindow(windows) {
|
||||
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: { getZoom: (done) => done(1.5) },
|
||||
// At 150% a window 396 screen pixels wide shows 264 page pixels.
|
||||
documentElement: { scrollWidth: 396, clientWidth: 264 },
|
||||
});
|
||||
return { env, answer };
|
||||
}
|
||||
|
||||
test("at 150% zoom the approval window widens by what its screen lacks, scrollbar included", async () => {
|
||||
const windows = windowsStub("popup");
|
||||
const { env, answer } = await bootZoomedApprovalWindow(windows);
|
||||
|
||||
// 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.
|
||||
expect(env.visibleViews()).toEqual(["approve-network"]);
|
||||
expect(windows.updates).toEqual([{ id: 7, info: { width: 609 } }]);
|
||||
expect(env.pageErrors).toEqual([]);
|
||||
});
|
||||
|
||||
test("a connection prompt in the toolbar popup leaves the browser window alone", async () => {
|
||||
const windows = windowsStub("normal");
|
||||
const { env, answer } = await bootZoomedApprovalWindow(windows);
|
||||
|
||||
answer({ origin: "https://dapp.example", isPhishingDomain: false });
|
||||
await settle();
|
||||
|
||||
expect(env.visibleViews()).toEqual(["approve-site"]);
|
||||
expect(windows.updates).toEqual([]);
|
||||
expect(env.pageErrors).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -2854,7 +2854,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 () => {
|
||||
@@ -2871,7 +2871,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();
|
||||
@@ -2893,7 +2893,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();
|
||||
|
||||
|
||||
@@ -640,6 +640,17 @@ step(
|
||||
await d.switchToWindow(popup);
|
||||
await d.waitVisible("#view-approve-sign");
|
||||
|
||||
// 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.
|
||||
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,
|
||||
|
||||
@@ -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
|
||||
* 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<object>} handles onto the booted page.
|
||||
*/
|
||||
async function bootPopup(stored, options) {
|
||||
@@ -277,6 +284,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 +299,8 @@ async function bootPopup(stored, options) {
|
||||
onMessage: { addListener: () => {} },
|
||||
...(options && options.runtime),
|
||||
},
|
||||
windows: options && options.windows,
|
||||
tabs: options && options.tabs,
|
||||
};
|
||||
globalThis.document = document;
|
||||
globalThis.window = {
|
||||
|
||||
Reference in New Issue
Block a user