fix: recognise the popup when a browser opens it in a tab

src/background/index.ts

@@ -106,15 +106,33 @@ function handle(message: Request) {
   });
 }
 
+/** Whether a message came from one of the extension's own pages, which are the
+ *  privileged callers. Desktop Chrome floats the popup in a panel of its own,
+ *  giving it no tab and exactly the popup's URL. Chromium ports with nowhere to
+ *  float it render it inside a tab instead — Kiwi on Android does — so the
+ *  absence of a tab does not identify it and the URL may carry a query string.
+ *  Coming from the extension's own URL is what actually separates the two: a
+ *  content script always reports the page it was injected into, and nothing
+ *  here is web-accessible, so no page can carry an extension URL of ours. */
+function fromExtensionPage(sender: chrome.runtime.MessageSender) {
+  // No URL and no tab is an extension context too; a content script has both.
+  if (!sender.url) return !sender.tab;
+  // A prefix, deliberately, not `URL.origin`: `chrome-extension:` is not a
+  // special scheme, so the standard parser gives every such URL the origin
+  // "null" and any extension's page would compare equal to ours. The trailing
+  // slash `getURL('')` leaves keeps a longer ID from matching as a prefix.
+  return sender.url.startsWith(chrome.runtime.getURL(''));
+}
+
 chrome.runtime.onMessage.addListener((raw: unknown, sender, respond) => {
   const program = Effect.gen(function* () {
     if (sender.id !== chrome.runtime.id)
       return yield* new OperationError({ message: 'Untrusted sender.' });
     const message = yield* Schema.decodeUnknown(Request)(raw);
-    const fromPopup = !sender.tab && sender.url === chrome.runtime.getURL('popup.html');
+    const privileged = fromExtensionPage(sender);
     const fromSite =
       sender.tab && /^https:\/\/(?:x\.com|twitter\.com|www\.youtube\.com)\//.test(sender.url ?? '');
-    if (!fromPopup && !(fromSite && contentRequests.has(message.type))) {
+    if (!privileged && !(fromSite && contentRequests.has(message.type))) {
       return yield* new OperationError({
         message: 'This operation is not available to content scripts.',
       });
@@ -137,7 +155,9 @@ chrome.runtime.onMessage.addListener((raw: unknown, sender, respond) => {
   return true;
 });
 
-chrome.commands.onCommand.addListener((command) => {
+// Optional for the same reason the popup's shortcut list is: a browser with no
+// keyboard shortcuts need not offer the API, and this runs at module scope.
+chrome.commands?.onCommand.addListener((command) => {
   const operation = Effect.gen(function* () {
     if (command === 'toggle-filtering') {
       const settings = yield* getSettings;

src/popup/App.tsx

@@ -18,6 +18,31 @@ const same = (a: unknown, b: unknown) => JSON.stringify(a) === JSON.stringify(b)
 const noop = () => {};
 /** `https://x.com/*` reads as x.com to the person being asked about it. */
 const host = (origin: string) => origin.replace(/^https:\/\//, '').replace(/\/\*$/, '');
+const filtered = /^https:\/\/(?:x\.com|twitter\.com|www\.youtube\.com)\//;
+
+/** The page the reader was looking at when they opened Sharp. Desktop Chrome
+ *  floats the popup above the page, so that is simply the active tab —
+ *  whatever it is. An active tab on an unrelated site is the honest answer
+ *  that the reader is not on a site Sharp filters, and is returned as such:
+ *  preferring a background X tab over it would put the X panel, and the thread
+ *  toggle with it, in front of someone looking at something else entirely.
+ *  The search below is not a better guess, it is the only guess available when
+ *  there is no page to read: where the popup has nowhere to float it opens in
+ *  a tab of its own — Kiwi on Android — and the active tab is the popup. */
+export async function readerTab() {
+  // A prefix, not `URL.origin`, which is "null" for every `chrome-extension:`
+  // URL under the standard parser and so matches nothing usefully.
+  const own = chrome.runtime.getURL('');
+  const active = await chrome.tabs.query({ active: true, currentWindow: true });
+  const page = active.find((tab) => tab.url && !tab.url.startsWith(own));
+  if (page?.url) return page.url;
+  // Only reached when the popup itself was the active tab. Restricted to
+  // filtered sites, so the guess is between pages the popup has a section for.
+  const all = await chrome.tabs.query({});
+  return all
+    .filter((tab) => tab.url && filtered.test(tab.url))
+    .sort((a, b) => (b.lastAccessed ?? 0) - (a.lastAccessed ?? 0))[0]?.url;
+}
 
 function normalize(settings: Settings): Settings {
   const lines = (values: readonly string[], author = false) => [
@@ -86,16 +111,16 @@ export function App() {
         tone: 'bad',
       }),
     );
-    void chrome.tabs.query({ active: true, currentWindow: true }).then(([tab]) => {
-      if (!tab?.url) return;
-      if (/^https:\/\/(?:x|twitter)\.com\//.test(tab.url)) {
+    void readerTab().then((url) => {
+      if (!url) return;
+      if (/^https:\/\/(?:x|twitter)\.com\//.test(url)) {
         setSite('x');
-        setThread(threadId(new URL(tab.url).pathname));
-      } else if (/^https:\/\/www\.youtube\.com\//.test(tab.url)) {
+        setThread(threadId(new URL(url).pathname));
+      } else if (/^https:\/\/www\.youtube\.com\//.test(url)) {
         setSite('youtube');
         setSection('youtube');
       }
-    });
+    }, noop);
     const onChange = (_changes: Record<string, chrome.storage.StorageChange>, area: string) => {
       if (area === 'local') void refresh().catch(() => {});
     };

src/popup/XPanel.tsx

@@ -54,7 +54,9 @@ const notation = (shortcut: string) =>
 function useShortcuts() {
   const [commands, setCommands] = useState<chrome.commands.Command[]>([]);
   useEffect(() => {
-    void chrome.commands.getAll().then(setCommands, () => {});
+    // Absent where there are no keyboard shortcuts to bind, as on Android.
+    // Reading through it unguarded throws before the rejection handler exists.
+    void chrome.commands?.getAll().then(setCommands, () => {});
   }, []);
   return commands.filter((command) => command.description);
 }

tests/messages.test.ts

@@ -124,6 +124,34 @@ it('denies privileged content-script requests but allows the popup', async () =>
       url: 'chrome-extension://extension-id/popup.html',
     }),
   ).toMatchObject({ ok: true, result: { apiKeys: {} } });
+  // Kiwi on Android has nowhere to float a popup, so it opens the popup in a
+  // tab. The tab is not what makes a caller a content script; the origin is.
+  expect(
+    await send('GET_SETTINGS', {
+      id: 'extension-id',
+      url: 'chrome-extension://extension-id/popup.html',
+      tab: content.tab,
+    }),
+  ).toMatchObject({ ok: true, result: { apiKeys: {} } });
+  // The same page reached with a query string is still the same page.
+  expect(
+    await send('GET_SETTINGS', {
+      id: 'extension-id',
+      url: 'chrome-extension://extension-id/popup.html?reopened=1',
+      tab: content.tab,
+    }),
+  ).toMatchObject({ ok: true, result: { apiKeys: {} } });
+  // Another extension's page shares the scheme but not the origin.
+  expect(
+    await send('GET_SETTINGS', {
+      id: 'extension-id',
+      url: 'chrome-extension://other-extension/popup.html',
+    }),
+  ).toMatchObject({ ok: false });
+  // A filtered page is still held to the content-script request set, tab or no.
+  expect(
+    await send('GET_SETTINGS', { id: 'extension-id', url: 'https://x.com/home' }),
+  ).toMatchObject({ ok: false });
 });
 
 it('fills settings from an older worker with defaults instead of failing', async () => {
@@ -153,3 +181,37 @@ it('reports an orphaned content script however the browser signals it', () => {
   });
   expect(orphaned()).toBe(true);
 });
+
+it('loads the worker on a browser with no keyboard shortcuts', async () => {
+  vi.resetModules();
+  const { chrome } = mockChrome({ settings: defaults });
+  let listener!: (
+    message: unknown,
+    sender: chrome.runtime.MessageSender,
+    respond: (response: unknown) => void,
+  ) => boolean;
+  // Android has no shortcuts to bind, so chrome.commands need not exist. The
+  // worker registers its message listener before touching it, but a throw at
+  // module scope still leaves the rest of the file unevaluated.
+  vi.stubGlobal('chrome', {
+    ...chrome,
+    runtime: {
+      ...chrome.runtime,
+      getURL: (path: string) => `chrome-extension://extension-id/${path}`,
+      onMessage: {
+        addListener: (handler: typeof listener) => {
+          listener = handler;
+        },
+      },
+    },
+  });
+  await expect(import('../src/background/index')).resolves.toBeDefined();
+  const reply = await new Promise((resolve) =>
+    listener(
+      { type: 'GET_SETTINGS' },
+      { id: 'extension-id', url: 'chrome-extension://extension-id/popup.html' },
+      resolve,
+    ),
+  );
+  expect(reply).toMatchObject({ ok: true });
+});

tests/reader-tab.test.ts

@@ -0,0 +1,71 @@
+// @vitest-environment jsdom
+import { afterEach, expect, it, vi } from 'vitest';
+import { readerTab } from '../src/popup/App';
+
+const POPUP = 'chrome-extension://extension-id/popup.html';
+
+/** `tabs.query` filtered the way Chrome filters it: `active`/`currentWindow`
+ *  narrow the set, and an empty query matches every tab. */
+function mockTabs(tabs: { url?: string; active?: boolean; lastAccessed?: number }[]) {
+  vi.stubGlobal('chrome', {
+    runtime: { getURL: (path: string) => `chrome-extension://extension-id/${path}` },
+    tabs: {
+      query: vi.fn(async (q: { active?: boolean }) =>
+        q.active ? tabs.filter((tab) => tab.active) : tabs,
+      ),
+    },
+  });
+}
+
+afterEach(() => vi.unstubAllGlobals());
+
+it('reads the active tab, which is the page a floating popup sits over', async () => {
+  mockTabs([{ url: 'https://x.com/home', active: true }]);
+  await expect(readerTab()).resolves.toBe('https://x.com/home');
+});
+
+it('reports an unrelated active tab rather than reaching for a background one', async () => {
+  // The reader is looking at Hacker News. Answering "x.com" because a tab is
+  // open there would show the X panel, and its thread toggle, over a thread
+  // they cannot see. Not selecting a site is the correct outcome here.
+  mockTabs([
+    { url: 'https://news.ycombinator.com/', active: true },
+    { url: 'https://x.com/home', lastAccessed: 10 },
+  ]);
+  await expect(readerTab()).resolves.toBe('https://news.ycombinator.com/');
+});
+
+it('looks past the popup when the popup is itself the active tab', async () => {
+  // Kiwi on Android: nowhere to float a panel, so popup.html occupies a tab.
+  mockTabs([
+    { url: POPUP, active: true },
+    { url: 'https://x.com/home', lastAccessed: 10 },
+  ]);
+  await expect(readerTab()).resolves.toBe('https://x.com/home');
+});
+
+it('prefers the most recently touched supported tab, and ignores the rest', async () => {
+  mockTabs([
+    { url: POPUP, active: true },
+    { url: 'https://x.com/home', lastAccessed: 10 },
+    { url: 'https://news.ycombinator.com/', lastAccessed: 99 },
+    { url: 'https://www.youtube.com/watch?v=1', lastAccessed: 42 },
+  ]);
+  await expect(readerTab()).resolves.toBe('https://www.youtube.com/watch?v=1');
+});
+
+it('answers with nothing when the popup is alone and no supported tab is open', async () => {
+  mockTabs([
+    { url: POPUP, active: true },
+    { url: 'https://news.ycombinator.com/', lastAccessed: 99 },
+  ]);
+  await expect(readerTab()).resolves.toBeUndefined();
+});
+
+it('is not fooled by an extension ID that ours is a prefix of', async () => {
+  mockTabs([
+    { url: 'chrome-extension://extension-id-other/popup.html', active: true },
+    { url: 'https://x.com/home', lastAccessed: 10 },
+  ]);
+  await expect(readerTab()).resolves.toBe('chrome-extension://extension-id-other/popup.html');
+});