Merge pull request #11 from tshmieldev/tshmieldev/mobile-save

Saving a key on a phone failed with "no active window"

Author: tshmieldev

src/popup/App.tsx

@@ -10,6 +10,7 @@ import {
   type SettingsPatch,
 } from '../common/settings';
 import { extensionsPage } from '../common/build';
+import { errorMessage } from '../common/errors';
 import { GeneralPanel, type GeneralTab } from './GeneralPanel';
 import { ListSheet, type ListKey } from './ListSheet';
 import { ModelBrowser } from './ModelBrowser';
@@ -50,6 +51,33 @@ export async function readerTab() {
     .sort((a, b) => (b.lastAccessed ?? 0) - (a.lastAccessed ?? 0))[0]?.url;
 }
 
+/** Makes sure the browser lets Sharp reach these origins, asking only for the
+ *  ones not yet granted. The prompt needs a browser window, and a popup opened
+ *  as a tab (Kiwi, and Firefox on a phone) has none: asking for an origin the
+ *  manifest already covers would fail there with "no active window" and block
+ *  the save for nothing. Must be called from a user gesture. */
+export async function ensureOrigins(origins: readonly string[]) {
+  const missing = (
+    await Promise.all(
+      origins.map(async (origin) =>
+        (await chrome.permissions.contains({ origins: [origin] })) ? null : origin,
+      ),
+    )
+  ).filter((origin): origin is string => origin !== null);
+  if (!missing.length) return;
+  const granted = await chrome.permissions.request({ origins: missing }).catch((error: unknown) => {
+    // The browser has nowhere to draw the prompt. Say what to do, not what
+    // went wrong inside.
+    if (/window/i.test(errorMessage(error))) {
+      throw new Error(
+        `This browser cannot ask for permission to reach ${missing.map(host).join(', ')} from here. OpenRouter needs no extra permission, so it works on phones; the other providers need a desktop browser.`,
+      );
+    }
+    throw error;
+  });
+  if (!granted) throw new Error('Permission to reach the provider was not granted.');
+}
+
 function normalize(settings: Settings): Settings {
   const lines = (values: readonly string[], author = false) => [
     ...new Set(
@@ -236,15 +264,9 @@ export function App() {
           throw new Error('Use an HTTPS base URL without credentials, query or fragment.');
         }
       }
-      // Every origin these settings will call: the chat endpoint, or the
-      // classifier's provider and whoever describes its images. Already-granted
-      // origins resolve without a prompt. Called from the submit gesture, before
-      // any await (required by Chrome).
-      const wanted = apiOrigins(next);
-      if (wanted.length) {
-        const granted = await chrome.permissions.request({ origins: [...wanted] });
-        if (!granted) throw new Error('Permission to reach the provider was not granted.');
-      }
+      // Every origin these settings will call. Called from the submit gesture,
+      // before any other await (required by Chrome).
+      await ensureOrigins(apiOrigins(next));
       // Only changed fields are sent; unrelated updates from a tab aren't overwritten.
       const patch: SettingsPatch = Object.fromEntries(
         Object.entries(next).filter(([key, value]) => !same(value, saved[key as keyof Settings])),

tests/permissions.test.ts

@@ -0,0 +1,46 @@
+// @vitest-environment jsdom
+import { afterEach, expect, it, vi } from 'vitest';
+import { ensureOrigins } from '../src/popup/App';
+
+function mockPermissions(granted: string[], request?: () => Promise<boolean>) {
+  const requested: string[][] = [];
+  vi.stubGlobal('chrome', {
+    permissions: {
+      contains: vi.fn(async ({ origins }: { origins: string[] }) =>
+        origins.every((origin) => granted.includes(origin)),
+      ),
+      request: vi.fn(async ({ origins }: { origins: string[] }) => {
+        requested.push(origins);
+        return request ? request() : true;
+      }),
+    },
+  });
+  return requested;
+}
+afterEach(() => vi.unstubAllGlobals());
+
+it('never shows a prompt for origins the browser already allows', async () => {
+  // Kiwi opens the popup as a tab with no window to draw a prompt in, so a
+  // request for OpenRouter, granted at install, would fail and block the save.
+  const requested = mockPermissions(['https://openrouter.ai/*']);
+  await expect(ensureOrigins(['https://openrouter.ai/*'])).resolves.toBeUndefined();
+  expect(requested).toEqual([]);
+});
+
+it('asks only for what is missing, and fails the save if that is refused', async () => {
+  let requested = mockPermissions(['https://openrouter.ai/*']);
+  await ensureOrigins(['https://openrouter.ai/*', 'https://ai-gateway.vercel.sh/*']);
+  expect(requested).toEqual([['https://ai-gateway.vercel.sh/*']]);
+
+  requested = mockPermissions([], async () => false);
+  await expect(ensureOrigins(['https://api.typesafe.ai/*'])).rejects.toThrow('not granted');
+});
+
+it('explains a prompt the browser cannot show, naming the host', async () => {
+  mockPermissions([], async () => {
+    throw new Error('No active window.');
+  });
+  await expect(ensureOrigins(['https://ai-gateway.vercel.sh/*'])).rejects.toThrow(
+    /cannot ask for permission to reach ai-gateway\.vercel\.sh/,
+  );
+});