diff --git a/src/background/index.ts b/src/background/index.ts index 347af31..cfaef3f 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.', }); @@ -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; diff --git a/src/popup/App.tsx b/src/popup/App.tsx index da10048..d27ea74 100644 --- a/src/popup/App.tsx +++ b/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, area: string) => { if (area === 'local') void refresh().catch(() => {}); }; 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 0412f44..05485a8 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 () => { @@ -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 }); +}); 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'); +});