diff --git a/packages/extension-shared/src/lib/tabs-permission.ts b/packages/extension-shared/src/lib/tabs-permission.ts new file mode 100644 index 0000000..6afe193 --- /dev/null +++ b/packages/extension-shared/src/lib/tabs-permission.ts @@ -0,0 +1,40 @@ +import browser from "webextension-polyfill"; + +export type TabsPermissionResult = + | { granted: true } + | { granted: false; error: string | null }; + +/** + * Request the optional "tabs" permission. + * + * MUST be invoked synchronously inside a user-input handler (e.g. as the + * first statement of a click listener, before any `await`): Firefox voids + * the input gesture across `await`s — even awaiting permissions.contains() + * — and rejects permissions.request() outside a gesture (issue #68). + * Chrome is lenient, which is how the bug shipped. + * + * There is deliberately no contains() pre-check: requesting an + * already-granted permission resolves true without showing a prompt in + * both browsers. + * + * Never rejects — a request() rejection is returned as + * `{ granted: false, error }` so callers can surface it instead of the + * handler dying silently. + */ +export function requestTabsPermission(): Promise { + let request: Promise; + try { + request = browser.permissions.request({ permissions: ["tabs"] }); + } catch (err) { + return Promise.resolve({ granted: false, error: errMessage(err) }); + } + return request.then( + (granted): TabsPermissionResult => + granted ? { granted: true } : { granted: false, error: null }, + (err): TabsPermissionResult => ({ granted: false, error: errMessage(err) }), + ); +} + +function errMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} diff --git a/packages/extension-shared/src/popup.ts b/packages/extension-shared/src/popup.ts index e48db47..83f05ad 100644 --- a/packages/extension-shared/src/popup.ts +++ b/packages/extension-shared/src/popup.ts @@ -9,6 +9,7 @@ import { type SaveAllTabsResult, } from "./lib/save-flow.js"; import { applySaveResult, applySaveAllResult } from "./lib/save-result-view.js"; +import { requestTabsPermission } from "./lib/tabs-permission.js"; import type { LastErrorRecord } from "./lib/background-core.js"; const root = document.getElementById("root"); @@ -119,17 +120,20 @@ async function render(): Promise { saveAllBtn.addEventListener("click", async () => { // Reading every tab's url/title needs the "tabs" permission, which is - // *optional* (kept out of the install prompt). Request it on this user - // gesture the first time. Requesting from a popup can close the popup when - // the prompt appears; if so the grant still sticks, and the next click sees - // it already granted and proceeds. - if (!(await browser.permissions.contains({ permissions: ["tabs"] }))) { - const granted = await browser.permissions.request({ permissions: ["tabs"] }); - if (!granted) { - status.className = "err"; - status.textContent = "Allow tab access to save all tabs."; - return; - } + // *optional* (kept out of the install prompt). requestTabsPermission() + // must stay the FIRST statement here — Firefox voids the user-input + // gesture across `await`s and rejects permissions.request() outside a + // gesture (issue #68). Requesting from a popup can close the popup when + // the prompt appears; if so the grant still sticks, and the next click + // sees it already granted (request resolves true, no prompt) and proceeds. + const perm = await requestTabsPermission(); + if (!perm.granted) { + status.className = "err"; + status.textContent = + perm.error != null + ? `Tab access failed: ${perm.error}` + : "Allow tab access to save all tabs."; + return; } saveAllBtn.disabled = true; saveAllBtn.textContent = "saving all…"; diff --git a/packages/extension-shared/test/setup.ts b/packages/extension-shared/test/setup.ts index 8d34acd..df8af13 100644 --- a/packages/extension-shared/test/setup.ts +++ b/packages/extension-shared/test/setup.ts @@ -60,6 +60,9 @@ const chromeStub = { tabs: { query: vi.fn(), }, + permissions: { + request: vi.fn(async () => true), + }, }; vi.stubGlobal("chrome", chromeStub); diff --git a/packages/extension-shared/test/tabs-permission.test.ts b/packages/extension-shared/test/tabs-permission.test.ts new file mode 100644 index 0000000..65c1ea3 --- /dev/null +++ b/packages/extension-shared/test/tabs-permission.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from "vitest"; +import { chromeStub } from "./setup.js"; +import { requestTabsPermission } from "../src/lib/tabs-permission.js"; + +describe("requestTabsPermission", () => { + it("issues permissions.request synchronously within the caller's stack (gesture preservation, #68)", () => { + chromeStub.permissions.request.mockReturnValue(Promise.resolve(true)); + + void requestTabsPermission(); + + // Firefox rejects permissions.request() once the user-input gesture is + // lost, and any await before the call loses it — so the request must + // already have been issued by the time requestTabsPermission returns, + // not on a later microtask. + expect(chromeStub.permissions.request).toHaveBeenCalledTimes(1); + expect(chromeStub.permissions.request).toHaveBeenCalledWith({ permissions: ["tabs"] }); + }); + + it("resolves granted:true when the user grants", async () => { + chromeStub.permissions.request.mockResolvedValue(true); + + await expect(requestTabsPermission()).resolves.toEqual({ granted: true }); + }); + + it("resolves granted:false with no error when the user declines", async () => { + chromeStub.permissions.request.mockResolvedValue(false); + + await expect(requestTabsPermission()).resolves.toEqual({ granted: false, error: null }); + }); + + it("converts a request() rejection into a result instead of rejecting (silent-failure guard, #68)", async () => { + chromeStub.permissions.request.mockRejectedValue( + new Error("permissions.request may only be called from a user input handler"), + ); + + await expect(requestTabsPermission()).resolves.toEqual({ + granted: false, + error: "permissions.request may only be called from a user input handler", + }); + }); + + it("converts a synchronous request() throw into a result instead of throwing", async () => { + chromeStub.permissions.request.mockImplementation(() => { + throw new Error("boom"); + }); + + await expect(requestTabsPermission()).resolves.toEqual({ granted: false, error: "boom" }); + }); +});