From 717580d39053504123aa6e308b9388599d0abdb9 Mon Sep 17 00:00:00 2001 From: pproenca <8202400+pproenca@users.noreply.github.com> Date: Mon, 2 Feb 2026 12:59:49 +0000 Subject: [PATCH] fix: persist pending captures in session storage and refine permissions - Move pendingCaptureTabs from in-memory Map to session storage to survive service worker restarts - Make manifest permissions conditional: offscreen only for Chrome, tabs only for E2E builds - Disable test-activate page in non-E2E builds for security - Fix offscreen message listener to use sendResponse callback pattern - Add quota error handling with automatic cleanup retry in storage utils - Set allFrames: false explicitly on content script --- entrypoints/background.ts | 60 ++++++++++++++++++---------- entrypoints/content/index.ts | 1 + entrypoints/offscreen/main.ts | 7 +++- entrypoints/test-activate/main.ts | 11 +++++- tests/types.ts | 1 + utils/storage-items.ts | 8 ++++ utils/storage.ts | 21 +++++++++- wxt.config.ts | 66 ++++++++++++++++++------------- 8 files changed, 124 insertions(+), 51 deletions(-) diff --git a/entrypoints/background.ts b/entrypoints/background.ts index cbca5d9..cb1648f 100644 --- a/entrypoints/background.ts +++ b/entrypoints/background.ts @@ -12,29 +12,53 @@ import { isExtensionSender, getContentScriptFiles, } from '@/utils/background-helpers'; +import {pendingCaptureTabs} from '@/utils/storage-items'; import {backgroundMessenger, contentMessenger} from '@/utils/messaging'; export default defineBackground(() => { - const pendingCaptureTabs = new Map(); const pendingCaptureTtlMs = 120000; const contextMenuId = 'df-export-snapshot'; - function markPendingCapture(tabId: number): void { - pendingCaptureTabs.set(tabId, Date.now()); + function trimExpiredPending( + current: Record + ): Record { + const now = Date.now(); + let changed = false; + const next: Record = {...current}; + for (const [key, startedAt] of Object.entries(current)) { + if (!startedAt || now - startedAt > pendingCaptureTtlMs) { + delete next[key]; + changed = true; + } + } + return changed ? next : current; } - function hasPendingCapture(tabId: number): boolean { - const startedAt = pendingCaptureTabs.get(tabId); - if (!startedAt) return false; - if (Date.now() - startedAt > pendingCaptureTtlMs) { - pendingCaptureTabs.delete(tabId); - return false; + async function markPendingCapture(tabId: number): Promise { + const key = String(tabId); + const current = await pendingCaptureTabs.getValue(); + const trimmed = trimExpiredPending(current); + await pendingCaptureTabs.setValue({...trimmed, [key]: Date.now()}); + } + + async function hasPendingCapture(tabId: number): Promise { + const key = String(tabId); + const current = await pendingCaptureTabs.getValue(); + if (!current[key]) return false; + const trimmed = trimExpiredPending(current); + if (trimmed !== current) { + await pendingCaptureTabs.setValue(trimmed); } - return true; + return Boolean(trimmed[key]); } - function clearPendingCapture(tabId: number): void { - pendingCaptureTabs.delete(tabId); + async function clearPendingCapture(tabId: number): Promise { + const key = String(tabId); + const current = await pendingCaptureTabs.getValue(); + if (!current[key]) return; + const next = {...current}; + delete next[key]; + await pendingCaptureTabs.setValue(next); } async function injectContentScripts(tabId: number): Promise { @@ -122,10 +146,10 @@ export default defineBackground(() => { await showToolbar(tab.id); - if (hasPendingCapture(tab.id)) { + if (await hasPendingCapture(tab.id)) { const resumed = await sendResumeExportWithRetry(tab.id); if (resumed) { - clearPendingCapture(tab.id); + await clearPendingCapture(tab.id); } } } @@ -143,10 +167,6 @@ export default defineBackground(() => { await handleUserInvocation(activeTab); }); - browser.tabs.onRemoved.addListener(tabId => { - clearPendingCapture(tabId); - }); - backgroundMessenger.onMessage('captureScreenshot', async ({sender}) => { if (!isExtensionSender(sender)) { throw new Error('Invalid sender'); @@ -156,11 +176,11 @@ export default defineBackground(() => { const result = await captureVisibleTabScreenshot(windowId); if (result.error && sender.tab?.id) { if (isActiveTabPermissionError(result.error)) { - markPendingCapture(sender.tab.id); + await markPendingCapture(sender.tab.id); return {...result, errorCode: 'activeTab-required' as const}; } } else if (sender.tab?.id) { - clearPendingCapture(sender.tab.id); + await clearPendingCapture(sender.tab.id); } return result; } catch (error) { diff --git a/entrypoints/content/index.ts b/entrypoints/content/index.ts index 695c0e2..8177351 100644 --- a/entrypoints/content/index.ts +++ b/entrypoints/content/index.ts @@ -16,6 +16,7 @@ export default defineContentScript({ registration: 'runtime', world: 'ISOLATED', cssInjectionMode: 'ui', + allFrames: false, main(ctx) { const isEligibleDocument = diff --git a/entrypoints/offscreen/main.ts b/entrypoints/offscreen/main.ts index 294a78a..5d50bde 100644 --- a/entrypoints/offscreen/main.ts +++ b/entrypoints/offscreen/main.ts @@ -57,7 +57,7 @@ async function handleDownload( } } -browser.runtime.onMessage.addListener((message, sender) => { +browser.runtime.onMessage.addListener((message, sender, sendResponse) => { if (sender.id && sender.id !== browser.runtime.id) { return; } @@ -68,5 +68,8 @@ browser.runtime.onMessage.addListener((message, sender) => { ) { return; } - return handleDownload(message as OffscreenDownloadMessage); + handleDownload(message as OffscreenDownloadMessage) + .then(response => sendResponse(response)) + .catch(error => sendResponse({ok: false, error: String(error)})); + return true; }); diff --git a/entrypoints/test-activate/main.ts b/entrypoints/test-activate/main.ts index 1e0258e..ac20a09 100644 --- a/entrypoints/test-activate/main.ts +++ b/entrypoints/test-activate/main.ts @@ -5,6 +5,8 @@ interface TargetInfo { targetUrl: string; } +const isE2E = import.meta.env.VITE_DF_E2E === '1'; + function getTargetInfo(params: URLSearchParams): TargetInfo { const target = params.get('target') ?? ''; const targetUrl = target.trim(); @@ -112,4 +114,11 @@ async function activate(targetInfo: TargetInfo): Promise { const params = new URLSearchParams(window.location.search); const targetInfo = getTargetInfo(params); -void activate(targetInfo); +if (!isE2E) { + window.__dfActivateStatus = 'disabled'; + window.__dfActivateDebug = { + reason: 'test-activate is only available in E2E builds', + }; +} else { + void activate(targetInfo); +} diff --git a/tests/types.ts b/tests/types.ts index b55211c..9a67bb8 100644 --- a/tests/types.ts +++ b/tests/types.ts @@ -14,6 +14,7 @@ export interface DfActivateDebug { flagCheck?: unknown; toolbarShown?: boolean; error?: string; + reason?: string; } declare global { diff --git a/utils/storage-items.ts b/utils/storage-items.ts index c6194d6..831f0d1 100644 --- a/utils/storage-items.ts +++ b/utils/storage-items.ts @@ -25,3 +25,11 @@ export const toolbarPositions = storage.defineItem< fallback: {}, version: 1, }); + +export const pendingCaptureTabs = storage.defineItem>( + 'session:designer-feedback:pending-capture-tabs', + { + fallback: {}, + version: 1, + } +); diff --git a/utils/storage.ts b/utils/storage.ts index 6fe7d0a..59cb044 100644 --- a/utils/storage.ts +++ b/utils/storage.ts @@ -6,7 +6,11 @@ import { import {hashString} from '@/utils/hash'; import {storage} from 'wxt/utils/storage'; import {backgroundMessenger} from '@/utils/messaging'; -import {maybeRunCleanup, getRetentionCutoff} from '@/utils/storage-cleanup'; +import { + maybeRunCleanup, + getRetentionCutoff, + cleanupExpiredAnnotations, +} from '@/utils/storage-cleanup'; import {getWindow} from '@/utils/dom/guards'; export function getStorageKey(targetUrl?: string): string { @@ -62,6 +66,12 @@ async function getLocal(key: string, fallback: T): Promise { } } +function isQuotaError(error: unknown): boolean { + if (!error) return false; + const message = error instanceof Error ? error.message : String(error); + return message.toLowerCase().includes('quota'); +} + async function setLocal(values: Record): Promise { const entries = Object.entries(values).map(([key, value]) => ({ key: `local:${key}` as const, @@ -71,6 +81,15 @@ async function setLocal(values: Record): Promise { try { await storage.setItems(entries); } catch (error) { + if (isQuotaError(error)) { + try { + await cleanupExpiredAnnotations(getRetentionCutoff()); + await storage.setItems(entries); + return; + } catch (retryError) { + console.warn('Storage quota exceeded; retry failed:', retryError); + } + } console.warn('Storage access failed (set):', error); } } diff --git a/wxt.config.ts b/wxt.config.ts index 1daea8a..f75964f 100644 --- a/wxt.config.ts +++ b/wxt.config.ts @@ -1,45 +1,57 @@ import {defineConfig} from 'wxt'; +const isE2E = process.env.VITE_DF_E2E === '1'; + export default defineConfig({ modules: ['@wxt-dev/module-react', '@wxt-dev/auto-icons'], srcDir: '.', outDir: '.output', - manifest: { - name: 'Designer Feedback', - description: - 'Annotate any webpage and share visual feedback with developers', - permissions: [ + manifest: ({browser}) => { + const permissions = [ 'storage', - 'tabs', 'downloads', - 'offscreen', 'activeTab', 'scripting', 'contextMenus', - ], - commands: { - 'activate-toolbar': { - suggested_key: { - default: 'Ctrl+Shift+S', - mac: 'Command+Shift+S', + ]; + + if (browser !== 'firefox') { + permissions.push('offscreen'); + } + + if (isE2E) { + permissions.push('tabs'); + } + + return { + name: 'Designer Feedback', + description: + 'Annotate any webpage and share visual feedback with developers', + permissions, + commands: { + 'activate-toolbar': { + suggested_key: { + default: 'Ctrl+Shift+S', + mac: 'Command+Shift+S', + }, + description: 'Open Designer Feedback on the current page', }, - description: 'Open Designer Feedback on the current page', }, - }, - content_security_policy: { - extension_pages: "script-src 'self'; object-src 'none';", - }, - action: { - default_title: 'Click to activate Designer Feedback', - }, - // CSS must be accessible from all URLs for runtime content script injection - web_accessible_resources: [ - { - resources: ['content-scripts/content.css'], - matches: ['http://*/*', 'https://*/*'], + content_security_policy: { + extension_pages: "script-src 'self'; object-src 'none';", + }, + action: { + default_title: 'Click to activate Designer Feedback', }, - ], + // CSS must be accessible from all URLs for runtime content script injection + web_accessible_resources: [ + { + resources: ['content-scripts/content.css'], + matches: ['http://*/*', 'https://*/*'], + }, + ], + }; }, // Maintain old output directory structure for E2E tests