From afaa0e5fb601b3739e8bdd9affb25344fa703039 Mon Sep 17 00:00:00 2001 From: tshmieldev Date: Tue, 15 Sep 2026 11:01:59 +0200 Subject: [PATCH 1/3] fix: recognise the popup when a browser opens it in a tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kiwi on Android is Chromium with nowhere to float a browser-action panel, so it renders popup.html inside an ordinary tab. Two things assumed the desktop shape and broke there. The worker identified the popup as "no tab, and exactly the popup URL", so on Kiwi every privileged request came back "This operation is not available to content scripts" — settings would not load or save. A tab is not what makes a caller a content script; coming from a page that is not the extension's own is. The check now tests that, which also tolerates a query string on the URL. The popup read the site under it from the active tab in the current window, which on Kiwi is the popup's own tab, so neither the X nor the YouTube section would appear. It now skips the extension's own pages and, when that leaves nothing, falls back to the most recently touched tab on a site Sharp filters. Both comparisons are prefix tests against getURL(''), not URL.origin. A test written against origin passed another extension's page: chrome-extension: is not a special scheme, so the standard parser gives every such URL the origin "null" and they all compare equal. Only sender.id stood between that and a privileged call. --- src/background/index.ts | 22 ++++++++++++++++++++-- src/popup/App.tsx | 33 +++++++++++++++++++++++++++------ tests/messages.test.ts | 28 ++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/background/index.ts b/src/background/index.ts index 347af31..93e0d6a 100644 --- a/src/background/index.ts +++ b/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.', }); diff --git a/src/popup/App.tsx b/src/popup/App.tsx index da10048..713cfc1 100644 --- a/src/popup/App.tsx +++ b/src/popup/App.tsx @@ -18,6 +18,27 @@ 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. Where + * there is nowhere to float it the popup opens in a tab of its own — Kiwi on + * Android — and the active tab is the popup itself, so the page behind it is + * the most recently touched one Sharp has anything to say about. */ +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; + // Restricted to filtered sites, so the guess can only ever be between pages + // the popup has a section for, never a wrong claim about an unrelated tab. + 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 +107,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, area: string) => { if (area === 'local') void refresh().catch(() => {}); }; diff --git a/tests/messages.test.ts b/tests/messages.test.ts index 0412f44..cc6ead9 100644 --- a/tests/messages.test.ts +++ b/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 () => { From 04e3ea777eb2d5b874f7cf62f794da73dceb65c8 Mon Sep 17 00:00:00 2001 From: tshmieldev Date: Tue, 15 Sep 2026 11:07:03 +0200 Subject: [PATCH 2/3] fix: treat chrome.commands as optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A browser with no keyboard shortcuts to bind need not offer the API, which is the ordinary case on Android. Both uses read through it unguarded. The popup called chrome.commands.getAll() inside an effect, where an absent API throws synchronously, before the rejection handler it was given could apply — taking the X panel down with it. The worker registered its onCommand listener at module scope, after the message listener, so messages survived but the rest of the module did not evaluate. The test covers the worker: it fails with the listener registration unguarded. --- src/background/index.ts | 4 +++- src/popup/XPanel.tsx | 4 +++- tests/messages.test.ts | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/background/index.ts b/src/background/index.ts index 93e0d6a..cfaef3f 100644 --- a/src/background/index.ts +++ b/src/background/index.ts @@ -155,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; diff --git a/src/popup/XPanel.tsx b/src/popup/XPanel.tsx index a99ae12..cd9a432 100644 --- a/src/popup/XPanel.tsx +++ b/src/popup/XPanel.tsx @@ -54,7 +54,9 @@ const notation = (shortcut: string) => function useShortcuts() { const [commands, setCommands] = useState([]); 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); } diff --git a/tests/messages.test.ts b/tests/messages.test.ts index cc6ead9..05485a8 100644 --- a/tests/messages.test.ts +++ b/tests/messages.test.ts @@ -181,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 }); +}); From 71657d68a363956a9f234e9a9d845c786cca0f2c Mon Sep 17 00:00:00 2001 From: tshmieldev Date: Tue, 15 Sep 2026 17:16:43 +0200 Subject: [PATCH 3/3] test: pin which tab the popup reads the current site from readerTab returns the active tab whatever site it is on, and only searches for a recently touched supported tab when the active tab was the popup itself. That ordering is deliberate and easy to mistake for a missing filter, so pin it. A floating popup sits over the page the reader is looking at, so an active tab on an unrelated site is the honest answer that they are not on a site Sharp filters. Preferring a background X tab over it would put the X panel, and the thread toggle with it, in front of someone reading something else. The search is not a better guess; it is the only guess available on a browser that gives the popup a tab of its own and so leaves no page to read. Filtering the active tab by site breaks two of these, including the prefix case that covers an extension ID ours is a prefix of. --- src/popup/App.tsx | 18 ++++++---- tests/reader-tab.test.ts | 71 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 tests/reader-tab.test.ts diff --git a/src/popup/App.tsx b/src/popup/App.tsx index 713cfc1..d27ea74 100644 --- a/src/popup/App.tsx +++ b/src/popup/App.tsx @@ -21,19 +21,23 @@ 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. Where - * there is nowhere to float it the popup opens in a tab of its own — Kiwi on - * Android — and the active tab is the popup itself, so the page behind it is - * the most recently touched one Sharp has anything to say about. */ -async function readerTab() { + * 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; - // Restricted to filtered sites, so the guess can only ever be between pages - // the popup has a section for, never a wrong claim about an unrelated tab. + // 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)) diff --git a/tests/reader-tab.test.ts b/tests/reader-tab.test.ts new file mode 100644 index 0000000..e32cad4 --- /dev/null +++ b/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'); +});