Firefox port: dual-target build, event-page background, OAuth via Worker redirect - #114
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wires buildManifest into webpack behind --env target=firefox flag; outputs to build-firefox/ with firefox-specific manifest (scripts-based background, no oauth2/key, browser_specific_settings). Adds dev:firefox and build:firefox scripts. Adds build-firefox/ to .gitignore and eslint config ignores. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wraps the importScripts block in a typeof guard so it runs only in Chrome's service-worker scope, not in Firefox's event-page scope where scripts are pre-loaded by the manifest. Normalizes the unguarded chrome.runtime.getManifest() call to browser.runtime.getManifest(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oauth2 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
browser.system.display is Chrome-only; Firefox lacks the entire namespace and all four unguarded calls would throw, aborting collection-open flows. This helper guards the call and falls back to a single pseudo-display built from window.screen, so multi-monitor position restore degrades to primary- screen clamping on Firefox while Chrome behavior remains unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
displayInfo.js was reading the bare global browser instead of importing
it like every other app/utils module. This fragile approach relied on
side-effects from the polyfill being loaded elsewhere in the bundle.
Update to follow the same import pattern as collectionBulkActions and
other app/utils modules: import { browser } from '../../static/globals'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add 'moz-extension://' to SYSTEM_URL_PREFIXES array in background.js for Firefox extension URL filtering - Update saveCollectionSnapshot.js to filter extension URLs using browser.runtime.getURL() instead of scheme-literal checks, enabling cross-browser compatibility for both chrome-extension:// and moz-extension:// schemes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… already scheme-agnostic The original unwrapDeferredUrl logic is already scheme-agnostic and handles both chrome-extension:// and moz-extension:// URLs correctly. The added filter logic that dropped extension-origin tabs could silently destroy user data (forbidden by migration rules). Revert to original implementation which only unwraps deferred URLs without filtering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add Firefox build and web-ext lint steps to the test job in release.yml, ensuring the Firefox target builds successfully and passes lint checks on every push and PR. The --self-hosted flag skips AMO-listing-only checks until Phase 2/3 sets up the AMO listing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed in Firefox) Firefox doesn't support browser.windows.onBoundsChanged; optional chaining prevents TypeError at service worker startup, allowing subsequent listeners to attach. Tab events continue triggering auto-updates on Firefox. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nge allowlist Adds a Worker GET /auth/callback that Firefox's launchWebAuthFlow can use as a fixed, Google-registered redirect_uri (per-profile allizom.org URLs can't be pre-registered). The unauthenticated, internet-facing callback validates state's embedded target strictly (https + hostname ends with the dot-prefixed .extensions.allizom.org, closing off lookalike-suffix and prefix-as-substring open-redirect tricks) before 302-redirecting the code (or error) back to it, with state echoed only in the Location header, never in an HTML body. googleAuth.js's isExtensionRedirect now also accepts exactly selfOrigin + /auth/callback so /auth/token's code exchange works for both the Chrome/Edge chromiumapp.org redirect and this new Firefox path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adversarial tests) Cache-Control: no-store on both the 302 redirect and 400 JSON responses (auth codes must never be cached). Strip embedded userinfo (user:pass@) from the target before building the redirect Location. Decode base64url state via TextDecoder instead of the latin1-per-byte atob path. Drop dead typeof-string checks on URLSearchParams.get() results (always string|null). Pin four adversarial targets already rejected as explicit regression tests: bare-domain-as-prefix, hyphen-boundary lookalike, backslash-form foreign host, and userinfo-smuggling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ew deps) Boots build-firefox/ as a temporary WebExtension in real Firefox (selenium-webdriver + geckodriver, staged into a throwaway npm prefix at run time so package.json/yarn.lock stay untouched) and asserts the popup, full-page view, and background event page all come up correctly. Playwright can't load Firefox extensions, hence the separate harness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Google sign-in via launchWebAuthFlow can't work on Firefox as-is: identity.getRedirectURL() returns a per-profile https://<uuid>.extensions.allizom.org/ URL that can't be pre-registered with Google. Branch createAuthEndpoint/getTokens on the redirect value (never UA): *.chromiumapp.org (Chrome/Edge) keeps the exact current auth URL and exchange, byte-identical; anything else routes through the Worker's fixed /auth/callback, packing the real per-profile redirect and a per-attempt nonce into `state`. The login handler verifies the Worker's echoed nonce before ever exchanging the code, rejecting on mismatch or missing state. Also fixes three pre-existing tests whose mocked identity.getRedirectURL values happened to be non-chromiumapp.org and therefore silently flip to the new viaWorker path, plus jsdom's crypto polyfill lacking randomUUID (mutate crypto.randomUUID directly; replacing the global crypto object is a silent no-op under this jsdom version). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t-denied short-circuit - Replace the unconditional crypto.randomUUID() nonce call with the file's existing guarded generateUidSafe() — Chrome 89-90 (this extension's manifest minimum) predates Crypto.randomUUID, so login would have thrown before launchWebAuthFlow ever ran. - Capture getAuthRedirectConfig() once into authConfig and reuse it for the post-flow viaWorker decision, instead of calling it again after launchWebAuthFlow resolves (removes a TOCTOU window). - On the Worker path, short-circuit on params.error / missing params.code before calling getTokens — a user declining consent is the normal case, not a failure worth a doomed token exchange. - Drop the crypto.randomUUID stubs from tests/backgroundLoginEntitlement and tests/sharedSyncAlarm now that generateUidSafe's own Math.random fallback covers jsdom's missing randomUUID (their getAuthRedirectConfig stubs stay); adjust tests/oauthConfig accordingly and add a test asserting getTokens is skipped on error=access_denied with no code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e login comment Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rney Adds e2e-firefox/journey.cjs: creates a real Firefox tab group, saves it as a collection through the extension's real background storage path (importData -> saveSingleCollectionBG), and restores it into a fresh window via the same openTabs message the popup sends, asserting the tab group and tab count come back correctly. run.sh now runs both smoke.cjs and journey.cjs and exits non-zero if either fails. Surfaces one real Firefox-port bug (documented, not fixed): restoring a collection into a new window leaves an extra blank tab because chrome/background.js:975's isNewWindow() only recognizes Chrome's "://newtab" URL shape, which never matches Firefox's about:home/about:blank default tab. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…b pages in isNewWindow Firefox's default new-window starter tab (about:home/about:newtab/about:blank, or about:privatebrowsing for a fresh private window) never matched the Chrome-specific '://newtab' substring check, so isNewWindow() was always false on Firefox and openTabs() left an extra blank tab behind after restoring a collection into a new window. Found by the real-Firefox journey harness (.superpowers/sdd/task-8-firefox-harness-report.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix two live-Firefox bugs reported from the Firefox port:
1. browser.windows.onRemoved.addListener(fn, { windowTypes: ['normal'] })
throws `TypeError: Incorrect argument types for windows.onRemoved` on
Firefox, at the top level of chrome/background.js. Since that throw is
synchronous and unhandled, it aborted the rest of the script load, so
every listener registered after it (windows.onCreated/onFocusChanged/
onBoundsChanged, all tabs.* events) never attached on Firefox at all.
Wrap the filtered registration in try/catch and fall back to registering
the same named callback without the filter on TypeError; Chrome's
behavior is unchanged since the filtered call still succeeds there.
2. Firefox reports chrome://mozapps/skin/... favIconUrl values for its own
privileged pages, and Chrome-authored collections can carry chrome://
favicons too, which extension pages are not allowed to load as images
(Security Error in Firefox). Add safeFavIconUrl() in
app/utils/sharedConstants.js as the single render-time guard (passes
http:/https:/data: URLs through, falls back otherwise) and apply it at
every favicon <img src> site across the popup and full-page views. This
never mutates stored collection data.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…only TypeError
The previous fix's catch block only swallowed TypeError and re-threw
everything else, but real Firefox's WebExtensions argument validator throws
a plain Error ("Incorrect argument types for windows.onRemoved.") for the
{ windowTypes } event filter, not a TypeError. That mismatch meant the
unfiltered-fallback registration never ran on real Firefox, and the original
top-level throw kept aborting every listener registered after it (confirmed
via live Firefox console after rebuild). Catch any throw from the filtered
registration and fall back unconditionally, since depending on a specific
Firefox error class isn't part of any spec and isn't future-proof.
Updated tests/backgroundWindowsOnRemovedFirefoxFallback.test.js to exercise
both the real plain-Error shape and TypeError as a secondary case, plus a
case for the (already-existing, unrelated to this fix) outer try/catch that
swallows a throw from the fallback registration itself.
Added real-Firefox proof beyond the jest mocks: the e2e-firefox journey now
restores with trackOpenedWindow: true, closes the tracked window, and
asserts collectionsToTrack was pruned by windowId afterward — that pruning
only happens if the windows.onRemoved listener actually attached and fired,
which is exactly what the { windowTypes } filter bug prevented. Verified
green against a real Firefox binary via e2e-firefox/run.sh (smoke 8/8,
journey 16/16 including the two new listener-registration assertions).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ardown can't orphan them Firefox destroys the popup document the moment a newly created window takes focus, so the popup's old flow (windows.create, then send openTabs) never sent the message — a blank window with no tabs. The popup now builds a createWindowSpec and sends ONE message before any window exists; the background creates the window (incl. incognito fallback), opens the tabs, and stamps lastOpened. All four new-window call sites converted (popup open, folder open-all, full-page sidebar, bulk actions). Verified: jest 257 suites/1907 tests, Chrome e2e 72/72 on a clean build, real-Firefox harness 20/20 incl. a createWindowSpec journey fence, Chrome manifest byte-identical to baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lete/sync machinery) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…absWindow consolidation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rag-resize guard test, and 4.2 marketing drafts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…background-clip:text Firefox paints a text-clipped layer inside a multi-value background-clip list as a full box, so the gradient covered the card and the transparent "Welcome" text vanished. The text gradient now lives on an inner span with a single-value clip; regression test guards all CSS files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…snap to frame zero; saved-box flight moved off the layout path Two causes of the perceived lag: 1. On every step change the outgoing scene both remounted (key flip to `-idle`) and lost `.is-active`, stripping its animations and visibly resetting it to frame zero while still sliding out. Scenes now keep a per-scene run counter: only the scene becoming active remounts (restarting its animation), and played scenes retain `.is-active` so fill-mode holds their end state during the slide. 2. `saved-box-to-tabox` animated left/top (reflow every frame). The flight now uses transform with container-query units (translate(calc(100cqw - 302px))), landing within 1px of the old position in both engines, verified in real Firefox and Chromium. Regression tests cover outgoing-scene persistence and ban left/top in scene keyframes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…droid) for data_collection_permissions; keep .DS_Store out of builds Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rs, replace webpack global shim AMO flagged three `Function` constructor sites: webpack's global-object runtime shim and regenerator-runtime. Added browserslist (Chrome/Edge 89+, Firefox 140+) so Babel stops transpiling async/await (removing regenerator entirely, -240KB fullpage) and replaced webpack's `global` handling with a globalThis define. Surfaced three latent const-TDZ bugs previously masked by ES5 var-hoisting (AIToolsModal dispatchAiRun, App refreshLastSyncTimeFromStorage, FPContentArea hasSelectedCollections / visibleSingleTabEntryKeySet) — declarations reordered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etadata (drop MIT badge, UNLICENSED) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolves #69 — repo no longer shows as unlicensed on GitHub. Terms match the UNLICENSED package metadata: view/build/contribute allowed, no redistribution or reuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tabox now builds and runs on Firefox 139+ (tested on 153) from the same codebase, with zero Chromium regression.
yarn build:firefox→build-firefox/via a purebuildManifest()transform; Chrome's manifest stays the single source of truth and its build output is byte-identical (gated per commit against a baseline)background.scriptson Firefox (guardedimportScriptsfor Chrome's SW)share.tbxpro.app/auth/callbackredirect with CSRF nonce, since Firefox's per-profile redirect URL can't be registered with Google; Chrome's auth URL is pinned byte-identical by testsystem.display(→ primary-screen fallback),windows.onBoundsChanged,moz-extension://scheme awareness, Firefox new-tab URLs inisNewWindow(fixes an extra blank tab on restore)e2e-firefox/run.sh, selenium via throwaway install — no lockfile changes): smoke 8/8 + save/restore journey with a grey tab group 14/14; CI now builds +web-ext lints the Firefox target on every pushVerification
build/manifest.jsonbyte-identical to pre-branch baseline; no lockfile changesBefore Firefox sign-in works live (not blocking merge)
https://share.tbxpro.app/auth/callbackin the Google OAuth client's authorized redirect URIs/auth/callbackroute)Notes for reviewer
fea42dd, share.tbxpro.app API-base switch) is pre-existing work fromfeature/4.2.1this branch was cut on — confirm intended to ship togetherdata_collection_permissions: ["browsingActivity"]needs owner sign-off before store submissiononBoundsChanged); share-link handoff deferred (noexternally_connectable)🤖 Generated with Claude Code