feat(extension): Firefox runtime — recorder window, storage fallbacks, and overlay injection - #2001
Conversation
ManthanNimodiya
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| chrome.windows.onRemoved.addListener(() => { | ||
| if (capabilities.supportsOffscreen) return; | ||
| void syncRecordingStatus().catch(() => undefined); | ||
| }); |
There was a problem hiding this comment.
When the Firefox recorder popup is closed while it is waiting on the arm button, this removal handler calls syncRecordingStatus(), but that path only resets statuses considered active. creating is not active, so the service worker keeps returning the stale starting state and the recording UI can stay stuck until another action overwrites it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/background/service-worker.ts
Line: 1925-1928
Comment:
**Creating State Stays Stuck**
When the Firefox recorder popup is closed while it is waiting on the arm button, this removal handler calls `syncRecordingStatus()`, but that path only resets statuses considered active. `creating` is not active, so the service worker keeps returning the stale starting state and the recording UI can stay stuck until another action overwrites it.
How can I resolve this? If you propose a fix, please make it concise.| // script (overlay, countdown, recording bar) stays inert until the user | ||
| // grants access here. The click on the button supplies the required user | ||
| // gesture for permissions.request. | ||
| const HOST_PERMISSION_ORIGINS = ["http://*/*", "https://*/*"]; |
There was a problem hiding this comment.
The Firefox manifest declares file:///* and the injection logic accepts file: tabs, but this permission request only asks for http and https. A Firefox user can complete the new grant flow and still never grant local-file access, so the content script, countdown, and recording toolbar will not run on file:// pages despite the manifest advertising support.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/welcome/main.ts
Line: 53
Comment:
**File Permission Never Granted**
The Firefox manifest declares `file:///*` and the injection logic accepts `file:` tabs, but this permission request only asks for `http` and `https`. A Firefox user can complete the new grant flow and still never grant local-file access, so the content script, countdown, and recording toolbar will not run on `file://` pages despite the manifest advertising support.
How can I resolve this? If you propose a fix, please make it concise.![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| // setAccessLevel at all — calling it unconditionally throws and kills the | ||
| // whole background script — so content scripts there rely on the runtime | ||
| // message fallbacks instead of the session-storage mirror. | ||
| chrome.storage.session.setAccessLevel?.({ |
There was a problem hiding this comment.
minor hardening: this still throws if chrome.storage.session is undefined (not just setAccessLevel).
| chrome.storage.session.setAccessLevel?.({ | |
| chrome.storage.session?.setAccessLevel?.({ |
| chrome.windows.onRemoved.addListener(() => { | ||
| if (capabilities.supportsOffscreen) return; | ||
| void syncRecordingStatus().catch(() => undefined); | ||
| }); |
There was a problem hiding this comment.
this fires on any window close; quick guard avoids extra getContexts calls when idle.
| chrome.windows.onRemoved.addListener(() => { | |
| if (capabilities.supportsOffscreen) return; | |
| void syncRecordingStatus().catch(() => undefined); | |
| }); | |
| chrome.windows.onRemoved.addListener(() => { | |
| if (capabilities.supportsOffscreen) return; | |
| if (recordingStatus.phase === "idle") return; | |
| void syncRecordingStatus().catch(() => undefined); | |
| }); |
| readFileSync( | ||
| resolve(__dirname, `../../manifests/manifest.${target}.json`), | ||
| "utf8", | ||
| ), |
There was a problem hiding this comment.
__dirname can be undefined under ESM (and this package is type: module). using import.meta.url keeps this stable in vitest.
| readFileSync( | |
| resolve(__dirname, `../../manifests/manifest.${target}.json`), | |
| "utf8", | |
| ), | |
| readFileSync( | |
| new URL(`../../manifests/manifest.${target}.json`, import.meta.url), | |
| "utf8", | |
| ), |
29f0437 to
03f6060
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
03f6060 to
82eb526
Compare
| const resolveAvailableMode = (mode: RecordingMode): RecordingMode => | ||
| mode === "tab" && !capabilities.supportsTabCapture ? "fullscreen" : mode; | ||
|
|
||
| const HOST_PERMISSION_ORIGINS = ["http://*/*", "https://*/*"]; |
There was a problem hiding this comment.
This popup check still treats http/https access as enough on Firefox. If a user grants those origins but leaves file access disabled, hostAccessGranted becomes true, the warning is hidden, and recording from a file:// tab still has no injected countdown or toolbar because file:///* was never granted. Keep this list in sync with the welcome-page request and the Firefox manifest.
| const HOST_PERMISSION_ORIGINS = ["http://*/*", "https://*/*"]; | |
| const HOST_PERMISSION_ORIGINS = ["http://*/*", "https://*/*", "file:///*"]; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/popup/main.tsx
Line: 101
Comment:
**File Access Check Missing**
This popup check still treats http/https access as enough on Firefox. If a user grants those origins but leaves file access disabled, `hostAccessGranted` becomes true, the warning is hidden, and recording from a `file://` tab still has no injected countdown or toolbar because `file:///*` was never granted. Keep this list in sync with the welcome-page request and the Firefox manifest.
```suggestion
const HOST_PERMISSION_ORIGINS = ["http://*/*", "https://*/*", "file:///*"];
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.… script for Firefox
…teScript overlay module on Firefox
…nclude file:// in permission request
056733a to
9825b54
Compare
| if (recordingStatus.phase === "creating") { | ||
| setRecordingStatusAndBroadcast({ phase: "idle" }); |
There was a problem hiding this comment.
When the Firefox recorder window is closed during the arm-button phase, this resets the visible status but leaves recordingStartInFlight pointing at the original start promise. If the start message already reached the recorder document, closing that document can leave awaitCaptureGesture() pending forever, so the .finally() that clears recordingStartInFlight never runs. The UI returns to idle, but later start attempts reuse the dead promise and recording cannot start again until the background is restarted.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/background/service-worker.ts
Line: 1855-1856
Comment:
**Start Promise Stays Wedged**
When the Firefox recorder window is closed during the arm-button phase, this resets the visible status but leaves `recordingStartInFlight` pointing at the original start promise. If the start message already reached the recorder document, closing that document can leave `awaitCaptureGesture()` pending forever, so the `.finally()` that clears `recordingStartInFlight` never runs. The UI returns to idle, but later start attempts reuse the dead promise and recording cannot start again until the background is restarted.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| chrome.runtime.onStartup.addListener(() => { | ||
| void clearSharedSessionState().catch(() => undefined); | ||
| }); |
There was a problem hiding this comment.
Firefox now stores shared recording UI state in storage.local, but this clears those session-like keys only on browser startup. Extension updates and reloads run runtime.onInstalled, which reinjects the bootstrap into open tabs without clearing stale cap-extension-recording-state or panel state first. If an old creating or recording value is present, the freshly injected content script reads it from storage.local and shows the recording UI even though no recorder host exists.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/chrome-extension/src/background/service-worker.ts
Line: 106-108
Comment:
**Local State Survives Updates**
Firefox now stores shared recording UI state in `storage.local`, but this clears those session-like keys only on browser startup. Extension updates and reloads run `runtime.onInstalled`, which reinjects the bootstrap into open tabs without clearing stale `cap-extension-recording-state` or panel state first. If an old `creating` or `recording` value is present, the freshly injected content script reads it from `storage.local` and shows the recording UI even though no recorder host exists.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.1 participant
Stacked on #2000 #1998 , only the last commits are new here, the earlier ones will disappear once the base PR merges.
Makes the Firefox build fully functional; every fix here was found and verified in live Firefox QA (recording → instant upload → share-page playback confirmed working on Firefox ESR 140):
Greptile Summary
This PR makes the Chrome extension work as a Firefox build. The main changes are:
Confidence Score: 4/5
This should be fixed before merging the Firefox runtime work.
Files Needing Attention: apps/chrome-extension/src/background/service-worker.ts
Important Files Changed
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix(extension): include file:// in Firef..." | Re-trigger Greptile
Context used: