Skip to content

fix: Jitsi loading freezes in RDP sessions#3104

Merged
jeanfbrito merged 11 commits intomasterfrom
feature/wgc-fallback-toggle
Sep 26, 2025
Merged

fix: Jitsi loading freezes in RDP sessions#3104
jeanfbrito merged 11 commits intomasterfrom
feature/wgc-fallback-toggle

Conversation

@jeanfbrito
Copy link
Copy Markdown
Member

@jeanfbrito jeanfbrito commented Sep 24, 2025

Video Call Window – Windows Graphics Capture Limitation

SUP-864

Summary

  • Users on Windows 10 and 11 hit a stalled "Loading video call" screen when they join Jitsi calls through Rocket.Chat Desktop 4.9.0.
  • The freeze happens mostly on Remote Desktop (RDP) sessions, but it can also appear on some on-console sessions depending on policy.
  • The root cause is Chromium switching to the Windows Graphics Capture (WGC) path, which cannot capture remote sessions. WebRTC reports "Source is not capturable" and the meeting never progresses.

Impact

  • Auto-open devtools never launches because the webview does not reach dom-ready.
  • The user-facing loading overlay never clears, so the call window appears frozen.
  • Frequency is “often but not always”, matching the intermittent success of WGC when users move between console and remote sessions.

Detection

  • On Windows, process.env.SESSIONNAME is Console for local sessions and RDP-Tcp#* (or similar) for remote sessions. We can check this once during startup.
  • If the value changes after launch (user switches to RDP), Chromium must be restarted to apply a new screen-capture backend.

Implemented Solution

  1. Windows-only "Screen Capture Fallback" toggle:
    • A new setting is available in Settings → General (only visible on Windows).
    • When enabled, this setting forces Chromium to use the legacy Desktop Duplication capturer, which works reliably in RDP sessions.
  2. Automatic RDP detection:
    • If process.env.SESSIONNAME is anything other than Console when the app starts, the app automatically disables Windows Graphics Capture, even if the toggle is off. This protects users who launch the client directly inside an RDP session.
  3. Smart restart behavior:
    • Users are advised to restart the desktop app after switching between local and RDP sessions to ensure the correct screen capture backend is active.
    • If the app is launched in an RDP session, the toggle can still be changed, but it will not trigger an immediate restart, as the fallback is already enforced by the environment. The change will be applied on the next app launch.
    • The UI shows a different description when the fallback is enforced by RDP detection: "Currently enforced because the app detected a Remote Desktop session. Toggle now controls future launches when running locally."

References

Summary by CodeRabbit

  • New Features

    • Windows-only setting to enable a video-call screen-capture fallback for more reliable screen sharing (auto-enforced in RDP); toggling may relaunch the app.
    • Setting is persisted and applied at startup to ensure consistent behavior.
  • Documentation

    • New guide on Windows Graphics Capture limitations, detecting remote sessions, and recommended workarounds.
  • Localization

    • UI translations added for EN, DE, ES, FI, FR, HU, NO, PT‑BR, RU, SV.

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.
Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.
Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

Adds a Windows-specific video-call screen-capture fallback with a new user setting and RDP-driven enforcement, wiring actions/reducers/selectors, startup initialization, settings UI and i18n entries, persistence migration, updates propagation, and documentation describing WGC limitations and workarounds.

Changes

Cohort / File(s) Summary of changes
Documentation
docs/video-call-window-wgc-limitations.md
New document describing Windows Graphics Capture (WGC) limitations, detection, and recommended workarounds.
Persistence & Migration
src/app/PersistableValues.ts
Add isVideoCallScreenCaptureFallbackEnabled to PersistableValues (v4.9.0), update exported type, and add migration defaults.
App Actions & Types
src/app/actions.ts, src/app/main/actions
Add APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET action constant and include it in AppActionTypeToPayloadMap.
Main App Logic / Init
src/app/main/app.ts, src/main.ts
Initialize screen-capture-fallback state on startup, detect Windows/RDP to force-disable WGC, dispatch forced state, log JSON reason, and conditionally relaunch on setting changes. Expose initializeScreenCaptureFallbackState.
Reducers (app/ui)
src/app/reducers/screenCaptureFallbackForced.ts, src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts
New reducer screenCaptureFallbackForced (boolean) and isVideoCallScreenCaptureFallbackEnabled reducer handling APP_SETTINGS_LOADED, UPDATES_READY, and SETTINGS change actions.
Selectors & Root Store
src/app/selectors.ts, src/store/rootReducer.ts
Expose isVideoCallScreenCaptureFallbackEnabled in selectPersistableValues; add new reducers to rootReducer so RootState includes new fields.
UI Actions & Settings UI
src/ui/actions.ts, src/ui/components/SettingsView/GeneralTab.tsx, src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx
Add SETTINGS_SET_IS_VIDEO_CALL_SCREEN_CAPTURE_FALLBACK_ENABLED_CHANGED action; add Windows-only ScreenCaptureFallback settings component, render in GeneralTab, and wire dispatch/state/forced description.
Internationalization
src/i18n/*.i18n.json
Add videoCallScreenCaptureFallback i18n entries (title, description, forcedDescription) across locales (en, de-DE, es, fi, fr, hu, no, pt-BR, ru, sv).
Updates Flow
src/updates/common.ts, src/updates/main.ts, src/updates/main.spec.ts
Add isVideoCallScreenCaptureFallbackEnabled to UpdateConfiguration, thread through load/merge/dispatch, and include in defaults/tests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Renderer as Renderer / UI
  participant Store as Redux Store
  participant Main as Electron Main
  participant OS as Windows / RDP

  Note over Main,Store: Startup initialization
  Main->>Store: createMainReduxStore()
  Main->>Main: initializeScreenCaptureFallbackState()
  Main->>OS: detect platform / SESSIONNAME (RDP?)
  Main->>Store: dispatch APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET(true|false)
  alt Windows && (setting enabled or forced)
    Main->>Main: appendSwitch WebRtcAllowWgcDesktopCapturer=false (log JSON reason)
  end
Loading
sequenceDiagram
  autonumber
  actor User
  participant UI as Settings UI
  participant Store as Redux Store
  participant Main as Electron Main
  participant Updates as Updates Loader

  User->>UI: toggle Screen Capture Fallback
  UI-->>Store: dispatch SETTINGS_SET_IS_VIDEO_CALL_SCREEN_CAPTURE_FALLBACK_ENABLED_CHANGED(bool)
  Updates-->>Store: dispatch UPDATES_READY({...isVideoCallScreenCaptureFallbackEnabled})
  Store->>Main: state updated
  alt not RDP and value changed
    Main->>Main: relaunch to apply WGC change
  else RDP
    Main->>Main: log skip relaunch (forced state active)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I twitch my whiskers, flip a tiny switch,
When RDP winds blow, WGC won’t twitch.
I hop through settings, fallback set right—
Screens keep on sharing, calls sleep tight.
A rabbit’s small patch, soft as moonlight. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check ✅ Passed The changes fully implement the objectives of SUP-864 by detecting RDP sessions via SESSIONNAME, disabling Windows Graphics Capture, introducing a “Screen Capture Fallback” toggle with persistence and migration logic, updating the UI and settings flow, and adding relevant documentation to address the video call hang in remote sessions.
Out of Scope Changes Check ✅ Passed All modified files and added assets, including the documentation, settings reducers, actions, UI components, i18n entries, and update flow adjustments, directly relate to the implementation of the Windows Graphics Capture fallback feature as specified in the linked issue and no unrelated functionality has been introduced.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title clearly identifies the primary change as a fix for Jitsi loading freezing in RDP sessions, matching the main objective of the pull request. It's concise, focused on the issue, and avoids unnecessary detail or noise.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/wgc-fallback-toggle

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/app/main/app.ts (1)

68-134: Do not override the existing disable-features switch

Electron only keeps the last value passed to app.commandLine.appendSwitch('disable-features', …). We already set 'HardwareMediaKeyHandling,MediaSessionService' above; this secondary call replaces it with 'WebRtcAllowWgcDesktopCapturer', so the media-session features we meant to keep disabled get silently re-enabled whenever the fallback kicks in. Please consolidate the values into a single invocation.

-  app.commandLine.appendSwitch(
-    'disable-features',
-    'HardwareMediaKeyHandling,MediaSessionService'
-  );
+  const disabledChromiumFeatures = [
+    'HardwareMediaKeyHandling',
+    'MediaSessionService',
+  ];-    if (isScreenCaptureFallbackEnabled || isRdpSession) {
+    if (isScreenCaptureFallbackEnabled || isRdpSession) {-      app.commandLine.appendSwitch(
-        'disable-features',
-        'WebRtcAllowWgcDesktopCapturer'
-      );
+      disabledChromiumFeatures.push('WebRtcAllowWgcDesktopCapturer');
     }
   }
+  app.commandLine.appendSwitch(
+    'disable-features',
+    disabledChromiumFeatures.join(',')
+  );
 };
🧹 Nitpick comments (2)
docs/video-call-window-wgc-limitations.md (1)

23-27: Wrap reference URLs in Markdown links

The bare URLs violate markdownlint rule MD034. Converting them to Markdown links will clear the lint warning and keep the references readable.

-- Microsoft documentation on Windows Graphics Capture limitations (remote sessions unsupported): https://learn.microsoft.com/windows/uwp/audio-video-camera/screen-capture#limitations
-- Chromium WebRTC bug describing the RDP failure with WGC: https://bugs.chromium.org/p/chromium/issues/detail?id=1258686
-- Chromium source comment explaining why WGC only works for interactive sessions: https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/desktop_capture/win/wgc_desktop_frame_capturer_win.cc;l=51-60
-- Electron desktopCapturer docs noting RDP limitations and the need to disable WGC: https://www.electronjs.org/docs/latest/api/desktop-capturer#windows-remote-sessions
-- Electron issue tracker confirming the recommended switch (`disable-features=WebRtcAllowWgcDesktopCapturer`): https://github.com/electron/electron/issues/27411
+- [Microsoft documentation on Windows Graphics Capture limitations (remote sessions unsupported)](https://learn.microsoft.com/windows/uwp/audio-video-camera/screen-capture#limitations)
+- [Chromium WebRTC bug describing the RDP failure with WGC](https://bugs.chromium.org/p/chromium/issues/detail?id=1258686)
+- [Chromium source comment explaining why WGC only works for interactive sessions](https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/desktop_capture/win/wgc_desktop_frame_capturer_win.cc;l=51-60)
+- [Electron desktopCapturer docs noting RDP limitations and the need to disable WGC](https://www.electronjs.org/docs/latest/api/desktop-capturer#windows-remote-sessions)
+- [Electron issue tracker confirming the recommended switch (`disable-features=WebRtcAllowWgcDesktopCapturer`)](https://github.com/electron/electron/issues/27411)
src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts (1)

19-27: Preserve forced state when the settings payload omits the flag

When APP_SETTINGS_LOADED arrives without the new key (older persisted profiles or partial payloads), we currently coerce the missing value to false, overwriting any forced true state (e.g., RDP detection dispatched earlier). Falling back to the existing state avoids that regression while still normalizing truthy payloads.

-    case APP_SETTINGS_LOADED:
-      return Boolean(action.payload.isVideoCallScreenCaptureFallbackEnabled);
+    case APP_SETTINGS_LOADED:
+      return Boolean(
+        action.payload?.isVideoCallScreenCaptureFallbackEnabled ?? state
+      );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a167ecf and 5c92616.

📒 Files selected for processing (25)
  • docs/video-call-window-wgc-limitations.md (1 hunks)
  • src/app/PersistableValues.ts (2 hunks)
  • src/app/actions.ts (2 hunks)
  • src/app/main/app.ts (6 hunks)
  • src/app/reducers/screenCaptureFallbackForced.ts (1 hunks)
  • src/app/selectors.ts (1 hunks)
  • src/i18n/de-DE.i18n.json (1 hunks)
  • src/i18n/en.i18n.json (1 hunks)
  • src/i18n/es.i18n.json (1 hunks)
  • src/i18n/fi.i18n.json (1 hunks)
  • src/i18n/fr.i18n.json (1 hunks)
  • src/i18n/hu.i18n.json (1 hunks)
  • src/i18n/no.i18n.json (1 hunks)
  • src/i18n/pt-BR.i18n.json (1 hunks)
  • src/i18n/ru.i18n.json (1 hunks)
  • src/i18n/sv.i18n.json (1 hunks)
  • src/main.ts (2 hunks)
  • src/store/rootReducer.ts (3 hunks)
  • src/ui/actions.ts (2 hunks)
  • src/ui/components/SettingsView/GeneralTab.tsx (2 hunks)
  • src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx (1 hunks)
  • src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts (1 hunks)
  • src/updates/common.ts (1 hunks)
  • src/updates/main.spec.ts (4 hunks)
  • src/updates/main.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/app/selectors.ts (3)
src/store/rootReducer.ts (1)
  • RootState (105-105)
src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts (1)
  • isVideoCallScreenCaptureFallbackEnabled (12-31)
src/app/reducers/screenCaptureFallbackForced.ts (1)
  • screenCaptureFallbackForced (6-16)
src/ui/components/SettingsView/GeneralTab.tsx (1)
src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx (1)
  • ScreenCaptureFallback (22-72)
src/main.ts (1)
src/app/main/app.ts (1)
  • initializeScreenCaptureFallbackState (137-142)
src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx (5)
src/store/rootReducer.ts (1)
  • RootState (105-105)
src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts (1)
  • isVideoCallScreenCaptureFallbackEnabled (12-31)
src/app/reducers/screenCaptureFallbackForced.ts (1)
  • screenCaptureFallbackForced (6-16)
src/store/actions.ts (1)
  • RootAction (44-46)
src/ui/actions.ts (1)
  • SETTINGS_SET_IS_VIDEO_CALL_SCREEN_CAPTURE_FALLBACK_ENABLED_CHANGED (111-112)
src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts (4)
src/store/actions.ts (1)
  • ActionOf (42-42)
src/ui/actions.ts (1)
  • SETTINGS_SET_IS_VIDEO_CALL_SCREEN_CAPTURE_FALLBACK_ENABLED_CHANGED (111-112)
src/updates/actions.ts (1)
  • UPDATES_READY (11-11)
src/app/actions.ts (1)
  • APP_SETTINGS_LOADED (6-6)
src/app/reducers/screenCaptureFallbackForced.ts (2)
src/store/actions.ts (1)
  • ActionOf (42-42)
src/app/actions.ts (1)
  • APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET (11-12)
src/app/main/app.ts (4)
src/store/readSetting.ts (1)
  • readSetting (6-16)
src/store/index.ts (3)
  • dispatch (38-40)
  • listen (88-114)
  • listen (144-157)
src/app/actions.ts (1)
  • APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET (11-12)
src/ui/actions.ts (1)
  • SETTINGS_SET_IS_VIDEO_CALL_SCREEN_CAPTURE_FALLBACK_ENABLED_CHANGED (111-112)
🪛 markdownlint-cli2 (0.18.1)
docs/video-call-window-wgc-limitations.md

23-23: Bare URL used

(MD034, no-bare-urls)


24-24: Bare URL used

(MD034, no-bare-urls)


25-25: Bare URL used

(MD034, no-bare-urls)


26-26: Bare URL used

(MD034, no-bare-urls)


27-27: Bare URL used

(MD034, no-bare-urls)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: check (windows-latest)
  • GitHub Check: check (ubuntu-latest)
  • GitHub Check: check (macos-latest)
🔇 Additional comments (22)
src/app/PersistableValues.ts (1)

90-173: Migration covers the new fallback flag cleanly

Adding PersistableValues_4_9_0 and the follow-on migration keeps persisted profiles backward compatible while seeding the new flag with a sane default. Nicely done.

src/ui/components/SettingsView/GeneralTab.tsx (1)

12-24: Windows-only toggle placement looks good

Conditionally rendering the fallback toggle behind process.platform === 'win32' keeps the setting scoped to the platform it applies to, and fits cleanly beside the existing hardware acceleration control.

src/app/actions.ts (1)

11-23: Action wiring is solid

The new APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET constant and payload typing integrate smoothly with the existing action map.

src/updates/common.ts (1)

11-12: Update config extended appropriately

Propagating isVideoCallScreenCaptureFallbackEnabled through UpdateConfiguration ensures remote configs can steer the new setting when needed.

src/updates/main.ts (1)

135-207: Fallback flag correctly wired through update payload

Thanks for threading the new flag through both loadConfiguration() and the UPDATES_READY payload so the renderer stays in lockstep with the main-process decision.

src/i18n/ru.i18n.json (1)

203-207: Localization aligns with fallback logic.
Strings mirror the English source and cover both the toggle description and forced state copy, so the UI will render correctly.

src/i18n/sv.i18n.json (1)

230-234: Swedish strings track the new fallback semantics.
The title/description/forcedDescription trio matches the intent of the feature and will localize the Windows-only toggle properly.

src/i18n/fi.i18n.json (1)

209-213: Finnish translation block looks consistent.
Terminology lines up with the English copy, including the forced-state note for RDP sessions.

src/i18n/fr.i18n.json (1)

209-213: French localization matches the feature wording.
Title, description, and forced-state messaging all map cleanly to the new toggle behavior.

src/i18n/no.i18n.json (1)

230-234: Norwegian strings capture the fallback behavior.
Copy mirrors the reference text and explains the forced state for RDP sessions, so the locale stays in sync.

src/i18n/es.i18n.json (1)

221-225: Spanish localization is on point.
Wording covers the fallback rationale and the forced-mode banner just like the source strings.

src/i18n/hu.i18n.json (1)

221-225: Hungarian entry follows the expected pattern.
It brings in all three fields with faithful translations, so the UI toggle will have complete copy.

src/main.ts (2)

5-9: Early initialization keeps forced state in sync.
Importing and calling initializeScreenCaptureFallbackState right after the store exists ensures the RDP-detected override is present before the rest of the startup wiring kicks in.


97-99: Nice touch making the forced flag available up front.
Dispatching the forced-state action immediately after store creation prevents downstream consumers from missing the initial RDP override.

src/ui/actions.ts (2)

111-112: New action constant is well-scoped.

The action naming and payload typing integrate cleanly with the existing settings action conventions.


246-246: Payload map entry lines up correctly.

Good to see the boolean payload registered here—this keeps the reducer wiring type-safe.

src/i18n/en.i18n.json (1)

230-234: Copy covers the new UX states clearly.

The title/description duo plus forced-state note give users the necessary context for the fallback toggle.

src/i18n/de-DE.i18n.json (1)

202-206: Danke!

German locale now mirrors the English messaging, including the forced-state explanation.

src/store/rootReducer.ts (1)

8-102: Reducer registration looks solid.

Both the forced flag and the persisted setting are correctly imported and registered in the root reducer.

src/updates/main.spec.ts (1)

20-117: Tests cover the new configuration field.

App/user merge scenarios now assert the fallback flag is carried through—great coverage.

src/app/reducers/screenCaptureFallbackForced.ts (1)

1-16: Reducer implementation LGTM.

Typed against the specific action and defaulting to false keeps the state predictable.

src/i18n/pt-BR.i18n.json (1)

216-220: Localization update appreciated.

Portuguese copy matches the UX intent, including the forced-session messaging.

…act 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState
Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5c92616 and 9fb2360.

📒 Files selected for processing (3)
  • docs/video-call-window-wgc-limitations.md (1 hunks)
  • src/app/selectors.ts (1 hunks)
  • src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/selectors.ts (2)
src/store/rootReducer.ts (1)
  • RootState (105-105)
src/ui/reducers/isVideoCallScreenCaptureFallbackEnabled.ts (1)
  • isVideoCallScreenCaptureFallbackEnabled (12-31)
🪛 markdownlint-cli2 (0.18.1)
docs/video-call-window-wgc-limitations.md

29-29: Bare URL used

(MD034, no-bare-urls)


30-30: Bare URL used

(MD034, no-bare-urls)


31-31: Bare URL used

(MD034, no-bare-urls)


32-32: Bare URL used

(MD034, no-bare-urls)


33-33: Bare URL used

(MD034, no-bare-urls)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: check (macos-latest)
  • GitHub Check: check (windows-latest)
  • GitHub Check: check (ubuntu-latest)
🔇 Additional comments (1)
src/app/selectors.ts (1)

68-70: Good call on persisting only the user-controlled toggle.

Keeping screenCaptureFallbackForced out of the persistable payload while persisting isVideoCallScreenCaptureFallbackEnabled addresses the earlier regression risk and maintains the intended session-only behavior.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
src/app/main/app.ts (6)

42-42: Avoid module-level mutable state; compute on demand.
The global isScreenCaptureFallbackForced creates ordering coupling with initializeScreenCaptureFallbackState. Prefer deriving the forced state at dispatch time.

Apply this diff to remove the global:

-let isScreenCaptureFallbackForced = false;

113-120: Tighten RDP detection to avoid false positives.
Using “anything not Console” may misclassify non-Console sessions. Check for RDP explicitly (SESSIONNAME usually starts with “RDP-”).

-    const isRdpSession =
-      typeof sessionName === 'string' && sessionName !== 'Console';
+    const isRdpSession =
+      typeof sessionName === 'string' && /^RDP/i.test(sessionName);

118-118: Remove redundant assignment tied to module state.
If you compute forced state on demand (see suggestions), this write is unnecessary.

-    isScreenCaptureFallbackForced = isRdpSession;

100-100: Remove redundant reset.
If the module-level variable is removed, this line becomes dead. Even if kept, it resets to the same initial value.

-  isScreenCaptureFallbackForced = false;

141-146: Derive forced state at dispatch time to eliminate ordering hazards.
This avoids relying on earlier mutation and guarantees correctness even if init order changes.

-export const initializeScreenCaptureFallbackState = (): void => {
-  dispatch({
-    type: APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET,
-    payload: isScreenCaptureFallbackForced,
-  });
-};
+export const initializeScreenCaptureFallbackState = (): void => {
+  const sessionName = process.env.SESSIONNAME;
+  const isRdpSession =
+    process.platform === 'win32' &&
+    typeof sessionName === 'string' &&
+    /^RDP/i.test(sessionName);
+  dispatch({
+    type: APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET,
+    payload: isRdpSession,
+  });
+};

180-201: Simplify relaunch logic; avoid read-after-write race on persisted settings.
Given the action is “...changed”, you can skip re-reading the file and relaunch unless in RDP.

-    (action) => {
-      const newSettingValue = action.payload;
-      const currentPersistedSetting = readSetting(
-        'isVideoCallScreenCaptureFallbackEnabled'
-      );
-      const sessionName = process.env.SESSIONNAME;
-      const isRdpSession =
-        typeof sessionName === 'string' && sessionName !== 'Console';
-
-      // Relaunch only if the setting actually changes AND it's not already forced by RDP
-      if (newSettingValue !== currentPersistedSetting && !isRdpSession) {
-        relaunchApp();
-      } else if (isRdpSession) {
-        console.log(
-          'Screen Capture Fallback setting changed, but app is in RDP session. Skipping relaunch.'
-        );
-      }
-    }
+    () => {
+      const sessionName = process.env.SESSIONNAME;
+      const isRdpSession =
+        typeof sessionName === 'string' && /^RDP/i.test(sessionName);
+      if (!isRdpSession) {
+        relaunchApp();
+      } else {
+        console.log(
+          'Screen Capture Fallback setting changed during RDP session; will take effect on next launch.'
+        );
+      }
+    }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9fb2360 and a58880b.

📒 Files selected for processing (1)
  • src/app/main/app.ts (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/main/app.ts (4)
src/store/readSetting.ts (1)
  • readSetting (6-16)
src/store/index.ts (3)
  • dispatch (38-40)
  • listen (88-114)
  • listen (144-157)
src/app/actions.ts (1)
  • APP_SCREEN_CAPTURE_FALLBACK_FORCED_SET (11-12)
src/ui/actions.ts (1)
  • SETTINGS_SET_IS_VIDEO_CALL_SCREEN_CAPTURE_FALLBACK_ENABLED_CHANGED (111-112)
🔇 Additional comments (7)
src/app/main/app.ts (7)

19-20: New settings action import looks correct.
Matches PR intent and wiring to settings change listener below.


29-30: New app action import looks correct.
Required for dispatching forced-state to the store.


69-73: Consolidating disabled Chromium features is good.
Single disable-features append reduces override risk and keeps flags centralized.


96-99: Reading the new setting at startup is fine.
readSetting’s null fallback is safe; the conditionals below handle it properly.


134-139: Confirm there are no other disable-features calls.
Multiple disable-features switches can override each other; ensure this is the only place appending the switch.

Use the script in the previous comment (step 2) to verify.


141-146: Init order correct: performElectronStartup() runs before initializeScreenCaptureFallbackState() in src/main.ts, so the dispatched forced state is accurate.


120-131: Verified feature flag: WebRtcAllowWgcDesktopCapturer is correct for Electron v37.2.4 per official docs and issue tracker.

@jeanfbrito jeanfbrito changed the title Jitsi loading freezes in RDP sessions fix: Jitsi loading freezes in RDP sessions Sep 25, 2025
@jeanfbrito jeanfbrito merged commit 3cdfb16 into master Sep 26, 2025
11 of 12 checks passed
@jeanfbrito jeanfbrito deleted the feature/wgc-fallback-toggle branch September 26, 2025 19:21
sreeja2007 pushed a commit to sreeja2007/Rocket.Chat.Electron that referenced this pull request Mar 2, 2026
* feat(windows): add screen capture fallback toggle

* chore(i18n): translate screen capture fallback setting

* feat(settings): add screen capture fallback toggle

* feat(windows): disable WGC when fallback toggle is active

* fix(windows): skip relaunch when fallback already forced

* feat(settings): show when screen capture fallback is forced

* fix(windows): prevent restart in RDP when toggle changes

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.

* fix(startup): dispatch fallback state after store initialization

Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.

* docs: update WGC limitations with implemented solution

Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment

* fix(settings): prevent screenCaptureFallbackForced persistence and React 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState

* fix(startup): consolidate disable-features switch to prevent override

Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Ram-sah19 pushed a commit to Ram-sah19/Rocket.Chat.Electron that referenced this pull request Mar 10, 2026
* feat(windows): add screen capture fallback toggle

* chore(i18n): translate screen capture fallback setting

* feat(settings): add screen capture fallback toggle

* feat(windows): disable WGC when fallback toggle is active

* fix(windows): skip relaunch when fallback already forced

* feat(settings): show when screen capture fallback is forced

* fix(windows): prevent restart in RDP when toggle changes

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.

* fix(startup): dispatch fallback state after store initialization

Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.

* docs: update WGC limitations with implemented solution

Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment

* fix(settings): prevent screenCaptureFallbackForced persistence and React 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState

* fix(startup): consolidate disable-features switch to prevent override

Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Ram-sah19 pushed a commit to Ram-sah19/Rocket.Chat.Electron that referenced this pull request Mar 10, 2026
* feat(windows): add screen capture fallback toggle

* chore(i18n): translate screen capture fallback setting

* feat(settings): add screen capture fallback toggle

* feat(windows): disable WGC when fallback toggle is active

* fix(windows): skip relaunch when fallback already forced

* feat(settings): show when screen capture fallback is forced

* fix(windows): prevent restart in RDP when toggle changes

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.

* fix(startup): dispatch fallback state after store initialization

Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.

* docs: update WGC limitations with implemented solution

Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment

* fix(settings): prevent screenCaptureFallbackForced persistence and React 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState

* fix(startup): consolidate disable-features switch to prevent override

Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Ram-sah19 pushed a commit to Ram-sah19/Rocket.Chat.Electron that referenced this pull request Mar 10, 2026
* feat(windows): add screen capture fallback toggle

* chore(i18n): translate screen capture fallback setting

* feat(settings): add screen capture fallback toggle

* feat(windows): disable WGC when fallback toggle is active

* fix(windows): skip relaunch when fallback already forced

* feat(settings): show when screen capture fallback is forced

* fix(windows): prevent restart in RDP when toggle changes

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.

* fix(startup): dispatch fallback state after store initialization

Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.

* docs: update WGC limitations with implemented solution

Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment

* fix(settings): prevent screenCaptureFallbackForced persistence and React 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState

* fix(startup): consolidate disable-features switch to prevent override

Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Ram-sah19 pushed a commit to Ram-sah19/Rocket.Chat.Electron that referenced this pull request Mar 10, 2026
* feat(windows): add screen capture fallback toggle

* chore(i18n): translate screen capture fallback setting

* feat(settings): add screen capture fallback toggle

* feat(windows): disable WGC when fallback toggle is active

* fix(windows): skip relaunch when fallback already forced

* feat(settings): show when screen capture fallback is forced

* fix(windows): prevent restart in RDP when toggle changes

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.

* fix(startup): dispatch fallback state after store initialization

Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.

* docs: update WGC limitations with implemented solution

Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment

* fix(settings): prevent screenCaptureFallbackForced persistence and React 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState

* fix(startup): consolidate disable-features switch to prevent override

Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Ram-sah19 pushed a commit to Ram-sah19/Rocket.Chat.Electron that referenced this pull request Mar 10, 2026
* feat(windows): add screen capture fallback toggle

* chore(i18n): translate screen capture fallback setting

* feat(settings): add screen capture fallback toggle

* feat(windows): disable WGC when fallback toggle is active

* fix(windows): skip relaunch when fallback already forced

* feat(settings): show when screen capture fallback is forced

* fix(windows): prevent restart in RDP when toggle changes

The listener was using a startup-time flag instead of checking RDP
environment when the action is triggered. Now properly detects RDP
session at action time and skips relaunch accordingly.

* fix(startup): dispatch fallback state after store initialization

Moved dispatch calls from performElectronStartup to after store
is created to prevent 'Cannot read properties of undefined' error.
Created initializeScreenCaptureFallbackState function called after
createMainReduxStore in the startup sequence.

* docs: update WGC limitations with implemented solution

Updated documentation to reflect the current working implementation:
- Windows-only toggle in Settings → General
- Automatic RDP detection and enforcement
- Smart restart behavior that avoids unnecessary restarts in RDP
- UI feedback when fallback is enforced by environment

* fix(settings): prevent screenCaptureFallbackForced persistence and React 17 compatibility

- Remove screenCaptureFallbackForced from selectPersistableValues to keep it session-only
- Replace useId with useMemo-based stable ID for React 17.x compatibility
- Ensures fallback forced flag is derived at runtime via initializeScreenCaptureFallbackState

* fix(startup): consolidate disable-features switch to prevent override

Electron only keeps the last value passed to appendSwitch('disable-features').
Previously we called it twice:
1. 'HardwareMediaKeyHandling,MediaSessionService'
2. 'WebRtcAllowWgcDesktopCapturer' (when fallback enabled)

The second call would override the first, silently re-enabling
media session features. Now we collect all disabled features
in an array and apply them in a single consolidated call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant