fix: Freezing video call window on loading inside RDP sessions#3143
fix: Freezing video call window on loading inside RDP sessions#3143jeanfbrito merged 25 commits intomasterfrom
Conversation
…ssions - Implemented comprehensive disabling of Windows Graphics Capture (WGC) features to ensure fallback to the Desktop Duplication API during RDP sessions. - Updated smart restart behavior to provide clearer user guidance when toggling settings in RDP environments. - Enhanced UI to reflect the current state of WGC settings, including disabling the toggle when in RDP sessions. - Added technical details on the implementation of disabled Chrome features and their impact on screen capture reliability. This update improves the user experience and ensures consistent behavior across different session types.
WalkthroughRefactors the video call renderer to a vanilla-JS bootstrap with deferred React, implements a stale‑while‑revalidate desktop‑capturer cache and prewarm IPC, extends WGC fallback to disable additional WebRTC flags at app and webview levels, and adds notification categories with enriched click payloads and download tagging. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant AppMain as App (main)
participant VideoWin as VideoCallWindow (renderer)
participant IPCMain as IPC handler (main)
participant Capturer as DesktopCapturerCache (main)
participant Webview as WebView (renderer / webcontents)
AppMain->>VideoWin: create BrowserWindow (maybe add additionalArguments to disable WGC)
VideoWin->>IPCMain: request capturer sources / prewarm (ipc)
IPCMain->>Capturer: getCachedSources()
alt cache fresh
Capturer-->>IPCMain: cached sources
else cache stale
Capturer-->>IPCMain: stale sources (returned) and triggers background refresh
Capturer->>Capturer: refreshDesktopCapturerCache() (deduplicated)
end
IPCMain-->>VideoWin: sources
VideoWin->>Webview: create / attach webview (with preload path)
VideoWin->>Webview: show screen picker (mount/show)
Note over AppMain,Webview: Notification clicks routed via preload -> main -> App store with category and serverUrl
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas needing extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
jeanfbrito
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/video-call-window-wgc-limitations.md (1)
62-73: Consider formatting URLs for better markdown compliance.The static analysis tool flags bare URLs that could be formatted as proper markdown links for improved readability and lint compliance.
As per static analysis hints, consider converting bare URLs to markdown link syntax for better readability:
### Windows Graphics Capture Limitations -- 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 +- [Microsoft documentation on Windows Graphics Capture limitations](https://learn.microsoft.com/windows/uwp/audio-video-camera/screen-capture#limitations) (remote sessions unsupported) +- [Chromium WebRTC bug describing the RDP failure with WGC](https://bugs.chromium.org/p/chromium/issues/detail?id=1258686) ... (apply same pattern to remaining URLs)This resolves the MD034 warnings and makes the references easier to scan.
📜 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.
📒 Files selected for processing (5)
docs/video-call-window-wgc-limitations.md(1 hunks)src/app/main/app.ts(2 hunks)src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx(1 hunks)src/videoCallWindow/ipc.ts(2 hunks)src/videoCallWindow/screenSharePicker.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/components/SettingsView/features/ScreenCaptureFallback.tsxsrc/app/main/app.tssrc/videoCallWindow/ipc.tssrc/videoCallWindow/screenSharePicker.tsx
src/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/ui/**/*.tsx: Implement React UI with functional components and hooks
Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from@rocket.chat/fuselageinstead of raw HTML elements
For Fuselage theming, validate tokens againstTheme.d.tsand only use documented values
Name React component files in PascalCase; non-component files should follow camelCase naming
Files:
src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/video-call-window-wgc-limitations.md
🧠 Learnings (2)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
src/videoCallWindow/screenSharePicker.tsx
🧬 Code graph analysis (1)
src/videoCallWindow/ipc.ts (1)
src/store/readSetting.ts (1)
readSetting(6-16)
🪛 markdownlint-cli2 (0.18.1)
docs/video-call-window-wgc-limitations.md
62-62: Bare URL used
(MD034, no-bare-urls)
63-63: Bare URL used
(MD034, no-bare-urls)
64-64: Bare URL used
(MD034, no-bare-urls)
65-65: Bare URL used
(MD034, no-bare-urls)
66-66: Bare URL used
(MD034, no-bare-urls)
67-67: Bare URL used
(MD034, no-bare-urls)
70-70: Bare URL used
(MD034, no-bare-urls)
71-71: 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 (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
🔇 Additional comments (8)
src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx (1)
64-69: LGTM! Toggle behavior correctly reflects forced state.The implementation properly handles RDP-detected scenarios by showing the toggle as checked and disabled when
isFallbackForcedis true. This provides clear visual feedback that WGC is disabled by RDP detection and cannot be manually changed.src/videoCallWindow/screenSharePicker.tsx (2)
247-256: Improved semantic token usage for selection states.The conditional token selection based on
selectedSourceIdprovides clearer semantic meaning. Usingsurface-selected/font-on-darkfor selected items andsurface-tint/font-defaultfor unselected items is more maintainable than the previous approach.
171-172: Theme tokens verified—no issues found.The tokens
surface-lightandfont-defaultare valid, documented Fuselage theme tokens from version 0.58.0. Both are actively used throughout the codebase in similar contexts, confirming their correctness.src/app/main/app.ts (2)
120-132: LGTM! Comprehensive WGC disabling implemented correctly.Adding
WebRtcAllowWgcScreenCaptureralongsideWebRtcAllowWgcDesktopCapturerensures complete fallback to the Desktop Duplication API. This dual-flag approach is necessary because Chromium's WebRTC implementation uses different code paths for desktop and screen capturing, as documented in the accompanying technical documentation.
200-207: Improved log message clarity for RDP scenarios.The expanded log message now clearly explains that setting changes in RDP sessions will apply to future local launches and that no restart is needed immediately since WGC is already disabled by RDP detection. This provides better operational visibility.
src/videoCallWindow/ipc.ts (2)
426-446: LGTM! Window-level WGC disabling implemented correctly.The code properly implements the second layer of WGC disabling by:
- Detecting RDP sessions using the same logic as app-level detection
- Dynamically checking the user's screen capture fallback setting
- Conditionally injecting the disable flags via
additionalArgumentsThe dynamic import of
readSettingis appropriate here to avoid potential circular dependencies. The dual-flag disablement of bothWebRtcAllowWgcDesktopCapturerandWebRtcAllowWgcScreenCaptureraligns with the comprehensive approach documented in the technical documentation.
464-466: Clean conditional spreading of additionalArguments.The short-circuit evaluation ensures
additionalArgumentsis only added towebPreferenceswhen there are actual arguments to pass, avoiding unnecessary empty arrays. This is a clean and idiomatic approach.docs/video-call-window-wgc-limitations.md (1)
23-57: Excellent technical documentation.The documentation clearly explains:
- The comprehensive dual-flag WGC disabling approach and its rationale
- Smart restart behavior for different scenarios (local vs RDP)
- Technical implementation details with specific code locations
- The two-layer enforcement strategy (app-level and window-level)
This provides valuable context for future maintainers and aligns perfectly with the implemented code changes.
- Changed the output directory for preload files from 'app' to 'app/preload' in the Rollup configuration. This adjustment improves project structure and organization for better clarity in file management.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rollup.config.mjs (1)
135-143: Preload output isolation toapp/preloadlooks correctRouting the videoCallWindow preload bundle to
app/preloadis a sensible separation of preload assets and should work fine from a Rollup perspective. Just ensure the BrowserWindowpreloadpath and any packaging (electron-builder) config are updated to point at this new subdirectory so there are no runtime “file not found” issues.Based on learnings, this keeps the root Rollup config aligned with the dedicated preload structure.
📜 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.
📒 Files selected for processing (1)
rollup.config.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Files:
rollup.config.mjs
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
rollup.config.mjs
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to {rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js} : Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Applied to files:
rollup.config.mjs
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
rollup.config.mjs
⏰ 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). (6)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
…allback - Revised the forced description for the video call screen capture fallback to enhance clarity and user understanding regarding its behavior in RDP sessions. The new translation provides a more accurate explanation of the feature's functionality and its impact on local launches.
- Reformatted the `loadFile` method call in `ipc.ts` for better readability. - Removed unnecessary blank line in `video-call-window.tsx`. - Adjusted visibility logic in `videoCallWindow.tsx` for improved clarity.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/videoCallWindow/screenSharePicker.tsx (1)
88-92: IPC listener leak - missing cleanup and unnecessary dependency.The
visibledependency causes this effect to re-run on every visibility change, adding duplicate IPC listeners without cleanup. This leads to memory leaks and multiple handler invocations.Apply this diff to fix the listener leak:
useEffect(() => { - ipcRenderer.on('video-call-window/open-screen-picker', () => { + const handleOpenScreenPicker = () => { setVisible(true); - }); -}, [visible]); + }; + ipcRenderer.on('video-call-window/open-screen-picker', handleOpenScreenPicker); + return () => { + ipcRenderer.removeListener('video-call-window/open-screen-picker', handleOpenScreenPicker); + }; +}, []);
🧹 Nitpick comments (2)
src/ui/components/SettingsView/SettingsView.tsx (1)
74-79: Consider addingverticalprop for consistent scroll behavior.Good use of Fuselage's
Scrollablecomponent. However, other usages in this PR (e.g.,screenSharePicker.tsxline 213) specify<Scrollable vertical>for vertical-only scrolling. Without the prop, Scrollable may enable both horizontal and vertical scrolling.- <Scrollable> + <Scrollable vertical> <Box m='x24'> {(currentTab === 'general' && <GeneralTab />) || (currentTab === 'certificates' && <CertificatesTab />)} </Box> </Scrollable>src/videoCallWindow/videoCallWindow.tsx (1)
695-705: Consider using refs to reduce effect re-runs.This effect has 9 dependencies, causing it to re-run frequently and re-attach all webview event listeners on every state change. While functionally correct, this is inefficient.
Consider using refs for state values that handlers need to check (like
isFailed,isLoading,isClosing) instead of including them in the dependency array. This would allow the effect to run once while handlers access current values:const isLoadingRef = useRef(isLoading); useEffect(() => { isLoadingRef.current = isLoading; }, [isLoading]); // Then in handlers: if (isLoadingRef.current) { ... }This pattern reduces unnecessary event listener teardown/setup cycles.
📜 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.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
package.json(1 hunks)src/downloads/main.ts(1 hunks)src/notifications/preload.ts(2 hunks)src/ui/components/SettingsView/SettingsView.tsx(2 hunks)src/ui/components/SettingsView/features/AvailableBrowsers.tsx(3 hunks)src/videoCallWindow/ipc.ts(4 hunks)src/videoCallWindow/screenSharePicker.tsx(1 hunks)src/videoCallWindow/video-call-window.tsx(1 hunks)src/videoCallWindow/videoCallWindow.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/screenSharePicker.tsxsrc/ui/components/SettingsView/SettingsView.tsxsrc/downloads/main.tssrc/ui/components/SettingsView/features/AvailableBrowsers.tsxsrc/videoCallWindow/video-call-window.tsxsrc/videoCallWindow/videoCallWindow.tsxsrc/notifications/preload.tssrc/videoCallWindow/ipc.ts
src/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/ui/**/*.tsx: Implement React UI with functional components and hooks
Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from@rocket.chat/fuselageinstead of raw HTML elements
For Fuselage theming, validate tokens againstTheme.d.tsand only use documented values
Name React component files in PascalCase; non-component files should follow camelCase naming
Files:
src/ui/components/SettingsView/SettingsView.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.tsx
🧠 Learnings (6)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from `rocket.chat/fuselage` instead of raw HTML elements
Applied to files:
src/ui/components/SettingsView/SettingsView.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Implement React UI with functional components and hooks
Applied to files:
src/ui/components/SettingsView/SettingsView.tsxsrc/ui/components/SettingsView/features/AvailableBrowsers.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
src/ui/components/SettingsView/features/AvailableBrowsers.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/videoCallWindow/video-call-window.tsxsrc/videoCallWindow/ipc.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/video-call-window.tsxsrc/videoCallWindow/ipc.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/notifications/preload.ts
🧬 Code graph analysis (3)
src/ui/components/SettingsView/features/AvailableBrowsers.tsx (4)
src/store/index.ts (1)
dispatch(38-40)src/ui/actions.ts (1)
SETTINGS_SELECTED_BROWSER_CHANGED(119-120)src/ui/reducers/selectedBrowser.ts (1)
selectedBrowser(11-27)src/ui/reducers/availableBrowsers.ts (1)
availableBrowsers(11-26)
src/notifications/preload.ts (3)
src/store/index.ts (1)
dispatch(38-40)src/ui/actions.ts (2)
SIDE_BAR_DOWNLOADS_BUTTON_CLICKED(44-45)WEBVIEW_FOCUS_REQUESTED(68-68)src/servers/preload/urls.ts (1)
getServerUrl(9-9)
src/videoCallWindow/ipc.ts (1)
src/store/readSetting.ts (1)
readSetting(6-16)
🪛 ESLint
src/videoCallWindow/videoCallWindow.tsx
[error] 865-865: Insert ⏎···········
(prettier/prettier)
src/videoCallWindow/ipc.ts
[error] 667-667: Insert ⏎········
(prettier/prettier)
[error] 668-668: Replace ········ with ··········
(prettier/prettier)
[error] 669-669: Insert ··
(prettier/prettier)
[error] 669-669: Expected property shorthand.
(object-shorthand)
[error] 670-670: Insert ··
(prettier/prettier)
[error] 671-671: Insert ··
(prettier/prettier)
[error] 672-672: Replace ······}) with ········})⏎········
(prettier/prettier)
🪛 GitHub Check: build (ubuntu-latest, linux)
src/videoCallWindow/ipc.ts
[failure] 672-672:
Replace ······***) with ········***)⏎········
[failure] 671-671:
Insert ··
[failure] 670-670:
Insert ··
[failure] 669-669:
Expected property shorthand
[failure] 669-669:
Insert ··
[failure] 668-668:
Replace ········ with ··········
[failure] 667-667:
Insert ⏎········
🪛 GitHub Check: check (macos-latest)
src/videoCallWindow/ipc.ts
[failure] 672-672:
Replace ······}) with ········})⏎········
[failure] 671-671:
Insert ··
[failure] 670-670:
Insert ··
[failure] 669-669:
Expected property shorthand
[failure] 669-669:
Insert ··
[failure] 668-668:
Replace ········ with ··········
[failure] 667-667:
Insert ⏎········
🪛 GitHub Check: check (ubuntu-latest)
src/videoCallWindow/ipc.ts
[failure] 672-672:
Replace ······}) with ········})⏎········
[failure] 671-671:
Insert ··
[failure] 670-670:
Insert ··
[failure] 669-669:
Expected property shorthand
[failure] 669-669:
Insert ··
[failure] 668-668:
Replace ········ with ··········
[failure] 667-667:
Insert ⏎········
⏰ 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: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
🔇 Additional comments (8)
src/downloads/main.ts (1)
78-100: Verify user feedback mechanism for completed downloads.Removing the download completion notification eliminates visual feedback when downloads finish. Users may not be aware of completed downloads unless they actively check the downloads UI.
Confirm that:
- The downloads UI provides clear visual indicators for completed downloads
- Users can easily discover when downloads finish without notifications
- This change aligns with the intended UX design for download management
If immediate feedback is still desired, consider restoring a subtle notification or in-app indicator for download completion.
src/videoCallWindow/screenSharePicker.tsx (1)
171-171: LGTM - theming updates appear intentional.The background color changes from
surfacetolightand the use ofselected/lightfor thumbnail tiles align with the broader UI consistency improvements in this PR. The tokens are valid for Fuselage 0.58.0.Also applies to: 247-247
src/ui/components/SettingsView/features/AvailableBrowsers.tsx (1)
49-58: LGTM - Select component migration is correct.The migration from
SelectLegacytoSelectproperly handles theKeytype from the onChange callback by converting to string before dispatching. The'system' → nullmapping aligns with the reducer's expectedstring | nullpayload type.src/videoCallWindow/video-call-window.tsx (1)
185-200: LGTM - Simplified initialization by removing URL negotiation.The removal of the IPC-based URL request flow is a good simplification. URL parameters are now passed directly via query string (handled in
videoCallWindow.tsx), eliminating the roundtrip and retry logic that was previously needed.src/videoCallWindow/videoCallWindow.tsx (1)
39-50: LGTM - URL initialization from query parameters.Initializing
videoCallUrlandshouldAutoOpenDevtoolsfrom query parameters is a clean approach that eliminates the need for IPC-based URL negotiation. Setting initial loading states based on URL presence prevents the grey screen flash appropriately.package.json (1)
116-116: No critical compatibility issues identified with Electron 39.2.3 upgrade.The three breaking changes in Electron 39 do not negatively impact this codebase:
- Paint event data structure changes: No paint event listeners found in the codebase.
- window.open popup resizing (now always resizable per WHATWG spec): Current code does not attempt to restrict popup resizing in
setWindowOpenHandler()return values, so it's compatible with the new behavior. BrowserWindow creation explicitly specifies dimensions, which works with the change.- --host-rules deprecation: No usage of this flag in the codebase.
The existing
setWindowOpenHandler()implementations (videoCallWindow/ipc.ts, rootWindow.ts, serverView/index.ts) already use the Electron v39-compatible pattern of returning only{ action: 'allow'/'deny' }without window sizing options.src/videoCallWindow/ipc.ts (2)
387-392: Windows WGC fallback flags & devtools state wiring look coherent
- Mapping
isAutoOpenEnabledfromstate.isVideoCallDevtoolsAutoOpenEnabledinto the localstateobject and then using it for both the query string and IPC URL response keeps the devtools auto-open behavior centralized and consistent.- The
additionalArgslogic is sensibly gated towin32, usesSESSIONNAMEand the persistedisVideoCallScreenCaptureFallbackEnabledflag to decide when to disable WGC, and only addsadditionalArgumentswhen needed, so non-Windows and non-fallback paths stay unaffected.- The conditional spread into
webPreferencesavoids polluting the options on platforms where no extra arguments are required.No functional issues spotted in this block; behavior matches the PR’s objective of enforcing WGC disablement for RDP/fallback scenarios while keeping the rest of the flow untouched.
Also applies to: 427-447, 455-467
667-677: I'll verify the review comment by examining the actual code and checking the project's linting configuration.
<function_calls>
<function_calls_item>
shell
find . -name "ipc.ts" -path "/videoCallWindow/" | head -5
</function_calls_item>
</function_calls>Let me search for the file:
<function_calls>
<function_calls_item>
shell
fd -type f -name "ipc.ts" | grep videoCallWindow
</function_calls_item>
</function_calls><function_calls>
shell
cat -n src/videoCallWindow/ipc.ts | sed -n '660,680p'
</function_calls_item>
</function_calls><function_calls>
shell
wc -l src/videoCallWindow/ipc.ts
</function_calls_item>
</function_calls><function_calls>
shell
ls -la src/videoCallWindow/
</function_calls_item>
</function_calls><function_calls>
shell
find . -type f -name "*.ts" | grep -i ipc
</function_calls_item>
</function_calls><function_calls>
shell
find src -type d | head -20
</function_calls_item>
</function_calls><function_calls>
shell
git ls-files | grep -i ipc
</function_calls_item>
</function_calls><function_calls>
shell
git ls-files | grep videoCall
</function_calls_item>
</function_calls><function_calls>
shell
git ls-files src/ | head -30
</function_calls_item>
</function_calls><function_calls>
shell
git branch -vv
</function_calls_item>
</function_calls><function_calls>
shell
git log --oneline -10
</function_calls_item>
</function_calls><function_calls>
shell
pwd && ls -la
</function_calls_item>
</function_calls><function_calls>
shell
git status
</function_calls_item>
</function_calls>Let me search for the videoCallWindow file structure more broadly:
<function_calls>
shell
git log --all --full-history --source -- "src/videoCallWindow/ipc.ts" | head -20
</function_calls_item>
</function_calls><function_calls>
shell
git show HEAD:src/videoCallWindow/ipc.ts 2>/dev/null | head -50 || echo "File not in HEAD"
</function_calls_item>
</function_calls>Let me check what the current repository state is and look for ESLint/Prettier configuration:
<function_calls>
shell
ls -la | grep -E "eslint|prettier|package"
</function_calls_item>
</function_calls><function_calls>
shell
cat package.json 2>/dev/null | grep -A5 -B5 "eslint|prettier" | head -30
</function_calls_item>
</function_calls><function_calls>
shell
cat .eslintrc* 2>/dev/null || cat eslint.config.* 2>/dev/null || echo "No ESLint config found in root"
</function_calls_item>
</function_calls><function_calls>
shell
cat .prettierrc* 2>/dev/null || echo "No Prettier config found"
</function_calls_item>
</function_calls>Let me try a different approach and check if there's a PR or branch reference available:
<function_calls>
shell
git diff --name-only HEAD^ HEAD 2>/dev/null | head -20 || git diff --name-only --cached | head -20
</function_calls_item>
</function_calls><function_calls>
shell
git log --all --oneline --graph | head -20
</function_calls_item>
</function_calls>Let me check for any recent changes or patches related to the code:
<function_calls>
shell
rg "videoCallWindow.loadFile" -A5 -B2 2>/dev/null || echo "Pattern not found"
</function_calls_item>
</function_calls><function_calls>
shell
rg "loadFile" -B2 -A8 2>/dev/null | head -50
</function_calls_item>
</function_calls>Let me check for ESLint rules related to object shorthand:
<function_calls>
shell
find . -type f ( -name ".eslintrc*" -o -name "eslint.config.*" -o -name "package.json" ) -exec grep -l "object-shorthand" {} ;
</function_calls_item>
</function_calls><function_calls>
shell
rg "object-shorthand" . 2>/dev/null
</function_calls_item>
</function_calls>Let me check if there's a TypeScript file with the code pattern mentioned in the review:
<function_calls>
shell
rg "url: url" -B5 -A10 2>/dev/null
</function_calls_item>
</function_calls><function_calls>
shell
rg "Failed to load HTML file" -B5 -A5 2>/dev/null
</function_calls_item>
</function_calls><function_calls>
shell
git branch -a
</function_calls_item>
</function_calls><function_calls>
shell
git log --all --name-status --oneline | grep -i "ipc|videocall" | head -20
</function_calls_item>
</function_calls><function_calls>
shell
git log --all --full-history --diff-filter=A -- "*/videoCallWindow/ipc.ts" 2>/dev/null | head -20
</function_calls_item>
</function_calls>Let me check if there are any TypeScript files in the repository at all:
<function_calls>
shell
find . -name ".ts" -o -name ".tsx" | head -30
</function_calls_item>
</function_calls><function_calls>
shell
ls -la
</function_calls_item>
</function_calls><function_calls>
shell
git remote -v
</function_calls_item>
</function_calls>Let me try to understand the repository structure better:
<function_calls>
shell
cat .git/config 2>/dev/null | head -20
</function_calls_item>
</function_calls><function_calls>
shell
git describe --all 2>/dev/null
</function_calls_item>
</function_calls>Based on my verification attempts, I'm unable to locate the file
src/videoCallWindow/ipc.tsor the code snippet referenced in the review comment within the current repository state. The repository either hasn't been properly cloned with the necessary branch, or the file doesn't exist in the current checkout.Unable to verify: Code file not found in repository
The review comment references specific code at
src/videoCallWindow/ipc.tslines 667-677, but this file and the associated code pattern (url: urlshorthand issue and Promise chaining suggestion) could not be located through multiple search strategies in the sandbox environment. Without access to the actual code and the project's ESLint/Prettier configuration, I cannot verify whether:
- The
object-shorthandESLint rule violations actually exist- The suggested formatting changes comply with project standards
- Similar patterns elsewhere in the codebase use the proposed chaining style
To complete verification, the code file needs to be accessible in the repository.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/videoCallWindow/videoCallWindow.tsx (2)
39-46: Consider parsing URL params once to avoid redundant parsing.The
URLSearchParamsobject is created twice during initialization. This is a minor inefficiency that only runs once at mount, but could be consolidated.- const [videoCallUrl, setVideoCallUrl] = useState(() => { - const params = new URLSearchParams(window.location.search); - return params.get('url') || ''; - }); - const [shouldAutoOpenDevtools, setShouldAutoOpenDevtools] = useState(() => { - const params = new URLSearchParams(window.location.search); - return params.get('autoOpenDevtools') === 'true'; - }); + const [videoCallUrl, setVideoCallUrl] = useState(() => { + const params = new URLSearchParams(window.location.search); + return params.get('url') || ''; + }); + const [shouldAutoOpenDevtools, setShouldAutoOpenDevtools] = useState(() => { + // Reuses the same parsing approach - acceptable for one-time initialization + const params = new URLSearchParams(window.location.search); + return params.get('autoOpenDevtools') === 'true'; + });Alternatively, extract params parsing outside the state initializers:
const initialParams = new URLSearchParams(window.location.search); const [videoCallUrl, setVideoCallUrl] = useState(initialParams.get('url') || ''); const [shouldAutoOpenDevtools, setShouldAutoOpenDevtools] = useState( initialParams.get('autoOpenDevtools') === 'true' );The current approach is acceptable given it only runs once at mount.
695-705: Consider using refs for state values in event handlers to reduce effect re-runs.The dependency array includes many state variables, causing the effect to re-run and re-register all webview event listeners on each state change. This is a pre-existing pattern, but for future improvement, consider using
useRefto store the latest values of frequently-changing states (isLoading,isFailed, etc.) that handlers read, allowing you to remove them from the dependency array.This is outside the scope of the current PR but worth noting for future maintenance.
📜 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.
📒 Files selected for processing (3)
src/videoCallWindow/ipc.ts(4 hunks)src/videoCallWindow/video-call-window.tsx(1 hunks)src/videoCallWindow/videoCallWindow.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/videoCallWindow/ipc.ts
- src/videoCallWindow/video-call-window.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/videoCallWindow.tsx
⏰ 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). (6)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
🔇 Additional comments (2)
src/videoCallWindow/videoCallWindow.tsx (2)
49-50: LGTM!The initialization of
isLoadingandshowLoadingbased on URL presence is correct. Starting in loading state when a URL is present prevents the grey screen flash mentioned in the PR objectives.
860-868: The visibility condition is intentionally comprehensive.The past review flagged
showLoading || isLoadingas potentially redundant sinceshowLoadingis derived fromisLoading. However, this is intentional:
isLoadinghides the webview immediately when loading startsshowLoadingremains true during the delayed spinner display period- Without
isLoading, there would be a brief flash of the webview during theLOADING_SHOW_DELAY(500ms) before the spinner appearsThe styling changes (
position: absolute,inset: 0,visibilityinstead ofdisplay,zIndex: 0) correctly prevent layout shifts and ensure proper layering with the overlay components.
…nce error/loading UI - Renamed `video-call-window.tsx` to `video-call-window.ts` and refactored the component to TypeScript for better type safety. - Introduced new loading and error overlay components with corresponding CSS for improved user experience during loading and error states. - Updated the Rollup configuration to reflect the new input file name. - Adjusted documentation to match the new file naming conventions.
… IPC handling - Removed the onUnmount prop from ScreenSharePicker to keep the component mounted for reuse, enhancing performance. - Updated IPC event handling to ensure proper visibility management without unmounting the component. - Cleaned up code formatting for better readability in screenSharePickerMount and video-call-window files.
- Changed the preload script path from using `path.join` to a relative path for improved compatibility and simplicity in the video call window component.
…ript handling - Updated ScreenSharePicker to accept an onMounted prop for better visibility management. - Improved IPC event handling in screenSharePickerMount to ensure proper visibility state when the component is mounted. - Refactored video-call-window to construct an absolute path for the preload script, enhancing compatibility with Electron's webview requirements.
… video call handling - Introduced a new IPC event 'video-call-window/prewarm-capturer-cache' to prewarm the desktop capturer cache for improved performance during video calls. - Updated the video call window to invoke the prewarm cache function on setup, ensuring a smoother user experience. - Enhanced the ScreenSharePicker and related components for better visibility management and optimized preload script handling.
- Implemented a vanilla JavaScript bootstrap architecture for the video call window, improving performance and reducing initial load times. - Introduced a stale-while-revalidate caching mechanism for the desktop capturer, ensuring instant access to cached data while refreshing in the background. - Enhanced the ScreenSharePicker component with deferred loading, allowing for faster user interactions during screen sharing. - Updated documentation to reflect architectural changes and improved user experience features, including cache pre-warming and error handling strategies.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/public/video-call-window.html (1)
8-9: Overlay/container structure is solid; consider minimal ARIA for accessibilityThe new
#webview-containerplus#loading-overlay-root,#error-overlay-root, and#screen-picker-rootgive a clear separation of concerns and line up with the new loading/error CSS and React-mounted picker. To make these overlays more accessible, consider (optionally) adding roles/ARIA hints, e.g.:
role="status"oraria-live="polite"on the loading text container.role="alertdialog"andaria-labelledby/aria-describedbyon the error overlay.This would help screen readers announce these transient states without changing the visual behavior.
Also applies to: 23-45
src/videoCallWindow/screenSharePickerMount.tsx (1)
7-67: ScreenSharePicker mount/show orchestration is robustThe combination of a cached
reactRoot, thesetVisibleCallback, and theshouldShowOnMountflag gives you a resilient API:show()works whether or not the picker has been mounted yet, and mount failures reset internal state to allow retrial. The onMounted callback is used exactly once to capture the visibility setter, keeping React concerns encapsulated in this helper.If you later need programmatic closing beyond the picker’s own buttons, a symmetric
hide()that reusessetVisibleCallback(false)would fit naturally into this design, but it’s not required for the current flow.docs/video-call-window-flow.md (2)
11-29: Add language identifiers to fenced code blocks for better tooling and syntax highlighting.The static analysis flagged these code blocks as missing language specifiers. While these are ASCII diagrams, adding a language identifier (even
textorplaintext) improves markdown linter compliance and ensures consistent rendering across tools.-``` +```text ┌─────────────────────────────────────────────────────────────┐Apply similar changes to the code blocks at lines 91 and 174.
Also applies to: 91-106, 174-187
126-149: Add blank lines around tables for markdown compliance.Tables at lines 129, 138, and 145 should be surrounded by blank lines per markdown best practices. This ensures consistent rendering across different markdown parsers.
## Performance Characteristics ### Window Startup + | Phase | Time | |-------|------| | BrowserWindow creation | ~50ms | ... | Provider load | ~1-3s (network dependent) | + ### Screen Sharing + | Scenario | Response Time |docs/video-call-screen-sharing.md (1)
224-229: Add language identifiers to fenced code blocks.These code blocks showing example flows and cache lifecycle should have language identifiers for consistency with the TypeScript examples later in the document.
**Example flow:** -``` +```text T=0s: User opens picker → Cache returns instantly (2s old)Apply similar changes to the blocks at lines 276, 288, and 318.
Also applies to: 276-292, 318-346
src/videoCallWindow/ipc.ts (1)
415-435: RDP detection and WGC disable logic is correct for Windows.The implementation properly:
- Checks
SESSIONNAMEenvironment variable (standard Windows RDP indicator)- Identifies RDP sessions when
SESSIONNAME !== 'Console'- Combines with user preference
isVideoCallScreenCaptureFallbackEnabled- Passes appropriate Chromium flags to disable WGC capturers
One consideration: the dynamic import of
readSettinghere could be hoisted to the top of the file for consistency with other imports.Consider hoisting the
readSettingimport to maintain consistent import patterns:+import { readSetting } from '../store/readSetting'; // ... later in code ... if (process.platform === 'win32') { const sessionName = process.env.SESSIONNAME; const isRdpSession = typeof sessionName === 'string' && sessionName !== 'Console'; - const { readSetting } = await import('../store/readSetting'); const isScreenCaptureFallbackEnabled = readSetting(
📜 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.
📒 Files selected for processing (15)
docs/video-call-screen-sharing.md(2 hunks)docs/video-call-window-flow.md(1 hunks)docs/video-call-window-management.md(3 hunks)rollup.config.mjs(2 hunks)src/ipc/channels.ts(1 hunks)src/public/error.css(1 hunks)src/public/loading.css(1 hunks)src/public/video-call-window.html(2 hunks)src/ui/main/serverView/index.ts(2 hunks)src/videoCallWindow/ipc.ts(7 hunks)src/videoCallWindow/screenSharePicker.tsx(3 hunks)src/videoCallWindow/screenSharePickerMount.tsx(1 hunks)src/videoCallWindow/video-call-window.ts(1 hunks)src/videoCallWindow/video-call-window.tsx(0 hunks)src/videoCallWindow/videoCallWindow.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/videoCallWindow/videoCallWindow.tsx
- src/videoCallWindow/video-call-window.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- rollup.config.mjs
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/video-call-window-management.mddocs/video-call-screen-sharing.mddocs/video-call-window-flow.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/main/serverView/index.tssrc/videoCallWindow/screenSharePickerMount.tsxsrc/videoCallWindow/screenSharePicker.tsxsrc/videoCallWindow/ipc.tssrc/videoCallWindow/video-call-window.tssrc/ipc/channels.ts
src/ipc/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define and maintain IPC channels in
src/ipc/for main/renderer state synchronization
Files:
src/ipc/channels.ts
🧠 Learnings (4)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
docs/video-call-window-management.mdsrc/ui/main/serverView/index.tssrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/ui/main/serverView/index.tssrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.tssrc/videoCallWindow/video-call-window.tssrc/ipc/channels.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (1)
src/videoCallWindow/screenSharePickerMount.tsx (1)
src/videoCallWindow/screenSharePicker.tsx (1)
ScreenSharePicker(30-343)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 650-656: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 650-656: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🪛 GitHub Check: CodeQL
src/videoCallWindow/video-call-window.ts
[failure] 290-290: Client-side cross-site scripting
Cross-site scripting vulnerability due to user-provided value.
[failure] 580-580: Client-side cross-site scripting
Cross-site scripting vulnerability due to user-provided value.
[warning] 290-290: Client-side URL redirect
Untrusted URL redirection depends on a user-provided value.
[warning] 580-580: Client-side URL redirect
Untrusted URL redirection depends on a user-provided value.
🪛 LanguageTool
docs/video-call-screen-sharing.md
[grammar] ~9-~9: Use a hyphen to join words.
Context: ...tron. ## Cache Architecture The screen sharing system uses a **stale-while-reva...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/video-call-window-management.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
328-328: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/video-call-screen-sharing.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
224-224: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
276-276: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
302-302: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
310-310: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
318-318: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/video-call-window-flow.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
129-129: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
138-138: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
145-145: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
174-174: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (6)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
🔇 Additional comments (21)
src/videoCallWindow/screenSharePicker.tsx (1)
26-30: onMounted visibility hook and styling change look correctThe new
IScreenSharePickerPropswithonMountedcleanly exposes the internalsetVisiblesetter, and the effect wiring ensures callers likescreenSharePickerMountcan control visibility without leaking implementation details. The dialog still defaults to hidden and is only shown via that callback, which keeps behavior predictable. The switch tobackgroundColor='light'is consistent with the thumbnail card styling and doesn’t affect logic.Also applies to: 92-96, 175-175
src/ipc/channels.ts (1)
52-54: New prewarm IPC channel is consistent with existing typingsThe
'video-call-window/prewarm-capturer-cache'entry follows the established{ success: boolean }shape used by other video-call-window IPC channels and keeps channel definitions centralized inChannelToArgsMap, which is exactly what we want for main/renderer synchronization. Based on learnings, this aligns with how IPC channels should be maintained here.src/public/error.css (1)
1-103: Error overlay styling matches the new HTML and layering modelThe error overlay CSS correctly mirrors the new
#error-overlay-rootmarkup, including the background/content layering and reload button states. Defaultdisplay: nonewith a.showmodifier is a good fit for the scripted toggle behavior, and the z-index ensures it properly supersedes the loading overlay when both might be active.src/public/loading.css (1)
1-93: Loading overlay and animation integrate cleanly with the new window HTMLThe three-dot bounce animation and
#loading-overlay-rootoverlay behavior are well-scoped and align with the containers added invideo-call-window.html. The z-index and flex centering ensure the loading state sits correctly beneath the error overlay but above the webview.docs/video-call-window-flow.md (1)
5-35: Well-structured architecture documentation.The overview clearly explains the vanilla JS bootstrap approach with deferred React loading, and the ASCII diagram effectively communicates the layered architecture. This aligns well with the PR's goals of faster load times and simpler recovery.
docs/video-call-screen-sharing.md (3)
7-24: Excellent cache architecture documentation.The stale-while-revalidate pattern explanation is clear and the key benefits section effectively communicates the UX improvements. The memory footprint estimates are helpful for understanding resource usage.
27-149: Comprehensive mermaid flowchart covering the complete flow.The diagram effectively shows pre-warming, stale-while-revalidate logic, deduplication, and cache persistence. The color-coded class definitions make the flow easy to follow.
351-407: Code examples align with the actual implementation.The TypeScript snippets accurately reflect the stale-while-revalidate logic, background refresh, and pre-warming handler as implemented in
src/videoCallWindow/ipc.ts. This consistency between documentation and code is valuable for maintainability.src/videoCallWindow/ipc.ts (6)
30-34: Constants are well-defined with clear naming.The renamed
DESKTOP_CAPTURER_STALE_THRESHOLD(3000ms) clearly indicates its purpose in the stale-while-revalidate pattern, and the validation cache TTL (30s) is appropriate for source availability checks.
76-124: Solid stale-while-revalidate cache refresh implementation.The function correctly:
- Deduplicates concurrent requests via the promise guard (line 79)
- Filters invalid sources (empty names, empty thumbnails)
- Maintains a validation cache with TTL
- Preserves last good data on error (line 119)
- Clears the promise in
finallyblock (line 121)
126-155: Cache handler correctly implements stale-while-revalidate pattern.The logic appropriately:
- Returns cached data immediately when available (line 138)
- Triggers background refresh only when stale AND no refresh in progress (lines 135-136)
- Falls back to awaiting the promise for cold starts (lines 141-148)
- Returns empty array as last resort with graceful error handling
453-455: Conditional spread for additionalArguments is clean.The pattern
...(additionalArgs.length > 0 && { additionalArguments: additionalArgs })correctly avoids adding an empty array to webPreferences when no flags are needed.
640-652: URL passed via query parameters is a safe approach.Passing the video call URL through
loadFile's query option is preferable to usingloadURLwith the external URL directly, as it loads the local HTML first and then the renderer handles the URL.
1016-1019: Pre-warm handler enables proactive cache population.This simple handler triggers the background cache refresh when the webview finishes loading, ensuring sources are available before the user opens the screen picker. The fire-and-forget pattern is appropriate here.
src/videoCallWindow/video-call-window.ts (7)
58-100: i18n initialization with retry and fallback is well-implemented.The async language loading with fallback to
fallbackLngensures the UI always has translations available. The conditional resource loading (lines 79-84) only loads the additional language if different from fallback, which is efficient.
338-360: Load start handler correctly skips UI updates after initial load.The guard at lines 351-360 prevents unnecessary loading UI flickers when the webview navigates internally after the initial load has completed. This is good UX design.
449-454: Cache pre-warming integration is well-placed.Triggering the cache pre-warm on
did-finish-loadensures sources are fetched before the user might click screen share. The silent catch is appropriate since this is an optional optimization.
568-576: Webview preload path construction using path.posix is appropriate.Using
path.posixensures consistent forward slashes in the preload path regardless of platform, which is correct for HTML attribute values. The partitionpersist:jitsi-sessionprovides session isolation.
650-656: innerHTML usage is safe here - static hardcoded content.The static analysis warning about innerHTML is a false positive in this case. The content is a hardcoded string literal with no user input or dynamic values. No sanitization is needed.
666-757: Start function handles initialization with comprehensive error recovery.The initialization flow properly:
- Waits for DOM if not ready (lines 683-692)
- Initializes i18n before UI operations
- Extracts URL from query params
- Performs handshake with main process
- Retries up to MAX_INIT_ATTEMPTS on failure
768-786: Proper cleanup of IPC listeners on unload prevents memory leaks.The
beforeunloadhandler correctly removes the screen picker listener and clears timeouts, preventing dangling references if the window is reloaded.
… handling - Enhanced the logic for identifying video call webviews using strict URL parsing and partition checks. - Implemented a function to resolve and validate preload file paths, ensuring fallback options are available if the primary preload file is missing. - Updated the ScreenSharePicker component to optimize the useEffect hook for better performance. - Revised documentation to reflect changes in caching architecture and performance characteristics.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/ui/main/serverView/index.ts (1)
81-88: Consider adding explicit Pexip support for completeness.The known hosts list only includes Jitsi-related domains. If Rocket.Chat supports Pexip video calls, consider adding Pexip patterns to ensure consistent preload handling across all video providers.
#!/bin/bash # Search for Pexip-related partition or URL patterns in the codebase echo "=== Searching for Pexip references ===" rg -n -i "pexip" --type ts -C2
🧹 Nitpick comments (2)
docs/video-call-window-flow.md (1)
11-29: Add language specification to fenced code block.The ASCII art diagram block is missing a language identifier, which causes the MD040 linting error. Add
`textto the opening fence to mark it as plain text.-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ Video Call Window │src/ui/main/serverView/index.ts (1)
99-106: Overly broad video call detection heuristics may cause false positives.The
jwtquery parameter check and the room name pattern are too generic:
- Many authentication systems use
jwtparameters (OAuth callbacks, API tokens, etc.)- The regex
/^\/[a-zA-Z0-9_-]{6,}$/matches common paths like/images,/api-key,/settings, etc.This could incorrectly classify non-video-call webviews, applying the wrong preload script.
Consider tightening these heuristics:
- if (url.searchParams.has('jwt')) { - return true; - } - - const roomNamePattern = /^\/[a-zA-Z0-9_-]{6,}$/; - if (roomNamePattern.test(pathname)) { - return true; - } + // Only treat JWT as video call indicator when combined with other Jitsi signals + const hasJitsiIndicator = hostname.includes('jitsi') || + pathname.includes('meet') || + url.searchParams.has('room'); + if (url.searchParams.has('jwt') && hasJitsiIndicator) { + return true; + }
📜 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.
📒 Files selected for processing (4)
docs/video-call-screen-sharing.md(2 hunks)docs/video-call-window-flow.md(1 hunks)src/ui/main/serverView/index.ts(3 hunks)src/videoCallWindow/screenSharePicker.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/videoCallWindow/screenSharePicker.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/main/serverView/index.ts
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/video-call-window-flow.mddocs/video-call-screen-sharing.md
🧠 Learnings (1)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/ui/main/serverView/index.ts
🧬 Code graph analysis (1)
src/ui/main/serverView/index.ts (1)
src/app/reducers/appPath.ts (1)
appPath(8-19)
🪛 LanguageTool
docs/video-call-screen-sharing.md
[grammar] ~9-~9: Use a hyphen to join words.
Context: ...tron. ## Cache Architecture The screen sharing system uses a **stale-while-reva...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/video-call-window-flow.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (6)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
🔇 Additional comments (5)
docs/video-call-screen-sharing.md (1)
9-9: Grammatical false positive detected.The hyphenation of "stale-while-revalidate" is correct and should not be flagged for modification. This is a false positive from the static analysis tool.
docs/video-call-window-flow.md (1)
80-84: External documentation reference verified.The referenced file
docs/video-call-window-wgc-limitations.mdexists in the repository at 72 lines, confirming the documentation link in line 80 is valid and properly integrated with the related video-call-window documentation files.src/ui/main/serverView/index.ts (3)
114-143: LGTM — Well-structured preload resolution with proper fallbacks.The function correctly handles missing files with clear warnings and graceful degradation. The synchronous
fs.existsSyncis acceptable here since thewill-attach-webviewhandler requires synchronous operation.
295-305: LGTM — Correct conditional preload handling.The logic correctly determines if the webview is a video call and applies the appropriate preload path. Passing
undefinedforframeNameis correct since frame names aren't available in thewill-attach-webviewcontext.
342-358: LGTM — Clean conditional preload injection for new windows.The implementation correctly applies preload only when both conditions are met: it's a video call and a valid preload path exists. The conditional spread pattern is idiomatic TypeScript.
…trieval logic - Modified the IPC event for opening the screen picker to return a success status, enhancing error handling. - Improved URL retrieval logic in the video call window to request the URL via IPC if not provided in query parameters, ensuring a smoother user experience. - Added comments for clarity and better understanding of the code flow in the video call window and screen share picker components.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/videoCallWindow/video-call-window.ts (2)
565-608: Add defense-in-depth URL validation before webview creation.CodeQL flags line 592 where
urlis assigned towebview.src. While the main process validates this URL, adding renderer-side validation provides defense-in-depth security.const createWebview = (url: string): void => { const container = document.getElementById('webview-container'); if (!container) { throw new Error('Webview container not found'); } + // Defense-in-depth URL validation + try { + const parsedUrl = new URL(url); + if (!['http:', 'https:'].includes(parsedUrl.protocol)) { + throw new Error(`Invalid URL protocol: ${parsedUrl.protocol}`); + } + } catch (error) { + console.error('Video call window: Invalid URL provided:', error); + throw error; + } + const htmlPath = window.location.pathname;
288-296: Validate URL before assignment in recovery path.CodeQL flags line 293 where
state.urlis assigned towebview.srcduring URL refresh recovery. Add defensive URL validation before assignment:case 2: if (webview && state.url) { + try { + const validatedUrl = new URL(state.url); + if (!['http:', 'https:'].includes(validatedUrl.protocol)) { + console.error('Invalid URL protocol in recovery:', validatedUrl.protocol); + return; + } + } catch (error) { + console.error('Invalid URL in recovery:', state.url); + return; + } webview.src = 'about:blank'; setTimeout(() => { if (webview && state.url) { webview.src = state.url; } }, 500); } break;
🧹 Nitpick comments (2)
src/videoCallWindow/ipc.ts (1)
421-441: Windows-specific WGC disabling logic is well-implemented.The RDP detection using
process.env.SESSIONNAMEis correct for Windows, and the dynamic import ofreadSettingavoids potential circular dependencies. The logic correctly disables WGC features when either RDP is detected or the user has enabled the screen capture fallback.Consider adding error handling around the
readSettingcall to gracefully handle potential import or read failures:const { readSetting } = await import('../store/readSetting'); const isScreenCaptureFallbackEnabled = readSetting( 'isVideoCallScreenCaptureFallbackEnabled' - ); + ) ?? false;src/videoCallWindow/video-call-window.ts (1)
669-675: Static innerHTML assignment is safe but could use DOM APIs for clarity.ast-grep flags the
innerHTMLassignment as a potential XSS risk, but this is a false positive since the content is entirely static with no user input. However, using DOM APIs would make the intent clearer and satisfy static analysis:- fallbackContainer.innerHTML = ` - <div style="text-align: center;"> - <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> - <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> - <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> - </div> - `; + const content = document.createElement('div'); + content.style.textAlign = 'center'; + + const h2 = document.createElement('h2'); + h2.style.cssText = 'color: #fff; margin: 0;'; + h2.textContent = 'Video Call Unavailable'; + + const p1 = document.createElement('p'); + p1.style.cssText = 'color: #ccc; margin: 10px 0;'; + p1.textContent = 'Unable to initialize video call window'; + + const p2 = document.createElement('p'); + p2.style.cssText = 'color: #999; margin: 10px 0; font-size: 14px;'; + p2.textContent = 'Retrying automatically in 3 seconds...'; + + content.appendChild(h2); + content.appendChild(p1); + content.appendChild(p2); + fallbackContainer.appendChild(content);
📜 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.
📒 Files selected for processing (4)
src/ipc/channels.ts(2 hunks)src/videoCallWindow/ipc.ts(8 hunks)src/videoCallWindow/screenSharePickerMount.tsx(1 hunks)src/videoCallWindow/video-call-window.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/videoCallWindow/screenSharePickerMount.tsx
- src/ipc/channels.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/ipc.tssrc/videoCallWindow/video-call-window.ts
🧠 Learnings (5)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.tssrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/**/!(*.main).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (2)
src/videoCallWindow/ipc.ts (3)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)src/ipc/renderer.ts (1)
handle(6-36)src/store/readSetting.ts (1)
readSetting(6-16)
src/videoCallWindow/video-call-window.ts (3)
src/ipc/renderer.ts (2)
invokeWithRetry(54-115)IRetryOptions(43-52)src/i18n/common.ts (1)
fallbackLng(3-3)src/videoCallWindow/screenSharePickerMount.tsx (1)
show(49-68)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 668-674: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 668-674: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🪛 GitHub Check: build (ubuntu-latest, linux)
src/videoCallWindow/video-call-window.ts
[failure] 42-42:
import() type annotations are forbidden
🪛 GitHub Check: check (macos-latest)
src/videoCallWindow/video-call-window.ts
[failure] 42-42:
import() type annotations are forbidden
🪛 GitHub Check: check (ubuntu-latest)
src/videoCallWindow/video-call-window.ts
[failure] 42-42:
import() type annotations are forbidden
🪛 GitHub Check: CodeQL
src/videoCallWindow/video-call-window.ts
[failure] 293-293: Client-side cross-site scripting
Cross-site scripting vulnerability due to user-provided value.
[failure] 592-592: Client-side cross-site scripting
Cross-site scripting vulnerability due to user-provided value.
[warning] 293-293: Client-side URL redirect
Untrusted URL redirection depends on a user-provided value.
[warning] 592-592: Client-side URL redirect
Untrusted URL redirection depends on a user-provided value.
⏰ 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: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
🔇 Additional comments (10)
src/videoCallWindow/ipc.ts (6)
76-155: Stale-while-revalidate desktop capturer cache implementation looks solid.The refactored caching logic implements a stale-while-revalidate pattern that improves performance by returning cached sources immediately while refreshing in the background when stale. The implementation correctly:
- Prevents concurrent refreshes via
desktopCapturerPromiseguard- Filters invalid sources (empty names, empty thumbnails)
- Handles errors gracefully by returning cached sources
- Resets the promise in the
finallyblock to ensure cleanup
312-321: LGTM!The updated handler now returns a success flag, allowing the caller to handle cases where the video call window is not available. This is consistent with the pattern used by other IPC handlers in the file.
646-658: LGTM!The query parameter passing via
loadFileis the correct approach for Electron. ConvertingautoOpenDevtoolsto a string is necessary since query parameters are always strings, and the renderer can parse it back to boolean.
1022-1025: LGTM!The prewarm handler allows the renderer to proactively refresh the desktop capturer cache. The fire-and-forget pattern is appropriate here since cache warming is an optimization that shouldn't block the caller.
459-461: LGTM!The conditional spread pattern correctly includes
additionalArgumentsonly when needed. This is an idiomatic TypeScript approach for optional object properties.
393-393: LGTM!The state extraction for
isAutoOpenEnabledfollows the existing pattern and properly propagates the devtools auto-open setting to the window creation flow.src/videoCallWindow/video-call-window.ts (4)
232-305: Auto-recovery logic with progressive strategies is well-designed.The three-tier recovery approach (webview reload → URL refresh → full reinitialize) with increasing delays provides good resilience for transient failures. The implementation correctly tracks attempts, applies appropriate delays, and falls back to an error UI after exhausting retries.
61-103: LGTM!The i18n initialization properly handles language fetching from the main process with retry logic and falls back gracefully to the default language. The implementation correctly loads both the requested language and the fallback language.
341-563: Comprehensive webview event handlers are well-implemented.The event handlers cover all critical webview lifecycle events and properly manage state transitions. The implementation includes:
- Loading timeout with auto-recovery
- Close page detection for graceful shutdown
- Proper error handling with retry logic
- Auto-devtools opening when configured
- Visibility management for smooth UX
610-633: Screen picker lazy loading is well-implemented.The preload logic correctly:
- Lazy loads the React-based screen picker only when needed
- Prevents concurrent imports via promise tracking
- Mounts the picker in a hidden state for instant display later
- Handles errors gracefully
The overall vanilla JS/TS architecture is appropriate for this use case, providing a lighter-weight alternative to a full React renderer while maintaining good state management and error handling.
… state - Added a line to set `screenPickerModulePromise` to null in the error handling block of the screen picker opening function, ensuring that the promise state is reset if an error occurs while attempting to open the screen picker.
- Included a type import for `screenPickerModulePromise` to ensure proper type checking and consistency in the video call window component.
…er mount - Moved the reset of `shouldShowOnMount` to the callback function to maintain proper visibility management when the screen share picker is shown.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/videoCallWindow/video-call-window.ts (2)
290-296: URL validation before assignment was previously recommended.The CodeQL warning about potential XSS/redirect at line 294 was addressed in a prior review. Consider adding defensive URL validation (protocol allowlist check) before assigning to
webview.srcfor defense-in-depth, as suggested previously.
587-593: URL validation before webview.src assignment was previously recommended.The CodeQL XSS/redirect warning at line 593 was addressed in a prior review. Consider adding defensive URL validation as previously suggested.
Note:
nodeIntegrationin webpreferences (line 589) is intentional for the Jitsi integration but should be documented to clarify the security trade-off.
🧹 Nitpick comments (3)
src/videoCallWindow/video-call-window.ts (3)
399-423: Consider extracting duplicate devtools auto-open logic.The devtools auto-opening code is duplicated in
handleDomReady(lines 402-422) andhandleWebviewAttached(lines 533-554). Consider extracting to a helper function.const autoOpenDevtools = (context: string): void => { if (!state.shouldAutoOpenDevtools) return; invokeWithRetry('video-call-window/open-webview-dev-tools', { maxAttempts: 2, retryDelay: 500, logRetries: process.env.NODE_ENV === 'development', }) .then((success: boolean) => { if (success) { console.log(`Video call window: Successfully auto-opened devtools (${context})`); } }) .catch((error: any) => { console.error(`Video call window: Error auto-opening devtools (${context}):`, error); }); };Also applies to: 519-555
669-676: Static innerHTML is safe in this context.The ast-grep warning about
innerHTMLis a false positive here. The content is entirely static strings with no user input interpolation, making XSS impossible. However, for consistency with the rest of the codebase, you could use DOM APIs if preferred:const wrapper = document.createElement('div'); wrapper.style.textAlign = 'center'; const h2 = document.createElement('h2'); h2.style.cssText = 'color: #fff; margin: 0;'; h2.textContent = 'Video Call Unavailable'; const p1 = document.createElement('p'); p1.style.cssText = 'color: #ccc; margin: 10px 0;'; p1.textContent = 'Unable to initialize video call window'; const p2 = document.createElement('p'); p2.style.cssText = 'color: #999; margin: 10px 0; font-size: 14px;'; p2.textContent = 'Retrying automatically in 3 seconds...'; wrapper.append(h2, p1, p2); fallbackContainer.appendChild(wrapper);
820-837: Async IIFE pattern works but could be simplified.The
handleOpenScreenPickeruses an async IIFE which is functional but unconventional. Consider making the function itself async and handling the promise at the call site, or accepting the fire-and-forget nature explicitly:-const handleOpenScreenPicker = (): void => { - (async () => { - try { +const handleOpenScreenPicker = async (): Promise<void> => { + try { if (!screenPickerModule) { if (!screenPickerModulePromise) { screenPickerModulePromise = import('./screenSharePickerMount'); } screenPickerModule = await screenPickerModulePromise; screenPickerModulePromise = null; } screenPickerModule.show(); - } catch (error) { - screenPickerModulePromise = null; - console.error('Video call window: Failed to open screen picker:', error); - } - })(); + } catch (error) { + screenPickerModulePromise = null; + console.error('Video call window: Failed to open screen picker:', error); + } }; + +ipcRenderer.on('video-call-window/open-screen-picker', () => { + handleOpenScreenPicker().catch(() => {}); +});
📜 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.
📒 Files selected for processing (1)
src/videoCallWindow/video-call-window.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/video-call-window.ts
🧠 Learnings (5)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/**/!(*.main).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (1)
src/videoCallWindow/video-call-window.ts (3)
src/ipc/renderer.ts (2)
invokeWithRetry(54-115)IRetryOptions(43-52)src/i18n/common.ts (1)
fallbackLng(3-3)src/videoCallWindow/screenSharePickerMount.tsx (1)
show(49-68)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 669-675: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 669-675: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🪛 GitHub Check: CodeQL
src/videoCallWindow/video-call-window.ts
[failure] 294-294: Client-side cross-site scripting
Cross-site scripting vulnerability due to user-provided value.
[failure] 593-593: Client-side cross-site scripting
Cross-site scripting vulnerability due to user-provided value.
[warning] 294-294: Client-side URL redirect
Untrusted URL redirection depends on a user-provided value.
[warning] 593-593: Client-side URL redirect
Untrusted URL redirection depends on a user-provided value.
⏰ 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). (6)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
🔇 Additional comments (6)
src/videoCallWindow/video-call-window.ts (6)
1-55: Well-structured module initialization with proper typing.The imports, constants, and state interface are appropriately defined. The eslint-disable comments for the
import()type annotations (lines 39-44) are an acceptable workaround for the ESLint restriction.
62-104: Robust i18n initialization with fallback handling.The implementation correctly handles missing language resources via optional chaining (line 86) and provides a fallback language. The retry logic via
invokeWithRetryensures resilience during IPC communication.
129-157: UI helper properly manages loading overlay state.The function correctly clears the overlay class and updates text content with i18n. The
as anycast for webview style access is a pragmatic workaround for Electron's webview typing limitations.
308-340: Close page detection with graceful shutdown.The detection logic and delayed IPC notification provide a clean window closing experience. The try/catch ensures errors are logged without breaking the close flow.
685-808: Comprehensive initialization with proper fallback paths.The
start()function handles multiple scenarios gracefully: waiting for DOM readiness, URL resolution via query params or IPC, retry logic with max attempts, and fallback UI. The recursive retry is properly bounded byinitAttemptscounter.
851-872: Clean entry point with proper error boundary.The setup pattern correctly handles DOM readiness and provides a fallback UI on fatal errors. The initialization flow is well-structured.
- Added support for both `persist:jitsi-session` and `persist:pexip-session` partitions in video call webviews to ensure proper identification and preload script application. - Introduced a URL validation function to enforce strict checks on video call URLs, allowing only specific protocols and known hosts, improving security and reliability. - Updated the preload script path handling for consistency and clarity across components. - Enhanced the ScreenSharePicker module with a new type definition for better type safety.
…rovider support - Updated the video call window documentation to clarify support for any video conferencing provider, emphasizing the generic, provider-agnostic architecture. - Enhanced URL validation to remove provider-specific patterns, allowing seamless integration with various video call services. - Improved the handling of webview partitions to use a generic partition name for all providers, ensuring consistent behavior across different video call implementations. - Added detailed sections on provider-specific requirements for Jitsi and PEXIP, highlighting necessary components and integration steps.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/videoCallWindow/video-call-window.ts (1)
41-43: Type import addresses ESLint concerns.Using the exported
ScreenSharePickerModuleTypeinstead oftypeof import(...)correctly addresses the previously flagged ESLint error about forbiddenimport()type annotations.
🧹 Nitpick comments (1)
src/videoCallWindow/screenSharePickerMount.tsx (1)
54-72: Consider edge case with rapidshow()calls.The logic handles the case where
setVisibleCallbackisn't ready, but ifshow()is called multiple times rapidly before the callback is set:
- Each call sets
shouldShowOnMount = true- Each call schedules a new
setTimeout- Multiple timeouts may call
setVisibleCallback(true)redundantlyWhile not a bug (calling
setVisible(true)multiple times is idempotent), consider clearing the previous timeout or adding a guard:+let showTimeout: ReturnType<typeof setTimeout> | null = null; + export const show = (): void => { if (!reactRoot) { shouldShowOnMount = true; mount(); return; } if (setVisibleCallback) { setVisibleCallback(true); } else { shouldShowOnMount = true; + if (showTimeout) clearTimeout(showTimeout); - setTimeout(() => { + showTimeout = setTimeout(() => { if (setVisibleCallback) { setVisibleCallback(true); } shouldShowOnMount = false; + showTimeout = null; }, 0); } };
📜 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.
📒 Files selected for processing (5)
docs/video-call-window-flow.md(1 hunks)docs/video-call-window-management.md(3 hunks)src/ui/main/serverView/index.ts(3 hunks)src/videoCallWindow/screenSharePickerMount.tsx(1 hunks)src/videoCallWindow/video-call-window.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/video-call-window-flow.mddocs/video-call-window-management.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/video-call-window.tssrc/ui/main/serverView/index.tssrc/videoCallWindow/screenSharePickerMount.tsx
🧠 Learnings (5)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
docs/video-call-window-management.mdsrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to **/*.{ts,tsx} : Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Applied to files:
docs/video-call-window-management.md
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/videoCallWindow/video-call-window.tssrc/ui/main/serverView/index.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (3)
src/videoCallWindow/video-call-window.ts (3)
src/videoCallWindow/screenSharePickerMount.tsx (2)
ScreenSharePickerModuleType(7-10)show(54-73)src/ipc/renderer.ts (2)
invokeWithRetry(54-115)IRetryOptions(43-52)src/i18n/common.ts (1)
fallbackLng(3-3)
src/ui/main/serverView/index.ts (2)
src/app/reducers/appPath.ts (1)
appPath(8-19)workspaces/desktop-release-action/dist/index.js (7)
path(142-142)path(621-621)path(1145-1145)path(2862-2862)path(3051-3051)fs(475-475)fs(2861-2861)
src/videoCallWindow/screenSharePickerMount.tsx (1)
src/videoCallWindow/screenSharePicker.tsx (1)
ScreenSharePicker(30-344)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 711-717: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 711-717: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
⏰ 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). (6)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
🔇 Additional comments (13)
src/ui/main/serverView/index.ts (3)
55-106: Well-structured preload resolution with proper fallback behavior.The
VIDEO_CALL_PRELOAD_PATHconstant,isVideoCallWebviewhelper, andresolvePreloadPathfunction provide clean abstractions for managing preload scripts. The fallback logic with console warnings is appropriate for debugging issues in production.Note: The
_srcparameter inisVideoCallWebviewis correctly prefixed to indicate intentional non-use, avoiding ESLint warnings while preserving the function signature for potential future use.
258-268: LGTM - Preload resolution integrated correctly.The integration properly uses the new helpers and handles the null case by removing the preload attribute, preventing undefined preload errors.
305-318: Conditional preload for video call windows is correct.The logic correctly applies preload only when both
isVideoCallis true andpreloadPathis non-null, using object spread to conditionally include the preload in webPreferences.src/videoCallWindow/screenSharePickerMount.tsx (1)
27-51: LGTM - Mount function with proper error handling.The mount function correctly guards against duplicate mounts, captures the visibility setter via
onMounted, and resets all module state on failure to allow retry.src/videoCallWindow/video-call-window.ts (4)
583-601: URL validation properly addresses security concerns.The
validateVideoCallUrlfunction validates URL format and restricts protocols tohttp:andhttps:, addressing the previously flagged CodeQL concerns about XSS and URL redirect vulnerabilities.
288-313: Recovery logic now validates URLs before assignment.The URL refresh recovery (case 2) now calls
validateVideoCallUrl(state.url)before assigning towebview.src, and properly handles validation failures by skipping to the next recovery attempt. This addresses the previously flagged CodeQL concerns.
695-725: Static HTML is safe - false positive from static analysis.The
innerHTMLassignment uses a completely static template literal with no user input or dynamic content. This is a false positive from static analysis tools that cannot determine the content is hardcoded. No changes needed.
862-878: IPC handler uses IIFE correctly but lacks error boundary.The
handleOpenScreenPickerfunction uses an async IIFE to handle the dynamic import. While the try/catch handles import failures, ifscreenPickerModule.show()throws after a successful import, the error would be unhandled.const handleOpenScreenPicker = (): void => { (async () => { try { if (!screenPickerModule) { if (!screenPickerModulePromise) { screenPickerModulePromise = import('./screenSharePickerMount'); } screenPickerModule = await screenPickerModulePromise; screenPickerModulePromise = null; } screenPickerModule.show(); } catch (error) { screenPickerModulePromise = null; console.error('Video call window: Failed to open screen picker:', error); } })(); };The current structure means if
show()throws, it's caught. However, sinceshow()is synchronous and doesn't throw, this is acceptable as-is.docs/video-call-window-management.md (2)
7-23: Documentation accurately describes the new vanilla JS architecture.The architecture section clearly explains the design decision to use vanilla JavaScript for the critical path with deferred React loading. The benefits listed (faster load, simpler recovery, reduced memory) align with the implementation.
333-344: File structure matches implementation.The documented file structure accurately reflects the actual project layout, with clear annotations indicating which components are generic versus Jitsi-specific.
docs/video-call-window-flow.md (3)
5-35: Architecture documentation is comprehensive.The overview clearly explains the vanilla JavaScript bootstrap with deferred React loading and stale-while-revalidate caching. The ASCII diagram effectively illustrates the layered architecture.
147-170: Provider support documentation clarifies generic vs Jitsi-specific requirements.This section is valuable as it clearly distinguishes:
- Generic functionality that works with all providers (PEXIP, self-hosted, etc.)
- Jitsi-specific requirements (jitsiBridge is required for Jitsi screen sharing)
This helps developers understand what's needed for different video call providers.
80-85: Good addition of WGC Limitations section.Adding a reference to the WGC limitations documentation helps users understand why screen sharing may fail in RDP sessions, which is the core issue this PR addresses.
- Removed the 'nativeWindowOpen=true' option from the webview's webpreferences attribute, streamlining the configuration for improved clarity and consistency.
…lassification - Introduced a 'category' field in notification options, allowing notifications to be classified as 'DOWNLOADS' or 'SERVER'. - Updated various notification creation points across the application to include the new category field, enhancing the organization and handling of notifications. - Adjusted tests to verify the inclusion of the category in notification payloads, ensuring consistency in notification behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/notifications/main.ts (1)
244-254: Consider cleaning up category in dismissed handler.The
NOTIFICATIONS_NOTIFICATION_DISMISSEDhandler cleans upnotificationTypesbut does not clean upnotificationCategories. While the close listener should eventually fire, adding explicit cleanup here would provide defense-in-depth against memory leaks if a notification is dismissed but the close event doesn't fire.listen(NOTIFICATIONS_NOTIFICATION_DISMISSED, (action) => { const notificationId = String(action.payload.id); const notificationType = notificationTypes.get(notificationId); notifications.get(notificationId)?.close(); if (notificationType === 'voice') { attentionDrawing.stopAttention(notificationId); } notificationTypes.delete(notificationId); + notificationCategories.delete(notificationId); });src/ui/main/serverView/index.ts (4)
55-106: Align isVideoCallWebview JSDoc and detection heuristics with implementation
isVideoCallWebviewcurrently only checkspartition === 'persist:video-call-session'orframeName === 'Video Call';_srcis unused and there is no URL parsing, so the JSDoc about “URL patterns” and “strict URL parsing” is now misleading. I’d either:
- Update the comment (and possibly drop
_srcfrom the signature) to reflect the actual behavior, or- Re‑introduce URL-based detection if you still need it.
Also worth double‑checking that all supported video-call providers consistently use either the
persist:video-call-sessionpartition or the"Video Call"frame name so this heuristic doesn’t miss any cases.
112-122: New getServerUrlByWebContentsId helper is correct but duplicates lookup logicThe implementation is straightforward and matches the existing
server-view/get-urlIPC handler below, which performs the samewebContentsByServerUrlreverse lookup from aWebContents. To avoid future drift, consider extracting a shared helper likegetServerUrlByWebContents(webContents: WebContents)and having both this function and the IPC handler call it.
270-280: Consider passing the webview name into isVideoCallWebview for attach-time detectionIn
handleWillAttachWebviewyou call:const isVideoCall = isVideoCallWebview( _params.partition, _params.src, undefined );This means attach-time detection relies solely on the partition. If any video-call webviews are identified only by their frame/name (as in other code paths), they won’t be recognized here.
If
will-attach-webview’sparamsexposes anamefield in your Electron version (which typically corresponds to the frame name), consider passing it through for consistency:- const isVideoCall = isVideoCallWebview( - _params.partition, - _params.src, - undefined - ); + const isVideoCall = isVideoCallWebview( + _params.partition, + _params.src, + // Electron's params.name usually matches the frame/window name if present + (_params as { name?: string }).name + );This keeps the detection logic aligned between attach-time and window-open flows.
317-327: Tighten window-open video-call handling and avoid unnecessary preload resolutionIn the
setWindowOpenHandler:
resolvePreloadPath(isVideoCall)is called even whenisVideoCallis false, butpreloadPathis only used in theisVideoCallbranch, so this does extra sync FS work and may log warnings that are irrelevant to non–video-call windows.- In this context, because
partitionis not passed,isVideoCallWebview(undefined, url, frameName)effectively reduces toframeName === 'Video Call'. Please verify that this name is stable across all providers; otherwise, a small change in frame name could silently drop the special handling.You could guard the preload resolution and make the intent clearer:
- const isVideoCall = isVideoCallWebview(undefined, url, frameName); - const preloadPath = resolvePreloadPath(isVideoCall); + const isVideoCall = isVideoCallWebview(undefined, url, frameName); + const preloadPath = isVideoCall ? resolvePreloadPath(true) : null;The rest of the override (conditional
webPreferences.preload+sandbox: false) then stays the same. Also please confirm that the “no preload but sandbox: false” case (when both target and fallback preloads are missing) is acceptable for the video-call window behavior.
📜 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.
📒 Files selected for processing (10)
src/downloads/main.ts(2 hunks)src/downloads/main/setup.ts(1 hunks)src/downloads/notifications.spec.ts(14 hunks)src/main.spec.ts(3 hunks)src/notifications/actions.ts(1 hunks)src/notifications/common.ts(2 hunks)src/notifications/main.ts(5 hunks)src/notifications/preload.ts(3 hunks)src/ui/main/serverView/index.ts(3 hunks)src/videoCallWindow/video-call-window.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/downloads/main.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/notifications/common.tssrc/main.spec.tssrc/downloads/notifications.spec.tssrc/ui/main/serverView/index.tssrc/notifications/actions.tssrc/notifications/main.tssrc/videoCallWindow/video-call-window.tssrc/downloads/main/setup.tssrc/notifications/preload.ts
src/**/!(*.main).spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Renderer-process tests should be named
*.spec.ts(without the.mainqualifier)
Files:
src/main.spec.tssrc/downloads/notifications.spec.ts
🧠 Learnings (6)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/**/!(*.main).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/main.spec.tssrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/**/*.main.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/main.spec.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/ui/main/serverView/index.tssrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/notifications/main.tssrc/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (3)
src/notifications/main.ts (3)
src/ui/main/serverView/index.ts (1)
getServerUrlByWebContentsId(112-122)src/store/index.ts (1)
dispatchSingle(42-49)src/notifications/actions.ts (1)
NOTIFICATIONS_NOTIFICATION_CLICKED(7-8)
src/videoCallWindow/video-call-window.ts (3)
src/videoCallWindow/screenSharePickerMount.tsx (2)
ScreenSharePickerModuleType(7-10)show(54-73)src/ipc/renderer.ts (2)
invokeWithRetry(54-115)IRetryOptions(43-52)src/i18n/common.ts (1)
fallbackLng(3-3)
src/notifications/preload.ts (3)
src/store/index.ts (1)
dispatch(38-40)src/ui/actions.ts (2)
SIDE_BAR_DOWNLOADS_BUTTON_CLICKED(44-45)WEBVIEW_FOCUS_REQUESTED(68-68)src/servers/preload/urls.ts (1)
getServerUrl(9-9)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 708-714: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 708-714: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ 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). (6)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
🔇 Additional comments (19)
src/videoCallWindow/video-call-window.ts (7)
1-54: Well-structured module initialization with clear state management.The constants, type definitions, and state interface provide a clean foundation for the video call window lifecycle. The use of typed state with explicit status values (
idle,loading,ready,error,closing) enables predictable state transitions.
109-126: Thorough timeout cleanup prevents memory leaks.The
clearAllTimeoutsfunction properly clears all four timeout handles and nullifies them, which is essential for preventing memory leaks during recovery cycles and window destruction.
232-323: Robust three-tier auto-recovery with proper URL validation.The recovery strategy (webview reload → URL refresh → full reinitialize) with increasing delays is well-designed. The URL validation at lines 290-313 properly catches validation failures and skips to the next recovery step rather than failing silently.
583-601: Proper URL validation with protocol allowlist.The
validateVideoCallUrlfunction correctly validates URLs and restricts protocols tohttp:andhttps:, addressing the CodeQL XSS/redirect concerns from previous reviews. This provides defense-in-depth for URL handling.
627-648: Webview created with appropriate security configuration.The webview is created with validated URL, computed preload path, and appropriate attributes. The
nodeIntegrationwebpreference is intentional for the video call context. The deprecatednativeWindowOpenhas been removed as suggested in previous reviews.
692-722: Static innerHTML usage is safe in this context.The static analysis warning about
innerHTMLat lines 709-715 is a false positive. The content is a hardcoded static string with no user input, making XSS impossible. The fallback UI provides a reasonable user experience when initialization fails.
880-888: Proper IPC listener cleanup on window unload.The
beforeunloadhandler correctly removes the IPC listener to prevent memory leaks on window reload or destruction. This is important for long-running Electron applications.src/downloads/main/setup.ts (1)
105-110: Category field enables proper notification routing.Adding
category: 'DOWNLOADS'to the notification payload enables the click handler inpreload.tsto route download notifications correctly without relying on the brittle title-based comparison that was previously flagged in reviews.src/main.spec.ts (2)
241-250: Test expectations correctly updated for new category field.The test helper
setupElectronDlWithTrackingand its expectations are properly updated to includecategory: 'DOWNLOADS', ensuring test coverage for the notification payload changes.
354-394: Comprehensive test coverage for notification category in onCompleted callbacks.Both test cases (
should create a notification when download completesandshould use translated subtitle) correctly verify that thecategory: 'DOWNLOADS'field is included in the notification payload.src/notifications/common.ts (1)
1-14: Clean type definitions for notification categorization.The
NotificationCategorytype provides type safety for notification routing, and adding it as an optional field toExtendedNotificationOptionsmaintains backward compatibility while enabling the new category-based routing logic.src/downloads/notifications.spec.ts (2)
51-74: Test properly verifies category field in electron-dl integration.The test correctly sets and expects
category: 'DOWNLOADS'in the notification payload, ensuring the electron-dl completion path is covered.
357-369: Missing category field in cancellation test may cause inconsistency.This test for cancellation messages in different languages doesn't include
category: 'DOWNLOADS', unlike other similar tests in this file. This could indicate either an intentional omission or a test gap.Consider whether this test should also include
category: 'DOWNLOADS'for consistency with the production code insrc/downloads/main/setup.ts, or if cancellation notifications intentionally omit the category.src/notifications/actions.ts (1)
22-27: Payload type correctly extended with optional fields.The
NOTIFICATIONS_NOTIFICATION_CLICKEDpayload type is properly extended with optionalserverUrlandcategoryfields, maintaining backward compatibility while enabling the new routing logic.src/notifications/preload.ts (2)
114-133: Category-based routing properly addresses past review concerns.The click handler now uses
category === 'DOWNLOADS'instead of the brittletitle === 'Downloads'comparison, and usesserverUrl || getServerUrl()to fix the multi-server focus bug. Both issues flagged in previous reviews are properly addressed.
36-48: createNotification signature correctly extended with category.The optional
categoryparameter is properly typed and passed through to the notification payload, enabling downstream routing based on notification type.src/notifications/main.ts (3)
50-50: Category tracking map enables notification-specific routing.The
notificationCategoriesmap correctly tracks the category per notification ID, enabling the click handler to route notifications appropriately.
110-126: Click handler properly resolves serverUrl and includes category.The serverUrl resolution via
getServerUrlByWebContentsId(ipcMeta.webContentsId)correctly fixes the multi-server focus bug by capturing the originating server context. The conditional spread operators cleanly handle optional fields.
94-108: Category cleanup on notification close prevents memory leaks.The close handler properly deletes the category entry from
notificationCategories, ensuring the map doesn't grow unboundedly over the application lifetime.
- Added 'nativeWindowOpen=true' back to the webview's webpreferences attribute for improved popup handling. - Changed the webview partition name from 'persist:video-call-session' to 'persist:jitsi-session' for better clarity and consistency in session management.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/videoCallWindow/video-call-window.ts (1)
603-651: Preload URL construction and webview security configuration need revisionThe code has two significant issues:
Preload URL construction using
window.location.originThe current approach relies on
window.location.originto build file:// URLs, which can behave inconsistently across browser/Electron versions. Replace withnew URL()for consistency:const preloadPath = new URL( preloadRelativePath, window.location.href ).toString();Webview security configuration
The webview enables
nodeIntegrationandallowpopupswhile loading an arbitrary URL (Jitsi). Per Electron's security guidance:
nodeIntegrationshould befalsefor any webview loading untrusted/remote content; enabling it grants Node/Electron API access to the embedded pageallowpopupsshould be removed; usewebContents.setWindowOpenHandlerin the main process instead for controlled popup handling- Remove
nativeWindowOpen=true(its behavior is managed differently in Electron 39.x; keeping it adds noise)Update to:
webview.setAttribute( 'webpreferences', 'nodeIntegration=false' ); webview.removeAttribute('allowpopups');Then implement
setWindowOpenHandlerin the main process to manage new window requests securely.Longer-term: The
<webview>tag itself is deprecated by Electron in favor ofWebContentsViewor a separateBrowserWindowwithpostMessagecommunication. Plan a migration when feasible.
🧹 Nitpick comments (4)
src/videoCallWindow/video-call-window.ts (4)
61-107: i18n initialization and retry path look solid; minor edge case on unknown languagesThe i18n setup with
invokeWithRetry,fallbackLng, and lazy resource loading is robust and should behave well under transient IPC failures. One small edge case: if the main process ever returns alanguageResult.languagethat does not exist inresources, the optional callresources[lng as keyof typeof resources]?.()will resolve toundefined, and you’ll still construct a bundle withtranslation: undefined. Consider skipping the non‑fallback block when the loader is missing to avoid an unexpected resource shape:- await i18next.init({ - lng, - fallbackLng, - resources: { - ...(lng && - lng !== fallbackLng && { - [lng]: { - translation: await resources[lng as keyof typeof resources]?.(), - }, - }), + const extraLanguage = + lng && lng !== fallbackLng && resources[lng as keyof typeof resources] + ? { + [lng]: { + translation: await resources[ + lng as keyof typeof resources + ](), + }, + } + : {}; + + await i18next.init({ + lng, + fallbackLng, + resources: { + ...extraLanguage, [fallbackLng]: { translation: await resources[fallbackLng](), }, },
202-213: Manual reload “Reloading…” label is overwritten by delayed loading overlayCurrently,
handleReloadshows the loading overlay immediately with the “Reloading…” text, but as soon as the webview emitsdid-start-loading,handleLoadStartcallsshowLoadingWithDelay(false), which overwrites the text back to the generic “Loading video call...” after the delay. That’s only a UX nit, but if you want to preserve the explicit “Reloading…” message for user-triggered reloads, you can thread that intent through:const handleLoadStart = (): void => { console.log('Video call window: Load started'); if (state.status === 'closing') { console.log( 'Video call window: Skipping load start handling - window is closing' ); return; } + const isManualReload = + state.isReloading && state.recoveryAttempt === 0; + if ( state.hasInitialLoadCompleted && !state.isReloading && state.recoveryAttempt === 0 ) { console.log( 'Video call window: Skipping loading UI - initial load already completed' ); return; } state.status = 'loading'; state.errorMessage = null; state.isReloading = false; clearAllTimeouts(); - showLoadingWithDelay(false); + showLoadingWithDelay(isManualReload); };This keeps initial loads and auto‑recovery using the generic text while preserving the “Reloading…” copy for explicit user reloads.
Also applies to: 362-409, 678-693
583-601: URL validation is a good defense-in-depth layer; consider host/origin allowlistingWrapping the webview URL in
new URL(url)and restricting protocols tohttp:/https:is a solid mitigation for the CodeQL XSS/redirect findings and avoidsjavascript:or other dangerous schemes from ever reachingwebview.src. As an additional hardening step, you could optionally enforce an allowlist of expected hosts or origins (e.g., your Jitsi/meet domains) before returningparsedUrl.href, and log/reject anything outside that set. That would further contain misuse if an upstream bug ever allowed arbitrary URLs into this code path.
653-676: Unify screen-share picker preload and open logic to avoid duplicationBoth
preloadScreenSharePickerandhandleOpenScreenPickerimplement very similar “import once, cache inscreenPickerModule/screenPickerModulePromise” logic. The duplication makes it easier for the two paths to diverge over time. You can simplify by having the IPC handler reuse the preload helper:const preloadScreenSharePicker = async (): Promise<void> => { if (screenPickerModule) { return; } try { if (process.env.NODE_ENV === 'development') { console.log('Video call window: Preloading React for screen picker'); } // Track import promise to prevent concurrent imports if (!screenPickerModulePromise) { screenPickerModulePromise = import('./screenSharePickerMount'); } screenPickerModule = await screenPickerModulePromise; screenPickerModulePromise = null; - screenPickerModule.mount(); // Mount only (stays hidden until IPC event) + screenPickerModule.mount(); // Mount only (stays hidden until IPC event) } catch (error) { console.error('Video call window: Failed to preload React:', error); screenPickerModulePromise = null; } }; // IPC listener for screen picker (with cleanup on page unload) const handleOpenScreenPicker = (): void => { (async () => { try { - if (!screenPickerModule) { - // Track import promise to prevent concurrent imports - if (!screenPickerModulePromise) { - screenPickerModulePromise = import('./screenSharePickerMount'); - } - screenPickerModule = await screenPickerModulePromise; - screenPickerModulePromise = null; - } - screenPickerModule.show(); + await preloadScreenSharePicker(); + screenPickerModule?.show(); } catch (error) { screenPickerModulePromise = null; console.error('Video call window: Failed to open screen picker:', error); } })(); };This keeps the “import once, cache, mount” behavior in a single place while the IPC handler focuses only on “ensure loaded, then show”, and your
beforeunloadcleanup already correctly removes the listener.Also applies to: 861-891
📜 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.
📒 Files selected for processing (1)
src/videoCallWindow/video-call-window.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/video-call-window.ts
🧠 Learnings (4)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (1)
src/videoCallWindow/video-call-window.ts (3)
src/videoCallWindow/screenSharePickerMount.tsx (2)
ScreenSharePickerModuleType(7-10)show(54-73)src/ipc/renderer.ts (2)
invokeWithRetry(54-115)IRetryOptions(43-52)src/i18n/common.ts (1)
fallbackLng(3-3)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 711-717: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 711-717: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">Retrying automatically in 3 seconds...</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (2)
src/videoCallWindow/video-call-window.ts (2)
232-323: Auto‑recovery strategy is well-bounded and state-awareThe three-step recovery (webview reload → URL refresh → full renderer reload) with
MAX_RECOVERY_ATTEMPTS, per-strategy delays, and state guards looks correct and avoids infinite loops. The recursive path on URL-validation failure is also safely bounded by the same counter, so you won’t spin forever even ifstate.urlis consistently invalid. No changes needed here.
695-725: Initialization, fallback UI, and global error handling are cohesive and resilientThe
start()flow does a good job of:
- Waiting for DOM readiness when needed.
- Initializing i18n before any user-facing strings render.
- Fetching the URL from both query string and IPC as a fallback.
- Using
invokeWithRetryconsistently for IPC handshakes and readiness.- Capping initialization attempts (
MAX_INIT_ATTEMPTS) and falling back to a clearly branded “Video Call Unavailable” overlay instead of silently hanging.- Logging global
errorandunhandledrejectionevents, plus wiring the reload button only once on DOMContentLoaded.The direct
innerHTMLinshowFallbackUIis safe here because it’s a static, non–user-controlled string, so the static analysis warning is effectively a false positive. As long as you keep that content fixed (or tightly controlled/localized) rather than echoing any user or remote data into it, this is fine.Also applies to: 727-850, 852-860, 893-914
…ource validation caching - Updated the screen sharing documentation to include illustrative timings in the example flow. - Added a new source validation cache mechanism to improve performance by caching the validation of selected sources for 30 seconds. - Modified the video call window HTML to provide a label for the reload button, enhancing user experience. - Refactored the video call window logic to limit automatic reload attempts, improving error handling during video call initialization.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/videoCallWindow/video-call-window.ts (1)
603-651: Simplifywebpreferencesand attach handlers before DOM insertion.Two small follow‑ups here:
nativeWindowOpen=trueis deprecated/ignored in recent Electron versions and your main process already handleswindow.openviawebContents.setWindowOpenHandler. Dropping it reduces noise without changing behavior:- webview.setAttribute( - 'webpreferences', - 'nodeIntegration,nativeWindowOpen=true' - ); + webview.setAttribute('webpreferences', 'nodeIntegration');
- For slightly more robust event handling, consider wiring listeners before inserting the
<webview>into the DOM so you never risk missing an earlydid-start-loading/webview-attachedevent:- container.appendChild(webview); - state.webview = webview; - state.url = url; - - setupWebviewEventHandlers(webview); + setupWebviewEventHandlers(webview); + container.appendChild(webview); + state.webview = webview; + state.url = url;Please double‑check against your current Electron version docs to confirm
nativeWindowOpenis indeed a no‑op in your runtime.
📜 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.
📒 Files selected for processing (4)
docs/video-call-screen-sharing.md(2 hunks)src/public/video-call-window.html(2 hunks)src/ui/main/serverView/index.ts(3 hunks)src/videoCallWindow/video-call-window.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/ui/main/serverView/index.ts
- src/public/video-call-window.html
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/video-call-window.ts
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/video-call-screen-sharing.md
🧠 Learnings (4)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/videoCallWindow/video-call-window.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/videoCallWindow/video-call-window.ts
🧬 Code graph analysis (1)
src/videoCallWindow/video-call-window.ts (3)
src/videoCallWindow/screenSharePickerMount.tsx (2)
ScreenSharePickerModuleType(7-10)show(54-73)src/ipc/renderer.ts (2)
invokeWithRetry(54-115)IRetryOptions(43-52)src/i18n/common.ts (1)
fallbackLng(3-3)
🪛 ast-grep (0.40.0)
src/videoCallWindow/video-call-window.ts
[warning] 720-726: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">${canAutoReload ? 'Retrying automatically in 3 seconds...' : 'Please close and reopen the video call.'}</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 720-726: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fallbackContainer.innerHTML = <div style="text-align: center;"> <h2 style="color: #fff; margin: 0;">Video Call Unavailable</h2> <p style="color: #ccc; margin: 10px 0;">Unable to initialize video call window</p> <p style="color: #999; margin: 10px 0; font-size: 14px;">${canAutoReload ? 'Retrying automatically in 3 seconds...' : 'Please close and reopen the video call.'}</p> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🪛 LanguageTool
docs/video-call-screen-sharing.md
[grammar] ~9-~9: Use a hyphen to join words.
Context: ...tron. ## Cache Architecture The screen sharing system uses a **stale-while-reva...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
docs/video-call-screen-sharing.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
218-218: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
270-270: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
282-282: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
296-296: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
304-304: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
311-311: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| ``` | ||
| Desktop Capturer Cache | ||
| ├── Always returns cached data immediately (instant UX) | ||
| ├── If data older than 3s, triggers background refresh | ||
| ├── Next request gets fresh data | ||
| └── Cache persists indefinitely (never expires) | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to all fenced code blocks.
Multiple code blocks lack language specification per markdownlint MD040. Specify plaintext for ASCII diagrams and text-based examples.
Apply these diffs:
Line 11 (ASCII diagram):
-```
+```plaintext
Desktop Capturer CacheLine 218 (illustrative timings):
-```
+```plaintext
T=0s: User opens picker → Cache returns instantly (2s old)Line 270 (TTL-based approach):
-```
+```plaintext
Cache hit within 3s → Return cached dataLine 282 (Stale-While-Revalidate approach):
-```
+```plaintext
Cache exists → Return immediately (always instant)Line 311 (cache lifecycle):
-```
+```plaintext
App StartAlso applies to: 218-224, 270-275, 282-287, 311-339
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/video-call-screen-sharing.md around lines 11-17, 218-224, 270-275,
282-287 and 311-339, several fenced code blocks (ASCII diagrams and text
examples) lack language identifiers; update each opening fence to use
```plaintext (e.g. change ``` to ```plaintext) for the specified blocks at lines
11, 218, 270, 282, and 311 and any other similar fences in the listed ranges so
all examples comply with markdownlint MD040.
| | Scenario | Behavior | | ||
| |----------|----------| | ||
| | Pre-warmed cache | Instant (no delay) | | ||
| | Cache hit (fresh) | Instant (no delay) | | ||
| | Cache hit (stale) | Instant return with background refresh | | ||
| | Cold start (no cache) | Requires initial system call to fetch sources | | ||
|
|
There was a problem hiding this comment.
Add blank lines after tables.
Tables should be surrounded by blank lines per markdownlint MD058. Add blank lines after the response times and memory usage tables.
Apply these diffs:
After the "Response Times" table (after line 302):
| Cold start (no cache) | Requires initial system call to fetch sources |
+
### Memory UsageAfter the "Memory Usage" table (after line 309):
| Validation cache | Negligible overhead |
+
### Cache LifecycleAlso applies to: 304-309
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
296-296: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
In docs/video-call-screen-sharing.md around lines 296–309, the Markdown tables
lack the required blank line following them (MD058). Insert a single blank line
immediately after the "Response Times" table (after line 302) and another single
blank line immediately after the "Memory Usage" table (after line 309); also
ensure any table spanning lines 304–309 is followed by a blank line so each
table is separated from subsequent text.
… parameters - Removed the unused `_params.src` parameter from the `isVideoCallWebview` function call to streamline the logic. - Adjusted the function call to ensure it only uses necessary parameters, improving code clarity and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/ui/main/serverView/index.ts (1)
262-277: Remove the underscore prefix from_paramssince it's now being used.The parameter
_paramsis accessed (.partition,.frameName), so the underscore prefix is misleading. This typically indicates an unused parameter.const handleWillAttachWebview = ( _event: Event, webPreferences: WebPreferences, - _params: Record<string, string> + params: Record<string, string> ): void => { delete webPreferences.enableBlinkFeatures; const isVideoCall = isVideoCallWebview( - _params.partition, - _params.frameName + params.partition, + params.frameName );
📜 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.
📒 Files selected for processing (1)
src/ui/main/serverView/index.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/main/serverView/index.ts
🧠 Learnings (1)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/ui/main/serverView/index.ts
🔇 Additional comments (5)
src/ui/main/serverView/index.ts (5)
55-56: LGTM!Good extraction of the preload path to a constant, addressing the previous review feedback about duplication.
57-73: LGTM!The helper function provides a clean abstraction for video call detection with a more generic partition name (
persist:video-call-session) that should work across different video call providers. The frameName fallback provides additional coverage.
75-104: LGTM!The fallback logic is well-structured with appropriate warning logs. The synchronous
fs.existsSyncis acceptable here since webview attachment is already a blocking operation and local file checks are fast.
110-120: LGTM!The reverse lookup function is correctly implemented. The O(n) iteration is acceptable given the typically small number of servers.
314-330: LGTM!Clean integration with the new helpers. The conditional spread pattern correctly handles the case where no valid preload path is available.
Linux installer download |
Windows installer download |
macOS installer download |
…tChat#3143) * feat(video-call): Enhance Windows Graphics Capture handling in RDP sessions - Implemented comprehensive disabling of Windows Graphics Capture (WGC) features to ensure fallback to the Desktop Duplication API during RDP sessions. - Updated smart restart behavior to provide clearer user guidance when toggling settings in RDP environments. - Enhanced UI to reflect the current state of WGC settings, including disabling the toggle when in RDP sessions. - Added technical details on the implementation of disabled Chrome features and their impact on screen capture reliability. This update improves the user experience and ensures consistent behavior across different session types. * fix(rollup): Update output directory for preload files - Changed the output directory for preload files from 'app' to 'app/preload' in the Rollup configuration. This adjustment improves project structure and organization for better clarity in file management. * fix(i18n): Update Russian translation for video call screen capture fallback - Revised the forced description for the video call screen capture fallback to enhance clarity and user understanding regarding its behavior in RDP sessions. The new translation provides a more accurate explanation of the feature's functionality and its impact on local launches. * refactor(videoCall): Clean up code formatting and improve readability - Reformatted the `loadFile` method call in `ipc.ts` for better readability. - Removed unnecessary blank line in `video-call-window.tsx`. - Adjusted visibility logic in `videoCallWindow.tsx` for improved clarity. * refactor(videoCall): Migrate video call window to TypeScript and enhance error/loading UI - Renamed `video-call-window.tsx` to `video-call-window.ts` and refactored the component to TypeScript for better type safety. - Introduced new loading and error overlay components with corresponding CSS for improved user experience during loading and error states. - Updated the Rollup configuration to reflect the new input file name. - Adjusted documentation to match the new file naming conventions. * refactor(videoCall): Simplify ScreenSharePicker component and improve IPC handling - Removed the onUnmount prop from ScreenSharePicker to keep the component mounted for reuse, enhancing performance. - Updated IPC event handling to ensure proper visibility management without unmounting the component. - Cleaned up code formatting for better readability in screenSharePickerMount and video-call-window files. * refactor(videoCall): Update preload script path in video call window - Changed the preload script path from using `path.join` to a relative path for improved compatibility and simplicity in the video call window component. * refactor(videoCall): Enhance ScreenSharePicker and improve preload script handling - Updated ScreenSharePicker to accept an onMounted prop for better visibility management. - Improved IPC event handling in screenSharePickerMount to ensure proper visibility state when the component is mounted. - Refactored video-call-window to construct an absolute path for the preload script, enhancing compatibility with Electron's webview requirements. * feat(videoCall): Add prewarm capturer cache functionality and enhance video call handling - Introduced a new IPC event 'video-call-window/prewarm-capturer-cache' to prewarm the desktop capturer cache for improved performance during video calls. - Updated the video call window to invoke the prewarm cache function on setup, ensuring a smoother user experience. - Enhanced the ScreenSharePicker and related components for better visibility management and optimized preload script handling. * feat(videoCall): Enhance video call window and screen sharing flow - Implemented a vanilla JavaScript bootstrap architecture for the video call window, improving performance and reducing initial load times. - Introduced a stale-while-revalidate caching mechanism for the desktop capturer, ensuring instant access to cached data while refreshing in the background. - Enhanced the ScreenSharePicker component with deferred loading, allowing for faster user interactions during screen sharing. - Updated documentation to reflect architectural changes and improved user experience features, including cache pre-warming and error handling strategies. * refactor(videoCall): Improve video call webview detection and preload handling - Enhanced the logic for identifying video call webviews using strict URL parsing and partition checks. - Implemented a function to resolve and validate preload file paths, ensuring fallback options are available if the primary preload file is missing. - Updated the ScreenSharePicker component to optimize the useEffect hook for better performance. - Revised documentation to reflect changes in caching architecture and performance characteristics. * feat(videoCall): Update screen picker IPC handling and improve URL retrieval logic - Modified the IPC event for opening the screen picker to return a success status, enhancing error handling. - Improved URL retrieval logic in the video call window to request the URL via IPC if not provided in query parameters, ensuring a smoother user experience. - Added comments for clarity and better understanding of the code flow in the video call window and screen share picker components. * fix(videoCall): Reset screen picker promise on error to prevent stale state - Added a line to set `screenPickerModulePromise` to null in the error handling block of the screen picker opening function, ensuring that the promise state is reset if an error occurs while attempting to open the screen picker. * fix(videoCall): Add missing type import for screenPickerModulePromise - Included a type import for `screenPickerModulePromise` to ensure proper type checking and consistency in the video call window component. * fix(videoCall): Ensure visibility state is reset on screen share picker mount - Moved the reset of `shouldShowOnMount` to the callback function to maintain proper visibility management when the screen share picker is shown. * Fix markdownlint MD040: Add language identifiers to code blocks * feat(videoCall): Enhance video call webview handling and URL validation - Added support for both `persist:jitsi-session` and `persist:pexip-session` partitions in video call webviews to ensure proper identification and preload script application. - Introduced a URL validation function to enforce strict checks on video call URLs, allowing only specific protocols and known hosts, improving security and reliability. - Updated the preload script path handling for consistency and clarity across components. - Enhanced the ScreenSharePicker module with a new type definition for better type safety. * feat(videoCall): Revise video call window documentation and enhance provider support - Updated the video call window documentation to clarify support for any video conferencing provider, emphasizing the generic, provider-agnostic architecture. - Enhanced URL validation to remove provider-specific patterns, allowing seamless integration with various video call services. - Improved the handling of webview partitions to use a generic partition name for all providers, ensuring consistent behavior across different video call implementations. - Added detailed sections on provider-specific requirements for Jitsi and PEXIP, highlighting necessary components and integration steps. * fix(videoCall): Simplify webview webpreferences attribute - Removed the 'nativeWindowOpen=true' option from the webview's webpreferences attribute, streamlining the configuration for improved clarity and consistency. * feat(notifications): Add category field to notifications for better classification - Introduced a 'category' field in notification options, allowing notifications to be classified as 'DOWNLOADS' or 'SERVER'. - Updated various notification creation points across the application to include the new category field, enhancing the organization and handling of notifications. - Adjusted tests to verify the inclusion of the category in notification payloads, ensuring consistency in notification behavior. * fix(videoCall): Update webview preferences and partition naming - Added 'nativeWindowOpen=true' back to the webview's webpreferences attribute for improved popup handling. - Changed the webview partition name from 'persist:video-call-session' to 'persist:jitsi-session' for better clarity and consistency in session management. * feat(videoCall): Enhance screen sharing documentation and implement source validation caching - Updated the screen sharing documentation to include illustrative timings in the example flow. - Added a new source validation cache mechanism to improve performance by caching the validation of selected sources for 30 seconds. - Modified the video call window HTML to provide a label for the reload button, enhancing user experience. - Refactored the video call window logic to limit automatic reload attempts, improving error handling during video call initialization. * fix(videoCall): Update video call webview handling by removing unused parameters - Removed the unused `_params.src` parameter from the `isVideoCallWebview` function call to streamline the logic. - Adjusted the function call to ensure it only uses necessary parameters, improving code clarity and maintainability.
…tChat#3143) * feat(video-call): Enhance Windows Graphics Capture handling in RDP sessions - Implemented comprehensive disabling of Windows Graphics Capture (WGC) features to ensure fallback to the Desktop Duplication API during RDP sessions. - Updated smart restart behavior to provide clearer user guidance when toggling settings in RDP environments. - Enhanced UI to reflect the current state of WGC settings, including disabling the toggle when in RDP sessions. - Added technical details on the implementation of disabled Chrome features and their impact on screen capture reliability. This update improves the user experience and ensures consistent behavior across different session types. * fix(rollup): Update output directory for preload files - Changed the output directory for preload files from 'app' to 'app/preload' in the Rollup configuration. This adjustment improves project structure and organization for better clarity in file management. * fix(i18n): Update Russian translation for video call screen capture fallback - Revised the forced description for the video call screen capture fallback to enhance clarity and user understanding regarding its behavior in RDP sessions. The new translation provides a more accurate explanation of the feature's functionality and its impact on local launches. * refactor(videoCall): Clean up code formatting and improve readability - Reformatted the `loadFile` method call in `ipc.ts` for better readability. - Removed unnecessary blank line in `video-call-window.tsx`. - Adjusted visibility logic in `videoCallWindow.tsx` for improved clarity. * refactor(videoCall): Migrate video call window to TypeScript and enhance error/loading UI - Renamed `video-call-window.tsx` to `video-call-window.ts` and refactored the component to TypeScript for better type safety. - Introduced new loading and error overlay components with corresponding CSS for improved user experience during loading and error states. - Updated the Rollup configuration to reflect the new input file name. - Adjusted documentation to match the new file naming conventions. * refactor(videoCall): Simplify ScreenSharePicker component and improve IPC handling - Removed the onUnmount prop from ScreenSharePicker to keep the component mounted for reuse, enhancing performance. - Updated IPC event handling to ensure proper visibility management without unmounting the component. - Cleaned up code formatting for better readability in screenSharePickerMount and video-call-window files. * refactor(videoCall): Update preload script path in video call window - Changed the preload script path from using `path.join` to a relative path for improved compatibility and simplicity in the video call window component. * refactor(videoCall): Enhance ScreenSharePicker and improve preload script handling - Updated ScreenSharePicker to accept an onMounted prop for better visibility management. - Improved IPC event handling in screenSharePickerMount to ensure proper visibility state when the component is mounted. - Refactored video-call-window to construct an absolute path for the preload script, enhancing compatibility with Electron's webview requirements. * feat(videoCall): Add prewarm capturer cache functionality and enhance video call handling - Introduced a new IPC event 'video-call-window/prewarm-capturer-cache' to prewarm the desktop capturer cache for improved performance during video calls. - Updated the video call window to invoke the prewarm cache function on setup, ensuring a smoother user experience. - Enhanced the ScreenSharePicker and related components for better visibility management and optimized preload script handling. * feat(videoCall): Enhance video call window and screen sharing flow - Implemented a vanilla JavaScript bootstrap architecture for the video call window, improving performance and reducing initial load times. - Introduced a stale-while-revalidate caching mechanism for the desktop capturer, ensuring instant access to cached data while refreshing in the background. - Enhanced the ScreenSharePicker component with deferred loading, allowing for faster user interactions during screen sharing. - Updated documentation to reflect architectural changes and improved user experience features, including cache pre-warming and error handling strategies. * refactor(videoCall): Improve video call webview detection and preload handling - Enhanced the logic for identifying video call webviews using strict URL parsing and partition checks. - Implemented a function to resolve and validate preload file paths, ensuring fallback options are available if the primary preload file is missing. - Updated the ScreenSharePicker component to optimize the useEffect hook for better performance. - Revised documentation to reflect changes in caching architecture and performance characteristics. * feat(videoCall): Update screen picker IPC handling and improve URL retrieval logic - Modified the IPC event for opening the screen picker to return a success status, enhancing error handling. - Improved URL retrieval logic in the video call window to request the URL via IPC if not provided in query parameters, ensuring a smoother user experience. - Added comments for clarity and better understanding of the code flow in the video call window and screen share picker components. * fix(videoCall): Reset screen picker promise on error to prevent stale state - Added a line to set `screenPickerModulePromise` to null in the error handling block of the screen picker opening function, ensuring that the promise state is reset if an error occurs while attempting to open the screen picker. * fix(videoCall): Add missing type import for screenPickerModulePromise - Included a type import for `screenPickerModulePromise` to ensure proper type checking and consistency in the video call window component. * fix(videoCall): Ensure visibility state is reset on screen share picker mount - Moved the reset of `shouldShowOnMount` to the callback function to maintain proper visibility management when the screen share picker is shown. * Fix markdownlint MD040: Add language identifiers to code blocks * feat(videoCall): Enhance video call webview handling and URL validation - Added support for both `persist:jitsi-session` and `persist:pexip-session` partitions in video call webviews to ensure proper identification and preload script application. - Introduced a URL validation function to enforce strict checks on video call URLs, allowing only specific protocols and known hosts, improving security and reliability. - Updated the preload script path handling for consistency and clarity across components. - Enhanced the ScreenSharePicker module with a new type definition for better type safety. * feat(videoCall): Revise video call window documentation and enhance provider support - Updated the video call window documentation to clarify support for any video conferencing provider, emphasizing the generic, provider-agnostic architecture. - Enhanced URL validation to remove provider-specific patterns, allowing seamless integration with various video call services. - Improved the handling of webview partitions to use a generic partition name for all providers, ensuring consistent behavior across different video call implementations. - Added detailed sections on provider-specific requirements for Jitsi and PEXIP, highlighting necessary components and integration steps. * fix(videoCall): Simplify webview webpreferences attribute - Removed the 'nativeWindowOpen=true' option from the webview's webpreferences attribute, streamlining the configuration for improved clarity and consistency. * feat(notifications): Add category field to notifications for better classification - Introduced a 'category' field in notification options, allowing notifications to be classified as 'DOWNLOADS' or 'SERVER'. - Updated various notification creation points across the application to include the new category field, enhancing the organization and handling of notifications. - Adjusted tests to verify the inclusion of the category in notification payloads, ensuring consistency in notification behavior. * fix(videoCall): Update webview preferences and partition naming - Added 'nativeWindowOpen=true' back to the webview's webpreferences attribute for improved popup handling. - Changed the webview partition name from 'persist:video-call-session' to 'persist:jitsi-session' for better clarity and consistency in session management. * feat(videoCall): Enhance screen sharing documentation and implement source validation caching - Updated the screen sharing documentation to include illustrative timings in the example flow. - Added a new source validation cache mechanism to improve performance by caching the validation of selected sources for 30 seconds. - Modified the video call window HTML to provide a label for the reload button, enhancing user experience. - Refactored the video call window logic to limit automatic reload attempts, improving error handling during video call initialization. * fix(videoCall): Update video call webview handling by removing unused parameters - Removed the unused `_params.src` parameter from the `isVideoCallWebview` function call to streamline the logic. - Adjusted the function call to ensure it only uses necessary parameters, improving code clarity and maintainability.
…tChat#3143) * feat(video-call): Enhance Windows Graphics Capture handling in RDP sessions - Implemented comprehensive disabling of Windows Graphics Capture (WGC) features to ensure fallback to the Desktop Duplication API during RDP sessions. - Updated smart restart behavior to provide clearer user guidance when toggling settings in RDP environments. - Enhanced UI to reflect the current state of WGC settings, including disabling the toggle when in RDP sessions. - Added technical details on the implementation of disabled Chrome features and their impact on screen capture reliability. This update improves the user experience and ensures consistent behavior across different session types. * fix(rollup): Update output directory for preload files - Changed the output directory for preload files from 'app' to 'app/preload' in the Rollup configuration. This adjustment improves project structure and organization for better clarity in file management. * fix(i18n): Update Russian translation for video call screen capture fallback - Revised the forced description for the video call screen capture fallback to enhance clarity and user understanding regarding its behavior in RDP sessions. The new translation provides a more accurate explanation of the feature's functionality and its impact on local launches. * refactor(videoCall): Clean up code formatting and improve readability - Reformatted the `loadFile` method call in `ipc.ts` for better readability. - Removed unnecessary blank line in `video-call-window.tsx`. - Adjusted visibility logic in `videoCallWindow.tsx` for improved clarity. * refactor(videoCall): Migrate video call window to TypeScript and enhance error/loading UI - Renamed `video-call-window.tsx` to `video-call-window.ts` and refactored the component to TypeScript for better type safety. - Introduced new loading and error overlay components with corresponding CSS for improved user experience during loading and error states. - Updated the Rollup configuration to reflect the new input file name. - Adjusted documentation to match the new file naming conventions. * refactor(videoCall): Simplify ScreenSharePicker component and improve IPC handling - Removed the onUnmount prop from ScreenSharePicker to keep the component mounted for reuse, enhancing performance. - Updated IPC event handling to ensure proper visibility management without unmounting the component. - Cleaned up code formatting for better readability in screenSharePickerMount and video-call-window files. * refactor(videoCall): Update preload script path in video call window - Changed the preload script path from using `path.join` to a relative path for improved compatibility and simplicity in the video call window component. * refactor(videoCall): Enhance ScreenSharePicker and improve preload script handling - Updated ScreenSharePicker to accept an onMounted prop for better visibility management. - Improved IPC event handling in screenSharePickerMount to ensure proper visibility state when the component is mounted. - Refactored video-call-window to construct an absolute path for the preload script, enhancing compatibility with Electron's webview requirements. * feat(videoCall): Add prewarm capturer cache functionality and enhance video call handling - Introduced a new IPC event 'video-call-window/prewarm-capturer-cache' to prewarm the desktop capturer cache for improved performance during video calls. - Updated the video call window to invoke the prewarm cache function on setup, ensuring a smoother user experience. - Enhanced the ScreenSharePicker and related components for better visibility management and optimized preload script handling. * feat(videoCall): Enhance video call window and screen sharing flow - Implemented a vanilla JavaScript bootstrap architecture for the video call window, improving performance and reducing initial load times. - Introduced a stale-while-revalidate caching mechanism for the desktop capturer, ensuring instant access to cached data while refreshing in the background. - Enhanced the ScreenSharePicker component with deferred loading, allowing for faster user interactions during screen sharing. - Updated documentation to reflect architectural changes and improved user experience features, including cache pre-warming and error handling strategies. * refactor(videoCall): Improve video call webview detection and preload handling - Enhanced the logic for identifying video call webviews using strict URL parsing and partition checks. - Implemented a function to resolve and validate preload file paths, ensuring fallback options are available if the primary preload file is missing. - Updated the ScreenSharePicker component to optimize the useEffect hook for better performance. - Revised documentation to reflect changes in caching architecture and performance characteristics. * feat(videoCall): Update screen picker IPC handling and improve URL retrieval logic - Modified the IPC event for opening the screen picker to return a success status, enhancing error handling. - Improved URL retrieval logic in the video call window to request the URL via IPC if not provided in query parameters, ensuring a smoother user experience. - Added comments for clarity and better understanding of the code flow in the video call window and screen share picker components. * fix(videoCall): Reset screen picker promise on error to prevent stale state - Added a line to set `screenPickerModulePromise` to null in the error handling block of the screen picker opening function, ensuring that the promise state is reset if an error occurs while attempting to open the screen picker. * fix(videoCall): Add missing type import for screenPickerModulePromise - Included a type import for `screenPickerModulePromise` to ensure proper type checking and consistency in the video call window component. * fix(videoCall): Ensure visibility state is reset on screen share picker mount - Moved the reset of `shouldShowOnMount` to the callback function to maintain proper visibility management when the screen share picker is shown. * Fix markdownlint MD040: Add language identifiers to code blocks * feat(videoCall): Enhance video call webview handling and URL validation - Added support for both `persist:jitsi-session` and `persist:pexip-session` partitions in video call webviews to ensure proper identification and preload script application. - Introduced a URL validation function to enforce strict checks on video call URLs, allowing only specific protocols and known hosts, improving security and reliability. - Updated the preload script path handling for consistency and clarity across components. - Enhanced the ScreenSharePicker module with a new type definition for better type safety. * feat(videoCall): Revise video call window documentation and enhance provider support - Updated the video call window documentation to clarify support for any video conferencing provider, emphasizing the generic, provider-agnostic architecture. - Enhanced URL validation to remove provider-specific patterns, allowing seamless integration with various video call services. - Improved the handling of webview partitions to use a generic partition name for all providers, ensuring consistent behavior across different video call implementations. - Added detailed sections on provider-specific requirements for Jitsi and PEXIP, highlighting necessary components and integration steps. * fix(videoCall): Simplify webview webpreferences attribute - Removed the 'nativeWindowOpen=true' option from the webview's webpreferences attribute, streamlining the configuration for improved clarity and consistency. * feat(notifications): Add category field to notifications for better classification - Introduced a 'category' field in notification options, allowing notifications to be classified as 'DOWNLOADS' or 'SERVER'. - Updated various notification creation points across the application to include the new category field, enhancing the organization and handling of notifications. - Adjusted tests to verify the inclusion of the category in notification payloads, ensuring consistency in notification behavior. * fix(videoCall): Update webview preferences and partition naming - Added 'nativeWindowOpen=true' back to the webview's webpreferences attribute for improved popup handling. - Changed the webview partition name from 'persist:video-call-session' to 'persist:jitsi-session' for better clarity and consistency in session management. * feat(videoCall): Enhance screen sharing documentation and implement source validation caching - Updated the screen sharing documentation to include illustrative timings in the example flow. - Added a new source validation cache mechanism to improve performance by caching the validation of selected sources for 30 seconds. - Modified the video call window HTML to provide a label for the reload button, enhancing user experience. - Refactored the video call window logic to limit automatic reload attempts, improving error handling during video call initialization. * fix(videoCall): Update video call webview handling by removing unused parameters - Removed the unused `_params.src` parameter from the `isVideoCallWebview` function call to streamline the logic. - Adjusted the function call to ensure it only uses necessary parameters, improving code clarity and maintainability.
…tChat#3143) * feat(video-call): Enhance Windows Graphics Capture handling in RDP sessions - Implemented comprehensive disabling of Windows Graphics Capture (WGC) features to ensure fallback to the Desktop Duplication API during RDP sessions. - Updated smart restart behavior to provide clearer user guidance when toggling settings in RDP environments. - Enhanced UI to reflect the current state of WGC settings, including disabling the toggle when in RDP sessions. - Added technical details on the implementation of disabled Chrome features and their impact on screen capture reliability. This update improves the user experience and ensures consistent behavior across different session types. * fix(rollup): Update output directory for preload files - Changed the output directory for preload files from 'app' to 'app/preload' in the Rollup configuration. This adjustment improves project structure and organization for better clarity in file management. * fix(i18n): Update Russian translation for video call screen capture fallback - Revised the forced description for the video call screen capture fallback to enhance clarity and user understanding regarding its behavior in RDP sessions. The new translation provides a more accurate explanation of the feature's functionality and its impact on local launches. * refactor(videoCall): Clean up code formatting and improve readability - Reformatted the `loadFile` method call in `ipc.ts` for better readability. - Removed unnecessary blank line in `video-call-window.tsx`. - Adjusted visibility logic in `videoCallWindow.tsx` for improved clarity. * refactor(videoCall): Migrate video call window to TypeScript and enhance error/loading UI - Renamed `video-call-window.tsx` to `video-call-window.ts` and refactored the component to TypeScript for better type safety. - Introduced new loading and error overlay components with corresponding CSS for improved user experience during loading and error states. - Updated the Rollup configuration to reflect the new input file name. - Adjusted documentation to match the new file naming conventions. * refactor(videoCall): Simplify ScreenSharePicker component and improve IPC handling - Removed the onUnmount prop from ScreenSharePicker to keep the component mounted for reuse, enhancing performance. - Updated IPC event handling to ensure proper visibility management without unmounting the component. - Cleaned up code formatting for better readability in screenSharePickerMount and video-call-window files. * refactor(videoCall): Update preload script path in video call window - Changed the preload script path from using `path.join` to a relative path for improved compatibility and simplicity in the video call window component. * refactor(videoCall): Enhance ScreenSharePicker and improve preload script handling - Updated ScreenSharePicker to accept an onMounted prop for better visibility management. - Improved IPC event handling in screenSharePickerMount to ensure proper visibility state when the component is mounted. - Refactored video-call-window to construct an absolute path for the preload script, enhancing compatibility with Electron's webview requirements. * feat(videoCall): Add prewarm capturer cache functionality and enhance video call handling - Introduced a new IPC event 'video-call-window/prewarm-capturer-cache' to prewarm the desktop capturer cache for improved performance during video calls. - Updated the video call window to invoke the prewarm cache function on setup, ensuring a smoother user experience. - Enhanced the ScreenSharePicker and related components for better visibility management and optimized preload script handling. * feat(videoCall): Enhance video call window and screen sharing flow - Implemented a vanilla JavaScript bootstrap architecture for the video call window, improving performance and reducing initial load times. - Introduced a stale-while-revalidate caching mechanism for the desktop capturer, ensuring instant access to cached data while refreshing in the background. - Enhanced the ScreenSharePicker component with deferred loading, allowing for faster user interactions during screen sharing. - Updated documentation to reflect architectural changes and improved user experience features, including cache pre-warming and error handling strategies. * refactor(videoCall): Improve video call webview detection and preload handling - Enhanced the logic for identifying video call webviews using strict URL parsing and partition checks. - Implemented a function to resolve and validate preload file paths, ensuring fallback options are available if the primary preload file is missing. - Updated the ScreenSharePicker component to optimize the useEffect hook for better performance. - Revised documentation to reflect changes in caching architecture and performance characteristics. * feat(videoCall): Update screen picker IPC handling and improve URL retrieval logic - Modified the IPC event for opening the screen picker to return a success status, enhancing error handling. - Improved URL retrieval logic in the video call window to request the URL via IPC if not provided in query parameters, ensuring a smoother user experience. - Added comments for clarity and better understanding of the code flow in the video call window and screen share picker components. * fix(videoCall): Reset screen picker promise on error to prevent stale state - Added a line to set `screenPickerModulePromise` to null in the error handling block of the screen picker opening function, ensuring that the promise state is reset if an error occurs while attempting to open the screen picker. * fix(videoCall): Add missing type import for screenPickerModulePromise - Included a type import for `screenPickerModulePromise` to ensure proper type checking and consistency in the video call window component. * fix(videoCall): Ensure visibility state is reset on screen share picker mount - Moved the reset of `shouldShowOnMount` to the callback function to maintain proper visibility management when the screen share picker is shown. * Fix markdownlint MD040: Add language identifiers to code blocks * feat(videoCall): Enhance video call webview handling and URL validation - Added support for both `persist:jitsi-session` and `persist:pexip-session` partitions in video call webviews to ensure proper identification and preload script application. - Introduced a URL validation function to enforce strict checks on video call URLs, allowing only specific protocols and known hosts, improving security and reliability. - Updated the preload script path handling for consistency and clarity across components. - Enhanced the ScreenSharePicker module with a new type definition for better type safety. * feat(videoCall): Revise video call window documentation and enhance provider support - Updated the video call window documentation to clarify support for any video conferencing provider, emphasizing the generic, provider-agnostic architecture. - Enhanced URL validation to remove provider-specific patterns, allowing seamless integration with various video call services. - Improved the handling of webview partitions to use a generic partition name for all providers, ensuring consistent behavior across different video call implementations. - Added detailed sections on provider-specific requirements for Jitsi and PEXIP, highlighting necessary components and integration steps. * fix(videoCall): Simplify webview webpreferences attribute - Removed the 'nativeWindowOpen=true' option from the webview's webpreferences attribute, streamlining the configuration for improved clarity and consistency. * feat(notifications): Add category field to notifications for better classification - Introduced a 'category' field in notification options, allowing notifications to be classified as 'DOWNLOADS' or 'SERVER'. - Updated various notification creation points across the application to include the new category field, enhancing the organization and handling of notifications. - Adjusted tests to verify the inclusion of the category in notification payloads, ensuring consistency in notification behavior. * fix(videoCall): Update webview preferences and partition naming - Added 'nativeWindowOpen=true' back to the webview's webpreferences attribute for improved popup handling. - Changed the webview partition name from 'persist:video-call-session' to 'persist:jitsi-session' for better clarity and consistency in session management. * feat(videoCall): Enhance screen sharing documentation and implement source validation caching - Updated the screen sharing documentation to include illustrative timings in the example flow. - Added a new source validation cache mechanism to improve performance by caching the validation of selected sources for 30 seconds. - Modified the video call window HTML to provide a label for the reload button, enhancing user experience. - Refactored the video call window logic to limit automatic reload attempts, improving error handling during video call initialization. * fix(videoCall): Update video call webview handling by removing unused parameters - Removed the unused `_params.src` parameter from the `isVideoCallWebview` function call to streamline the logic. - Adjusted the function call to ensure it only uses necessary parameters, improving code clarity and maintainability.
…tChat#3143) * feat(video-call): Enhance Windows Graphics Capture handling in RDP sessions - Implemented comprehensive disabling of Windows Graphics Capture (WGC) features to ensure fallback to the Desktop Duplication API during RDP sessions. - Updated smart restart behavior to provide clearer user guidance when toggling settings in RDP environments. - Enhanced UI to reflect the current state of WGC settings, including disabling the toggle when in RDP sessions. - Added technical details on the implementation of disabled Chrome features and their impact on screen capture reliability. This update improves the user experience and ensures consistent behavior across different session types. * fix(rollup): Update output directory for preload files - Changed the output directory for preload files from 'app' to 'app/preload' in the Rollup configuration. This adjustment improves project structure and organization for better clarity in file management. * fix(i18n): Update Russian translation for video call screen capture fallback - Revised the forced description for the video call screen capture fallback to enhance clarity and user understanding regarding its behavior in RDP sessions. The new translation provides a more accurate explanation of the feature's functionality and its impact on local launches. * refactor(videoCall): Clean up code formatting and improve readability - Reformatted the `loadFile` method call in `ipc.ts` for better readability. - Removed unnecessary blank line in `video-call-window.tsx`. - Adjusted visibility logic in `videoCallWindow.tsx` for improved clarity. * refactor(videoCall): Migrate video call window to TypeScript and enhance error/loading UI - Renamed `video-call-window.tsx` to `video-call-window.ts` and refactored the component to TypeScript for better type safety. - Introduced new loading and error overlay components with corresponding CSS for improved user experience during loading and error states. - Updated the Rollup configuration to reflect the new input file name. - Adjusted documentation to match the new file naming conventions. * refactor(videoCall): Simplify ScreenSharePicker component and improve IPC handling - Removed the onUnmount prop from ScreenSharePicker to keep the component mounted for reuse, enhancing performance. - Updated IPC event handling to ensure proper visibility management without unmounting the component. - Cleaned up code formatting for better readability in screenSharePickerMount and video-call-window files. * refactor(videoCall): Update preload script path in video call window - Changed the preload script path from using `path.join` to a relative path for improved compatibility and simplicity in the video call window component. * refactor(videoCall): Enhance ScreenSharePicker and improve preload script handling - Updated ScreenSharePicker to accept an onMounted prop for better visibility management. - Improved IPC event handling in screenSharePickerMount to ensure proper visibility state when the component is mounted. - Refactored video-call-window to construct an absolute path for the preload script, enhancing compatibility with Electron's webview requirements. * feat(videoCall): Add prewarm capturer cache functionality and enhance video call handling - Introduced a new IPC event 'video-call-window/prewarm-capturer-cache' to prewarm the desktop capturer cache for improved performance during video calls. - Updated the video call window to invoke the prewarm cache function on setup, ensuring a smoother user experience. - Enhanced the ScreenSharePicker and related components for better visibility management and optimized preload script handling. * feat(videoCall): Enhance video call window and screen sharing flow - Implemented a vanilla JavaScript bootstrap architecture for the video call window, improving performance and reducing initial load times. - Introduced a stale-while-revalidate caching mechanism for the desktop capturer, ensuring instant access to cached data while refreshing in the background. - Enhanced the ScreenSharePicker component with deferred loading, allowing for faster user interactions during screen sharing. - Updated documentation to reflect architectural changes and improved user experience features, including cache pre-warming and error handling strategies. * refactor(videoCall): Improve video call webview detection and preload handling - Enhanced the logic for identifying video call webviews using strict URL parsing and partition checks. - Implemented a function to resolve and validate preload file paths, ensuring fallback options are available if the primary preload file is missing. - Updated the ScreenSharePicker component to optimize the useEffect hook for better performance. - Revised documentation to reflect changes in caching architecture and performance characteristics. * feat(videoCall): Update screen picker IPC handling and improve URL retrieval logic - Modified the IPC event for opening the screen picker to return a success status, enhancing error handling. - Improved URL retrieval logic in the video call window to request the URL via IPC if not provided in query parameters, ensuring a smoother user experience. - Added comments for clarity and better understanding of the code flow in the video call window and screen share picker components. * fix(videoCall): Reset screen picker promise on error to prevent stale state - Added a line to set `screenPickerModulePromise` to null in the error handling block of the screen picker opening function, ensuring that the promise state is reset if an error occurs while attempting to open the screen picker. * fix(videoCall): Add missing type import for screenPickerModulePromise - Included a type import for `screenPickerModulePromise` to ensure proper type checking and consistency in the video call window component. * fix(videoCall): Ensure visibility state is reset on screen share picker mount - Moved the reset of `shouldShowOnMount` to the callback function to maintain proper visibility management when the screen share picker is shown. * Fix markdownlint MD040: Add language identifiers to code blocks * feat(videoCall): Enhance video call webview handling and URL validation - Added support for both `persist:jitsi-session` and `persist:pexip-session` partitions in video call webviews to ensure proper identification and preload script application. - Introduced a URL validation function to enforce strict checks on video call URLs, allowing only specific protocols and known hosts, improving security and reliability. - Updated the preload script path handling for consistency and clarity across components. - Enhanced the ScreenSharePicker module with a new type definition for better type safety. * feat(videoCall): Revise video call window documentation and enhance provider support - Updated the video call window documentation to clarify support for any video conferencing provider, emphasizing the generic, provider-agnostic architecture. - Enhanced URL validation to remove provider-specific patterns, allowing seamless integration with various video call services. - Improved the handling of webview partitions to use a generic partition name for all providers, ensuring consistent behavior across different video call implementations. - Added detailed sections on provider-specific requirements for Jitsi and PEXIP, highlighting necessary components and integration steps. * fix(videoCall): Simplify webview webpreferences attribute - Removed the 'nativeWindowOpen=true' option from the webview's webpreferences attribute, streamlining the configuration for improved clarity and consistency. * feat(notifications): Add category field to notifications for better classification - Introduced a 'category' field in notification options, allowing notifications to be classified as 'DOWNLOADS' or 'SERVER'. - Updated various notification creation points across the application to include the new category field, enhancing the organization and handling of notifications. - Adjusted tests to verify the inclusion of the category in notification payloads, ensuring consistency in notification behavior. * fix(videoCall): Update webview preferences and partition naming - Added 'nativeWindowOpen=true' back to the webview's webpreferences attribute for improved popup handling. - Changed the webview partition name from 'persist:video-call-session' to 'persist:jitsi-session' for better clarity and consistency in session management. * feat(videoCall): Enhance screen sharing documentation and implement source validation caching - Updated the screen sharing documentation to include illustrative timings in the example flow. - Added a new source validation cache mechanism to improve performance by caching the validation of selected sources for 30 seconds. - Modified the video call window HTML to provide a label for the reload button, enhancing user experience. - Refactored the video call window logic to limit automatic reload attempts, improving error handling during video call initialization. * fix(videoCall): Update video call webview handling by removing unused parameters - Removed the unused `_params.src` parameter from the `isVideoCallWebview` function call to streamline the logic. - Adjusted the function call to ensure it only uses necessary parameters, improving code clarity and maintainability.
2 participants
Fix Windows Graphics Capture Handling in RDP Sessions
Summary
This PR resolves a critical issue where users joining video calls (Jitsi) through Rocket.Chat Desktop on Windows get stuck on a frozen "Loading video call" screen when running in Remote Desktop (RDP) sessions. The app now automatically detects RDP environments and forces the reliable Desktop Duplication API for screen capture, while also providing users with a new Screen Capture Fallback toggle in Settings for local sessions that need this workaround.
The Problem
Affected Users: Windows 10/11 users joining video calls from RDP sessions
Symptoms:
Root Cause:
Chromium's WebRTC implementation prefers Windows Graphics Capture (WGC) for screen capture on Windows 10+. However, WGC cannot capture remote desktop sessions and reports "Source is not capturable". This causes WebRTC to fail silently, and the loading overlay never clears.
Why This Matters:
Solution Overview
The fix implements a two-pronged approach to reliably fallback to the Desktop Duplication API:
Changes Made
Automatic RDP Detection and Feature Disabling
When the app launches on Windows, it checks
process.env.SESSIONNAME:If RDP is detected, the app automatically disables Windows Graphics Capture features before any windows are created. This happens silently in the background without requiring user action.
Implementation (
src/app/main/app.ts):WebRtcAllowWgcScreenCapturerto the list of disabled Chromium features (in addition to existingWebRtcAllowWgcDesktopCapturer)app.commandLine.appendSwitchbefore window creationNew "Screen Capture Fallback" Setting
A new toggle is now visible in Settings → General on Windows, allowing users to manually enable the fallback for local sessions.
UI Behavior:
Implementation (
src/ui/components/SettingsView/features/ScreenCaptureFallback.tsx):isFallbackForcedis trueVideo Call Window Flag Propagation
To ensure flags propagate to the Jitsi webview inside the video call window, the fix implements a second layer of flag application.
Implementation (
src/videoCallWindow/ipc.ts):--disable-features=WebRtcAllowWgcDesktopCapturer,WebRtcAllowWgcScreenCapturerviaadditionalArgumentsin BrowserWindow webPreferencesThis dual-layer approach ensures WGC disabling is enforced both globally and specifically for the webview renderer process.
Improved User Messaging
Russian Translation Update (
src/i18n/ru.i18n.json):Build Configuration Fix
Rollup Configuration (
rollup.config.mjs):apptoapp/preloadfor better project structure organizationUI Color Scheme Consistency
Screen Share Picker (
src/videoCallWindow/screenSharePicker.tsx):backgroundColor='surface'→surface-lightfor better contrast and consistencycolor='default'→color='font-default'for proper text renderingselected→surface-selectedbackground,selected→font-on-darktextlight→surface-tintbackground for better visual hierarchySmart Restart Behavior
When users toggle the Screen Capture Fallback setting:
Implementation (
src/app/main/app.ts):Testing
Test 1: RDP Session Auto-Detection
isRdpSession: trueTest 2: Local Session with Manual Toggle
isScreenCaptureFallbackEnabled: trueTest 3: Session Switching
Test 4: Color Scheme Consistency
surface-tintbackground withfont-defaulttextsurface-selectedbackground withfont-on-darktextTest 5: Cross-Platform Verification
Impact & Benefits
For Users:
For Enterprise Deployments:
For Implementation:
Technical References
Windows Graphics Capture Limitations
Related Documentation
docs/video-call-window-wgc-limitations.mdfor comprehensive technical details on the implementation, including code locations, Chromium feature flags, and architectural considerations for webview flag inheritanceBreaking Changes
None. This is a backward-compatible enhancement.
Notes for Reviewers
Dual-Layer Flag Application: The flags are applied at both app startup and video call window creation. This is intentional to handle Chromium's webview process isolation.
RDP Detection Timing: RDP detection happens once at startup via
process.env.SESSIONNAME. If a user switches between sessions (local to RDP or vice versa), they must restart the app for the correct backend to be used.User Setting Persistence: The user's toggle preference is persisted across restarts. If they enable the fallback and then restart in an RDP session, the setting value doesn't change - but RDP detection takes precedence to disable WGC.
Console Logging: Detailed logging is included to help debug if issues occur. Look for "Video call window: Explicitly passing WGC disable flags" in console output.
Color Token Updates: The screen share picker was updated to use correct Fuselage design tokens (
surface-light,font-default, etc.). These are cosmetic improvements for consistency but don't change functionality.SUP-864
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.