fix(linux/wayland): collapse 3-step capture flow into a single portal dialog (closes #268)#269
Conversation
… dialog Previously on Linux/Wayland, starting a fullscreen recording required three separate picker interactions: an in-app source dropdown plus two xdg-desktop-portal dialogs. The duplicate portal dialogs were caused by: 1. resolveBrowserCaptureSource() calling desktopCapturer.getSources() in the renderer, which itself triggers the portal on Wayland. 2. setDisplayMediaRequestHandler() in main also calling getSources(), which triggers another portal. 3. Returning a pre-enumerated source id to Chromium, which on Wayland is stale and forces Chromium to re-prompt via the portal during MediaStream creation. Additionally, the editor window failed to appear after recording on some Wayland sessions because 'ready-to-show' did not fire reliably. Changes: - LaunchWindow: skip the in-app source dropdown on Linux and start recording directly; the OS portal becomes the source picker. - useScreenRecorder: introduce a 'screen:linux-portal' sentinel source. When set, route capture through navigator.mediaDevices.getDisplayMedia() so the portal handles selection in a single dialog. Skip resolveBrowserCaptureSource for the sentinel to avoid an extra getSources() call. - electron/main: in setDisplayMediaRequestHandler, when the sentinel is set, skip desktopCapturer.getSources() entirely and return a synthetic source so Chromium opens the portal exactly once for the actual capture. - electron/windows: in createEditorWindow, also call win.show() from did-finish-load as a fallback for Linux/Wayland where ready-to-show may not fire.
|
|
📝 WalkthroughWalkthroughIntroduces a Linux/Wayland "portal sentinel" flow: auto-selects a synthetic source on Linux, bypasses desktopCapturer enumeration and in-app picker, routes capture via getDisplayMedia for portal-compatible grants, and forces editor window visibility post-load on problematic platforms. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as LaunchWindow
participant Recorder as useScreenRecorder
participant IPC as Electron Main
participant Browser as Browser API
participant OS as OS Portal
rect rgba(200,100,100,0.5)
Note over User,OS: Old Linux/Wayland Flow (multiple picker steps)
User->>UI: Click Record (no source)
UI->>Recorder: openSourcesDropdown()
Recorder->>IPC: getSources()
IPC->>OS: portal dialog (xdg-desktop-portal)
OS->>IPC: user selects screen
IPC->>UI: sources list
UI->>User: show in-app dropdown
User->>UI: pick source
UI->>Recorder: startRecording(selected)
Recorder->>Browser: getDisplayMedia() / getUserMedia()
Browser->>OS: portal dialog (second)
OS->>Browser: grant
Browser->>Recorder: stream
end
rect rgba(100,200,100,0.5)
Note over User,OS: New Linux/Wayland Flow (single portal)
User->>UI: Click Record (no source)
UI->>Recorder: startRecording()
Recorder->>Recorder: auto-select sentinel (screen:linux-portal)
Recorder->>Browser: getDisplayMedia(constraints)
Browser->>OS: single portal dialog
OS->>Browser: grant
Browser->>Recorder: stream
Recorder->>Recorder: start recording
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/launch/LaunchWindow.tsx (1)
1094-1110:⚠️ Potential issue | 🟠 MajorThe source-selector button is still rendered on Linux and can re-introduce an extra portal dialog.
The record-button branch correctly delegates to
toggleRecordingon Linux, but the screen/source selector button (lines 1096–1110) is still visible on Linux. If a user clicks it,toggleDropdown("sources")callsfetchSources()(line 858) →window.electronAPI.getSources(...), which on Wayland triggers anxdg-desktop-portaldialog purely to populate the in-app dropdown. That directly reintroduces the extra portal the PR is meant to eliminate (and the entries it surfaces are the stale IDs called out in#268).Per the PR objectives ("Eliminate the in-app source picker on Wayland"), consider hiding this control on Linux, e.g.:
♻️ Proposed fix
- <button - type="button" - className={`${styles.screenSel} ${styles.electronNoDrag}`} - onClick={() => toggleDropdown("sources")} - title={selectedSource} - > - <Monitor size={16} /> - <ContentClamp className={styles.sourceLabel} truncateLength={36}> - {selectedSource} - </ContentClamp> - <ChevronUp - size={10} - className={`text-[`#6b6b78`] ml-0.5 transition-transform duration-200 ${activeDropdown === "sources" ? "" : "rotate-180"}`} - /> - </button> - - <Separator /> + {platform !== "linux" && ( + <> + <button + type="button" + className={`${styles.screenSel} ${styles.electronNoDrag}`} + onClick={() => toggleDropdown("sources")} + title={selectedSource} + > + <Monitor size={16} /> + <ContentClamp className={styles.sourceLabel} truncateLength={36}> + {selectedSource} + </ContentClamp> + <ChevronUp + size={10} + className={`text-[`#6b6b78`] ml-0.5 transition-transform duration-200 ${activeDropdown === "sources" ? "" : "rotate-180"}`} + /> + </button> + <Separator /> + </> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/LaunchWindow.tsx` around lines 1094 - 1110, The source-selector button inside idleControls (the JSX block rendering the button that calls onClick={() => toggleDropdown("sources")}) must be hidden on Linux to avoid triggering fetchSources() → window.electronAPI.getSources(...) (which spawns xdg-desktop-portal on Wayland); update the conditional rendering so this button is only rendered when the platform is not Linux (use your existing platform helper or process.platform !== 'linux') and leave the toggleRecording logic for Linux unchanged; ensure you reference the same symbols (idleControls, toggleDropdown("sources"), fetchSources, window.electronAPI.getSources, selectedSource, activeDropdown) so the button is excluded on Linux while keeping behavior identical on other OSes.src/hooks/useScreenRecorder.ts (1)
838-848:⚠️ Potential issue | 🟠 MajorRenderer-only sentinel synthesis — may not be visible to the main process's display-media handler.
This synthesizes
{ id: "screen:linux-portal", name: "Linux Portal" }locally but doesn't callwindow.electronAPI.selectSource(selectedSource)to tell the main process. ThesetDisplayMediaRequestHandlerinelectron/main.ts(lines 892–924) gates the single-portal bypass ongetSelectedSourceId() === "screen:linux-portal", so unless that state is populated elsewhere, the bypass won't engage on a fresh session. See the related comment onelectron/main.ts— please either persist the sentinel here beforegetDisplayMediais invoked, or make the main-side default kick in whenprocess.platform === "linux"and no source has been selected.Also:
"Linux Portal"is hardcoded English. If this name can ever surface in the UI (e.g. via theselectedSourcelabel inLaunchWindow.tsx), it should be routed through the i18n scope the rest of recording labels use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 838 - 848, The code in useScreenRecorder.ts creates a local sentinel selectedSource ({ id: "screen:linux-portal", name: "Linux Portal" }) but never informs the main process, so electron/main.ts's setDisplayMediaRequestHandler (which checks getSelectedSourceId() === "screen:linux-portal") won't see it; before calling getDisplayMedia (or when assigning selectedSource), call window.electronAPI.selectSource(selectedSource) to persist the sentinel in the main process, and ensure the label uses the app i18n string rather than the hardcoded "Linux Portal" (route through the same i18n scope used by other recording labels). Also consider the alternative fix noted in the review: update the main-side handler to default to the portal on linux when no selected source exists by checking process.platform === "linux" and getSelectedSourceId() is null.electron/main.ts (1)
892-924:⚠️ Potential issue | 🔴 CriticalVerify the sentinel ever reaches
getSelectedSourceId()on Linux — the fix is ineffective on a fresh session.This handler only triggers the single-portal path when
getSelectedSourceId() === "screen:linux-portal". However,selectedSourceinitializes tonull(electron/ipc/state.ts:16) and is only set via the"select-source"IPC handler whenhandleSourceSelectis called.The problem: on Linux, the dropdown is always skipped (LaunchWindow.tsx:1148:
hasSelectedSource || platform === "linux"), sohandleSourceSelectnever runs andselectSourceis never invoked. Meanwhile, inuseScreenRecorder.ts:843, the sentinel is synthesized locally insidestartRecordingbut is never persisted to main:const selectedSource = existingSource ?? (platform === "linux" ? { id: "screen:linux-portal", name: "Linux Portal" } : null);There is no corresponding
window.electronAPI.selectSource(selectedSource)call.On a fresh session,
getSelectedSourceId()returnsnull(not"screen:linux-portal"),isLinuxPortalSentinelevaluates tofalse, the handler falls through todesktopCapturer.getSources()— the exact call that triggers the xdg-desktop-portal dialog and produces stale IDs — and you'll hit the double-portal behavior#268is trying to prevent.The fix requires either:
- Send the sentinel to main via
window.electronAPI.selectSource(selectedSource)before invokinggetDisplayMediaon Linux, or- Default to the sentinel in the main handler when
process.platform === "linux"andsourceIdis null.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/main.ts` around lines 892 - 924, The Linux sentinel ("screen:linux-portal") never reaches getSelectedSourceId() on fresh sessions, so setDisplayMediaRequestHandler falls back to desktopCapturer.getSources() and triggers a double portal; fix by ensuring the sentinel is persisted to main before calling getDisplayMedia on Linux: in useScreenRecorder.startRecording (or the code path that builds selectedSource) call window.electronAPI.selectSource(selectedSource) when platform === "linux" and selectedSource.id === "screen:linux-portal" so the main IPC selectSource handler (handleSourceSelect) updates selectedSource used by getSelectedSourceId(); alternatively, if you prefer a main-side change, modify session.defaultSession.setDisplayMediaRequestHandler to treat null sourceId on process.platform === "linux" as the sentinel (i.e., if process.platform === "linux" && !sourceId then act like sourceId === "screen:linux-portal").
🧹 Nitpick comments (1)
src/hooks/useScreenRecorder.ts (1)
1031-1103: Heavy duplication across the three Linux-portalgetDisplayMediabranches.The three
getDisplayMedia({ ..., video: { displaySurface: "monitor", width, height, frameRate, cursor: "never" }, selfBrowserSurface: "exclude", surfaceSwitching: "exclude" })calls at lines 1037–1049, 1067–1079, and 1086–1098 differ only in theaudioboolean. Extracting a small helper removes ~30 lines and makes the constraint set easier to keep in sync with the non-audio fallback at lines 1172–1185 (which currently also duplicates the same shape).♻️ Proposed refactor
+ const acquireLinuxPortalStream = (withAudio: boolean) => + mediaDevices.getDisplayMedia({ + audio: withAudio, + video: { + displaySurface: "monitor", + width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH }, + height: { ideal: TARGET_HEIGHT, max: TARGET_HEIGHT }, + frameRate: { ideal: TARGET_FRAME_RATE, max: TARGET_FRAME_RATE }, + cursor: "never", + }, + selfBrowserSurface: "exclude", + surfaceSwitching: "exclude", + }); + if (wantsAudioCapture) { let screenMediaStream: MediaStream; const useLinuxPortal = selectedSource.id === "screen:linux-portal"; if (systemAudioEnabled) { try { screenMediaStream = useLinuxPortal - ? await mediaDevices.getDisplayMedia({ /* audio: true ... */ }) + ? await acquireLinuxPortalStream(true) : await mediaDevices.getUserMedia({ /* ... */ }); } catch (audioError) { ... screenMediaStream = useLinuxPortal - ? await mediaDevices.getDisplayMedia({ /* audio: false ... */ }) + ? await acquireLinuxPortalStream(false) : await mediaDevices.getUserMedia({ /* ... */ }); } } else { screenMediaStream = useLinuxPortal - ? await mediaDevices.getDisplayMedia({ /* audio: false ... */ }) + ? await acquireLinuxPortalStream(false) : await mediaDevices.getUserMedia({ /* ... */ }); }Separately — worth a thought but not a blocker: on Wayland, the
audio: trueportal request will frequently be rejected (many portal backends still don't implement system audio), and your catch falls through to a secondgetDisplayMedia({ audio: false }), which means the user sees two portal dialogs on failure. If you have a way to probe capability (or a setting for "try system audio on Wayland"), wiring that in would keep the "exactly one portal" promise even in the failure path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 1031 - 1103, Duplicate getDisplayMedia constraint objects for the Linux portal are used in useScreenRecorder (inside the wantsAudioCapture branch where useLinuxPortal is computed and in the systemAudioEnabled catch/fallback and the else branch); extract a small helper function (e.g., buildLinuxDisplayMediaConstraints or getLinuxDisplayMedia) that accepts an audio boolean and returns the shared { audio, video: { displaySurface: "monitor", width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH }, height: { ideal: TARGET_HEIGHT, max: TARGET_HEIGHT }, frameRate: { ideal: TARGET_FRAME_RATE, max: TARGET_FRAME_RATE }, cursor: "never" }, selfBrowserSurface: "exclude", surfaceSwitching: "exclude" } shape, then replace the three direct mediaDevices.getDisplayMedia({...}) calls with mediaDevices.getDisplayMedia(getLinuxDisplayMedia(true/false)); keep references to systemAudioEnabled, useLinuxPortal, TARGET_WIDTH/TARGET_HEIGHT/TARGET_FRAME_RATE and browserScreenVideoConstraints intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 1147-1151: The onClick currently branches on hasSelectedSource ||
platform === "linux" but platform is initialized as null and populated async, so
add an explicit check for platform === null to force the non-Linux path until
platform is known; update the handler that references toggleRecording and
toggleDropdown so it first returns () => toggleDropdown("sources") when platform
=== null (or otherwise defer action) to avoid the race where a click before
platform resolution takes the Linux branch.
---
Outside diff comments:
In `@electron/main.ts`:
- Around line 892-924: The Linux sentinel ("screen:linux-portal") never reaches
getSelectedSourceId() on fresh sessions, so setDisplayMediaRequestHandler falls
back to desktopCapturer.getSources() and triggers a double portal; fix by
ensuring the sentinel is persisted to main before calling getDisplayMedia on
Linux: in useScreenRecorder.startRecording (or the code path that builds
selectedSource) call window.electronAPI.selectSource(selectedSource) when
platform === "linux" and selectedSource.id === "screen:linux-portal" so the main
IPC selectSource handler (handleSourceSelect) updates selectedSource used by
getSelectedSourceId(); alternatively, if you prefer a main-side change, modify
session.defaultSession.setDisplayMediaRequestHandler to treat null sourceId on
process.platform === "linux" as the sentinel (i.e., if process.platform ===
"linux" && !sourceId then act like sourceId === "screen:linux-portal").
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 1094-1110: The source-selector button inside idleControls (the JSX
block rendering the button that calls onClick={() => toggleDropdown("sources")})
must be hidden on Linux to avoid triggering fetchSources() →
window.electronAPI.getSources(...) (which spawns xdg-desktop-portal on Wayland);
update the conditional rendering so this button is only rendered when the
platform is not Linux (use your existing platform helper or process.platform !==
'linux') and leave the toggleRecording logic for Linux unchanged; ensure you
reference the same symbols (idleControls, toggleDropdown("sources"),
fetchSources, window.electronAPI.getSources, selectedSource, activeDropdown) so
the button is excluded on Linux while keeping behavior identical on other OSes.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 838-848: The code in useScreenRecorder.ts creates a local sentinel
selectedSource ({ id: "screen:linux-portal", name: "Linux Portal" }) but never
informs the main process, so electron/main.ts's setDisplayMediaRequestHandler
(which checks getSelectedSourceId() === "screen:linux-portal") won't see it;
before calling getDisplayMedia (or when assigning selectedSource), call
window.electronAPI.selectSource(selectedSource) to persist the sentinel in the
main process, and ensure the label uses the app i18n string rather than the
hardcoded "Linux Portal" (route through the same i18n scope used by other
recording labels). Also consider the alternative fix noted in the review: update
the main-side handler to default to the portal on linux when no selected source
exists by checking process.platform === "linux" and getSelectedSourceId() is
null.
---
Nitpick comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 1031-1103: Duplicate getDisplayMedia constraint objects for the
Linux portal are used in useScreenRecorder (inside the wantsAudioCapture branch
where useLinuxPortal is computed and in the systemAudioEnabled catch/fallback
and the else branch); extract a small helper function (e.g.,
buildLinuxDisplayMediaConstraints or getLinuxDisplayMedia) that accepts an audio
boolean and returns the shared { audio, video: { displaySurface: "monitor",
width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH }, height: { ideal:
TARGET_HEIGHT, max: TARGET_HEIGHT }, frameRate: { ideal: TARGET_FRAME_RATE, max:
TARGET_FRAME_RATE }, cursor: "never" }, selfBrowserSurface: "exclude",
surfaceSwitching: "exclude" } shape, then replace the three direct
mediaDevices.getDisplayMedia({...}) calls with
mediaDevices.getDisplayMedia(getLinuxDisplayMedia(true/false)); keep references
to systemAudioEnabled, useLinuxPortal,
TARGET_WIDTH/TARGET_HEIGHT/TARGET_FRAME_RATE and browserScreenVideoConstraints
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba1e6910-8491-412e-9515-0dc5266b2a54
📒 Files selected for processing (4)
electron/main.tselectron/windows.tssrc/components/launch/LaunchWindow.tsxsrc/hooks/useScreenRecorder.ts
| onClick={ | ||
| hasSelectedSource || platform === "linux" | ||
| ? toggleRecording | ||
| : () => toggleDropdown("sources") | ||
| } |
There was a problem hiding this comment.
Minor: the platform === "linux" branch fires before platform is known.
platform starts as null and is populated asynchronously by the effect at lines 644–658. That's fine here (clicking record with null platform just opens the dropdown, and after platform resolves the Linux branch takes over), but if you want this deterministic you could also early-return when platform === null to avoid a subtle window where a Linux user who clicks immediately on launch gets the non-Linux branch. Low priority.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/launch/LaunchWindow.tsx` around lines 1147 - 1151, The onClick
currently branches on hasSelectedSource || platform === "linux" but platform is
initialized as null and populated async, so add an explicit check for platform
=== null to force the non-Linux path until platform is known; update the handler
that references toggleRecording and toggleDropdown so it first returns () =>
toggleDropdown("sources") when platform === null (or otherwise defer action) to
avoid the race where a click before platform resolution takes the Linux branch.
|
please address coderabbit review findings |
- electron/main.ts: treat missing sourceId on linux as portal sentinel so the request handler never calls getSources() (which itself opens an extra portal dialog) on fresh sessions. - useScreenRecorder.ts: persist the synthesized portal sentinel via selectSource() so main has the source set before getDisplayMedia. Extract acquireLinuxPortalStream() helper to dedupe the three duplicated getDisplayMedia constraint blocks. - LaunchWindow.tsx: hide the screen-source selector button (and its separator) on Linux so users cannot trigger an extra portal dialog via the dropdown.
|
Addressed review feedback in b200ded:
|
|
|
1 similar comment
|
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/useScreenRecorder.ts (2)
1043-1092: ReuseacquireLinuxPortalStreamin the no-audio branch to avoid constraint drift.
acquireLinuxPortalStreamis defined inside thewantsAudioCapturebranch and only used there. The no-audio branch (lines 1161-1175) independently builds an almost-identicalgetDisplayMediacall. For the sentinel today both paths behave the same (displaySurface resolves to"monitor"for"screen:linux-portal"), but keeping two copies means future tweaks to portal-capture constraints (framerate, cursor, selfBrowserSurface, etc.) must be applied twice or they'll silently diverge.Consider hoisting the helper above the
wantsAudioCapturecheck and routing the no-audio sentinel path through it as well.♻️ Sketch
+ const useLinuxPortal = selectedSource.id === "screen:linux-portal"; + const acquireLinuxPortalStream = (withAudio: boolean) => + mediaDevices.getDisplayMedia({ + audio: withAudio, + video: { + displaySurface: "monitor", + width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH }, + height: { ideal: TARGET_HEIGHT, max: TARGET_HEIGHT }, + frameRate: { ideal: TARGET_FRAME_RATE, max: TARGET_FRAME_RATE }, + cursor: "never", + }, + selfBrowserSurface: "exclude", + surfaceSwitching: "exclude", + }); + if (wantsAudioCapture) { let screenMediaStream: MediaStream; - const useLinuxPortal = selectedSource.id === "screen:linux-portal"; - const acquireLinuxPortalStream = (withAudio: boolean) => ... ... } else { - const mediaStream = await mediaDevices.getDisplayMedia({ ... }); + const mediaStream = useLinuxPortal + ? await acquireLinuxPortalStream(false) + : await mediaDevices.getDisplayMedia({ ... existing non-sentinel constraints ... }); stream.current = mediaStream; videoTrack = mediaStream.getVideoTracks()[0]; }Also applies to: 1161-1179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 1043 - 1092, The no-audio branch duplicates the getDisplayMedia constraints instead of reusing the helper; hoist the acquireLinuxPortalStream function (currently defined near useLinuxPortal) above the systemAudioEnabled/wantsAudioCapture check and replace both places that build the Linux portal getDisplayMedia call with calls to acquireLinuxPortalStream(withAudio: boolean) (use useLinuxPortal to choose between acquireLinuxPortalStream(...) and mediaDevices.getUserMedia(...)), removing the duplicated getDisplayMedia block so all portal-specific constraints (displaySurface, width/height/frameRate, cursor, selfBrowserSurface, surfaceSwitching) are maintained in one place for both audio and no-audio flows.
838-858: Consider improving the user-facing name for the Linux portal sentinel.The synthetic sentinel's
nameof"Linux Portal"will display in the tray tooltip as"Recording: Linux Portal". Something like"Screen (via portal)"would be clearer to end users. The{ id, name }literal structure is already type-safe —ProcessedDesktopSourcecontains only these two fields, so no undefined values are passed through the IPC boundary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useScreenRecorder.ts` around lines 838 - 858, The synthetic Linux portal sentinel uses a user-facing name "Linux Portal" which can be unclear; update the created ProcessedDesktopSource literal in useScreenRecorder (where selectedSource is set when existingSource is null and platform === "linux") to use a clearer name such as "Screen (via portal)" while keeping the id "screen:linux-portal", and ensure the same name is passed when persisting via window.electronAPI.selectSource so the tray tooltip shows the improved label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/useScreenRecorder.ts`:
- Around line 1043-1092: The no-audio branch duplicates the getDisplayMedia
constraints instead of reusing the helper; hoist the acquireLinuxPortalStream
function (currently defined near useLinuxPortal) above the
systemAudioEnabled/wantsAudioCapture check and replace both places that build
the Linux portal getDisplayMedia call with calls to
acquireLinuxPortalStream(withAudio: boolean) (use useLinuxPortal to choose
between acquireLinuxPortalStream(...) and mediaDevices.getUserMedia(...)),
removing the duplicated getDisplayMedia block so all portal-specific constraints
(displaySurface, width/height/frameRate, cursor, selfBrowserSurface,
surfaceSwitching) are maintained in one place for both audio and no-audio flows.
- Around line 838-858: The synthetic Linux portal sentinel uses a user-facing
name "Linux Portal" which can be unclear; update the created
ProcessedDesktopSource literal in useScreenRecorder (where selectedSource is set
when existingSource is null and platform === "linux") to use a clearer name such
as "Screen (via portal)" while keeping the id "screen:linux-portal", and ensure
the same name is passed when persisting via window.electronAPI.selectSource so
the tray tooltip shows the improved label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb707694-3d73-468f-8571-c2e529fc8ad3
📒 Files selected for processing (3)
electron/main.tssrc/components/launch/LaunchWindow.tsxsrc/hooks/useScreenRecorder.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/launch/LaunchWindow.tsx
2 participants
Summary
On Linux/Wayland, starting a fullscreen recording previously required three separate picker interactions:
desktopCapturer.getSources()in the renderer)After this PR, Linux users go straight from the record button to a single OS portal dialog, matching the streamlined flow on macOS/Windows.
Fixes #268. Builds on top of #267.
Root cause
Wayland's
xdg-desktop-portalmodel is fundamentally different from X11/macOS/Windows:desktopCapturer.getSources()itself triggers a portal dialog on Wayland.getSourcesare stale by the time Chromium tries to start the actual capture, so Chromium opens the portal again to resolve a real source.useSystemPickeroption (which bypasses the handler) is currently macOS-only.Changes
src/components/launch/LaunchWindow.tsx: Skip the in-app source dropdown on Linux — the OS portal is the source picker.src/hooks/useScreenRecorder.ts:screen:linux-portalsentinel source.navigator.mediaDevices.getDisplayMedia()(instead ofgetUserMediawithchromeMediaSource: 'desktop') when the sentinel is active.resolveBrowserCaptureSourcefor the sentinel to avoid an extragetSources()portal trigger.electron/main.ts(setDisplayMediaRequestHandler): When the sentinel is set, skipdesktopCapturer.getSources()entirely and return a synthetic source — Chromium then opens the portal exactly once, for the actual capture.electron/windows.ts(createEditorWindow): Also callwin.show()fromdid-finish-loadas a fallback. On some Wayland sessionsready-to-showdoes not fire reliably, leaving the editor window hidden after a recording finishes.Testing
Tested locally on Linux/Wayland (Electron 39):
No changes to macOS/Windows code paths — all sentinel logic is gated on
process.platform === "linux"orselectedSource.id === "screen:linux-portal".Summary by CodeRabbit