fix: Screen picker not loading again after closing by clicking outside it#3205
Merged
jeanfbrito merged 2 commits intomasterfrom Feb 20, 2026
Merged
fix: Screen picker not loading again after closing by clicking outside it#3205jeanfbrito merged 2 commits intomasterfrom
jeanfbrito merged 2 commits intomasterfrom
Conversation
- Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain
Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX.
Contributor
WalkthroughThis pull request fixes the screen sharing button lock issue by restructuring the IPC communication flow between the screen picker, Electron window, and JitsiBridge. Changes include installing a screen obtainer hook early, removing double-close calls, ensuring picker dismissals send proper cancellation responses, and adding a new URL opener handler. A new translation entry for system preferences is included. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/videoCallWindow/preload/jitsiBridge.ts (3)
340-347: No-op message listener adds a permanent event listener towindow.
setupMessageEventListenernow registers an empty callback that will never be removed. If it's truly reserved for future use, consider deferring registration until there's actual logic, or adding aTODOso it isn't forgotten.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/videoCallWindow/preload/jitsiBridge.ts` around lines 340 - 347, The no-op listener added in setupMessageEventListener registers window.addEventListener('message', ...) with an empty callback and is never removed; either remove this registration now or change it to a named handler that you register only when needed (or add a clear TODO) and ensure it is removed during teardown (use a stored handler and call window.removeEventListener('message', handler) from your cleanup/dispose method); locate setupMessageEventListener and the anonymous message callback to implement one of these fixes.
393-396:removeAllListenersis a blunt instrument — acceptable here but worth documenting.
ipcRenderer.removeAllListeners('video-call-window/screen-sharing-source-responded')will remove all renderer-side listeners on this channel, not just the ones registered by this function. Currently that's safe because jitsiBridge is the sole renderer-side consumer, but a comment noting this assumption would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/videoCallWindow/preload/jitsiBridge.ts` around lines 393 - 396, The call to ipcRenderer.removeAllListeners('video-call-window/screen-sharing-source-responded') in jitsiBridge is destructive because it removes all renderer-side listeners for that channel; update the code by adding a clear comment above the call stating the explicit assumption that jitsiBridge is the sole renderer consumer of that channel (so removeAllListeners is safe), and optionally (preferred) change the implementation to remove only the specific handler by keeping a reference to the handler function and calling ipcRenderer.removeListener('video-call-window/screen-sharing-source-responded', handler) in the relevant cleanup path (identify the handler registration near this call to implement the targeted removal).
364-366: Misleading comment: code overwrites rather than wraps.Line 366 says "If Jitsi has already set it, we wrap the existing implementation," but the code unconditionally overwrites
window.JitsiMeetScreenObtaineron line 434. Since this runs in preload before page JS, the overwrite is safe, but the comment should be corrected to avoid confusion.Suggested comment fix
- * We install this on window immediately so it is available before Jitsi's - * JS runs. If Jitsi has already set it, we wrap the existing implementation. + * We install this on window immediately (in preload) so it is available + * before Jitsi's JS runs. lib-jitsi-meet checks for its existence and + * uses it instead of creating its own.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/videoCallWindow/preload/jitsiBridge.ts` around lines 364 - 366, The comment above the installation of window.JitsiMeetScreenObtainer is misleading: it claims we "wrap the existing implementation" when the code unconditionally overwrites window.JitsiMeetScreenObtainer; update the comment near the installation of window.JitsiMeetScreenObtainer to state that the preload script installs/overwrites the global implementation (safe because preload runs before page JS) instead of saying it wraps an existing implementation, and mention that any existing value will be replaced.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/i18n/en.i18n.jsonsrc/ui/components/Dialog/hooks.tssrc/videoCallWindow/ipc.tssrc/videoCallWindow/preload/jitsiBridge.tssrc/videoCallWindow/screenSharePicker.tsx
💤 Files with no reviewable changes (1)
- src/ui/components/Dialog/hooks.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/videoCallWindow/screenSharePicker.tsxsrc/videoCallWindow/preload/jitsiBridge.tssrc/videoCallWindow/ipc.ts
🧬 Code graph analysis (1)
src/videoCallWindow/ipc.ts (2)
src/ipc/renderer.ts (1)
handle(6-36)src/utils/browserLauncher.ts (1)
openExternal(58-90)
⏰ 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 (7)
src/videoCallWindow/screenSharePicker.tsx (3)
41-45: Well-designed safety net for IPC response lifecycle.The
responseSentRef/wasVisibleRefpattern correctly covers all dismissal paths (click-outside, ESC, programmatic close) and the effect ordering guarantees no double-sends becauseresponseSentRefis set synchronously beforesetVisible(false)in bothhandleShareandhandleClose.Also applies to: 107-121
169-175: Correct ordering prevents double IPC responses.Setting
responseSentRef.current = truebeforesetVisible(false)inhandleShareensures the safety-net effect andhandleClosewon't fire a second response. The guard inhandleClosecorrectly short-circuits when a response was already sent.Also applies to: 178-186
246-264: I need the review comment and context (such as the code being reviewed, file names, and any change details) to proceed with verification and rewriting. Please provide the review comment and relevant code context.src/videoCallWindow/preload/jitsiBridge.ts (3)
111-113: Good: early installation before Jitsi's JS runs.Installing the screen obtainer in the constructor (preload phase) ensures
window.JitsiMeetScreenObtaineris available whenlib-jitsi-meet'sScreenObtainer.obtainScreenOnElectronchecks for it.
405-421: UsingipcRenderer.oninstead of.once— works correctly with the manualcleanup()call.The handler calls
cleanup()as its first action (line 408), which removes the listener immediately. This is functionally equivalent to.oncebut gives explicit control over removal timing. Just ensure no exception can be thrown beforecleanup()runs — currently safe sincecleanup()is the first statement.
441-446: No callers ofstartScreenSharing()exist in the codebase.The method serves as an API compatibility placeholder and is documented accordingly. The actual screen sharing flow is delegated to the
openDesktopPickerhook (line 434). Since no code internally calls this method or inspects its return value, returningfalseposes no risk of misinterpretation.src/videoCallWindow/ipc.ts (1)
355-359: Good fix: marking completion before the callback prevents re-entrancy deadlock.Moving
markScreenSharingComplete()beforecb()ensures that if Jitsi synchronously triggers a newgetDisplayMediarequest inside the callback,isScreenSharingRequestPendingis already cleared. This directly addresses the "screen picker won't reopen" regression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/en.i18n.json`:
- Line 465: The key "openSystemPreferences" should use a platform-neutral label
to match existing strings; update its value to "Open Settings" or replace usages
to reuse the existing keys dialog.mediaPermission.openSettings /
dialog.microphonePermissionDenied.openSettings so wording is consistent across
the app and avoids macOS-specific "System Preferences" naming; ensure any
components referencing openSystemPreferences are updated to the chosen key/value
for consistency.
In `@src/videoCallWindow/ipc.ts`:
- Around line 516-536: The ipcMain.once listener registered for
'video-call-window/screen-sharing-source-responded' is anonymous and not
tracked, so cleanupScreenSharingListener() can't remove it; update the
registration to create a named/assigned listener (e.g., assign the callback to
activeScreenSharingListener) and use ipcMain.once/handle accordingly so
cleanupScreenSharingListener() can call ipcMain.removeListener or removeOnce on
that stored listener before registering a new one; ensure the code that sends
the picker
(videoCallWindow.webContents.send('video-call-window/open-screen-picker')) still
forwards the response to callerWebContents.send and that you check
callerWebContents.isDestroyed() inside the tracked callback.
- Around line 504-506: The video-call-window handler for
'video-call-window/open-url' calls openExternal(url) without validating the URL;
update the handler (handle('video-call-window/open-url')) to parse and validate
the URL scheme before calling openExternal by reusing the existing
isProtocolAllowed() helper (or same scheme checks used elsewhere) and explicitly
allow OS-level protocols like x-apple.systempreferences: and ms-settings: to be
passed to the OS shell instead of routed through browserLauncher.ts; if the
scheme is not allowed, reject/throw or return an error and do not call
openExternal.
---
Nitpick comments:
In `@src/videoCallWindow/preload/jitsiBridge.ts`:
- Around line 340-347: The no-op listener added in setupMessageEventListener
registers window.addEventListener('message', ...) with an empty callback and is
never removed; either remove this registration now or change it to a named
handler that you register only when needed (or add a clear TODO) and ensure it
is removed during teardown (use a stored handler and call
window.removeEventListener('message', handler) from your cleanup/dispose
method); locate setupMessageEventListener and the anonymous message callback to
implement one of these fixes.
- Around line 393-396: The call to
ipcRenderer.removeAllListeners('video-call-window/screen-sharing-source-responded')
in jitsiBridge is destructive because it removes all renderer-side listeners for
that channel; update the code by adding a clear comment above the call stating
the explicit assumption that jitsiBridge is the sole renderer consumer of that
channel (so removeAllListeners is safe), and optionally (preferred) change the
implementation to remove only the specific handler by keeping a reference to the
handler function and calling
ipcRenderer.removeListener('video-call-window/screen-sharing-source-responded',
handler) in the relevant cleanup path (identify the handler registration near
this call to implement the targeted removal).
- Around line 364-366: The comment above the installation of
window.JitsiMeetScreenObtainer is misleading: it claims we "wrap the existing
implementation" when the code unconditionally overwrites
window.JitsiMeetScreenObtainer; update the comment near the installation of
window.JitsiMeetScreenObtainer to state that the preload script
installs/overwrites the global implementation (safe because preload runs before
page JS) instead of saying it wraps an existing implementation, and mention that
any existing value will be replaced.
| "permissionDenied": "Screen Recording Permission Denied", | ||
| "permissionRequired": "Screen recording permission is required to share your screen.", | ||
| "permissionInstructions": "Please enable it in your system preferences and try again.", | ||
| "openSystemPreferences": "Open System Preferences", |
Contributor
There was a problem hiding this comment.
Consider a more platform-neutral label, or at least align with macOS Ventura+ naming.
The string "Open System Preferences" uses pre-macOS-Ventura terminology (Apple renamed it to "System Settings" in macOS 13). It's also Windows-inaccurate since Windows calls the destination "Settings." The existing dialog.mediaPermission.openSettings and dialog.microphonePermissionDenied.openSettings both use the more neutral "Open Settings" — consider reusing one of those keys or adopting the same wording for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/i18n/en.i18n.json` at line 465, The key "openSystemPreferences" should
use a platform-neutral label to match existing strings; update its value to
"Open Settings" or replace usages to reuse the existing keys
dialog.mediaPermission.openSettings /
dialog.microphonePermissionDenied.openSettings so wording is consistent across
the app and avoids macOS-specific "System Preferences" naming; ensure any
components referencing openSystemPreferences are updated to the chosen key/value
for consistency.
Comment on lines
+504
to
+506
| handle('video-call-window/open-url', async (_webContents, url) => { | ||
| await openExternal(url); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check openExternal implementation and how it handles protocols
rg -n 'openExternal' --type ts -B 3 -A 5Repository: RocketChat/Rocket.Chat.Electron
Length of output: 14925
🏁 Script executed:
#!/bin/bash
# Find browserLauncher.ts and examine the openExternal function implementation
find . -type f -name 'browserLauncher.ts' -o -name 'browserLauncher.js'Repository: RocketChat/Rocket.Chat.Electron
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Check for URL validation patterns in other IPC handlers
rg -n 'handle\(' src/videoCallWindow/ipc.ts -A 5 | head -100Repository: RocketChat/Rocket.Chat.Electron
Length of output: 4014
🏁 Script executed:
#!/bin/bash
# Find screenSharePicker.tsx and check what URLs are being passed
find . -type f -name 'screenSharePicker.tsx' -o -name 'screenSharePicker.ts'Repository: RocketChat/Rocket.Chat.Electron
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Search for references to video-call-window/open-url IPC call
rg -n "video-call-window/open-url" --type ts --type tsx -B 3 -A 3Repository: RocketChat/Rocket.Chat.Electron
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Check isProtocolAllowed function to see what validation pattern it uses
rg -n "isProtocolAllowed" --type ts -A 10 | head -50Repository: RocketChat/Rocket.Chat.Electron
Length of output: 3238
🏁 Script executed:
#!/bin/bash
# Check screenSharePicker.tsx for how it invokes video-call-window/open-url
cat ./src/videoCallWindow/screenSharePicker.tsx | head -300Repository: RocketChat/Rocket.Chat.Electron
Length of output: 9526
🏁 Script executed:
#!/bin/bash
# Get the isProtocolAllowed implementation to understand the validation pattern
rg -n "function isProtocolAllowed|export.*isProtocolAllowed" --type ts -A 30Repository: RocketChat/Rocket.Chat.Electron
Length of output: 1633
🏁 Script executed:
#!/bin/bash
# Search for all places that invoke 'video-call-window/open-url'
rg -n "video-call-window/open-url" --type ts --type jsRepository: RocketChat/Rocket.Chat.Electron
Length of output: 468
🏁 Script executed:
#!/bin/bash
# Check if there are any other renderers or bridges that could invoke this handler
rg -n "ipcRenderer.invoke.*open-url" --type ts --type jsRepository: RocketChat/Rocket.Chat.Electron
Length of output: 195
Add URL validation to the open-url handler for consistency with other handlers.
This handler currently accepts any URL without validation, unlike other openExternal() calls in the codebase which use isProtocolAllowed() or explicit scheme checks (lines 919, 951). Additionally, the openExternal helper in browserLauncher.ts may route x-apple.systempreferences: / ms-settings: protocol URLs through a custom browser instead of the OS shell, causing silent failures when a custom browser is selected in settings.
Consider using isProtocolAllowed() like other handlers, or validate that only expected OS protocol handlers are allowed:
Example using isProtocolAllowed
- handle('video-call-window/open-url', async (_webContents, url) => {
- await openExternal(url);
- });
+ handle('video-call-window/open-url', async (_webContents, url) => {
+ if (await isProtocolAllowed(url)) {
+ await openExternal(url);
+ }
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| handle('video-call-window/open-url', async (_webContents, url) => { | |
| await openExternal(url); | |
| }); | |
| handle('video-call-window/open-url', async (_webContents, url) => { | |
| if (await isProtocolAllowed(url)) { | |
| await openExternal(url); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/videoCallWindow/ipc.ts` around lines 504 - 506, The video-call-window
handler for 'video-call-window/open-url' calls openExternal(url) without
validating the URL; update the handler (handle('video-call-window/open-url')) to
parse and validate the URL scheme before calling openExternal by reusing the
existing isProtocolAllowed() helper (or same scheme checks used elsewhere) and
explicitly allow OS-level protocols like x-apple.systempreferences: and
ms-settings: to be passed to the OS shell instead of routed through
browserLauncher.ts; if the scheme is not allowed, reject/throw or return an
error and do not call openExternal.
Comment on lines
+516
to
536
| // Clean up any stale listener before registering a new one, to ensure only | ||
| // one ipcMain listener is active at a time (same pattern as createInternalPickerHandler). | ||
| cleanupScreenSharingListener(); | ||
|
|
||
| videoCallWindow.webContents.send('video-call-window/open-screen-picker'); | ||
|
|
||
| // Forward the picker response back to the calling webContents (e.g. the Jitsi webview | ||
| // preload that called ipcRenderer.invoke here). The screenSharePicker renderer sends | ||
| // the result via ipcRenderer.send → ipcMain; we relay it to the caller so that | ||
| // jitsiBridge's ipcRenderer.on listener fires correctly. | ||
| ipcMain.once( | ||
| 'video-call-window/screen-sharing-source-responded', | ||
| (_event, sourceId: string | null) => { | ||
| if (!callerWebContents.isDestroyed()) { | ||
| callerWebContents.send( | ||
| 'video-call-window/screen-sharing-source-responded', | ||
| sourceId | ||
| ); | ||
| } | ||
| } | ||
| ); |
Contributor
There was a problem hiding this comment.
Untracked ipcMain.once listener — not cleaned up by cleanupScreenSharingListener().
The listener registered at line 526 is anonymous and not assigned to activeScreenSharingListener, so cleanupScreenSharingListener() (called on window close, new internal-picker requests, etc.) cannot remove it. If the video call window is destroyed while a Jitsi-initiated picker is awaiting a response, this orphaned listener will fire on the next screen-sharing-source-responded event from a completely different request.
Consider tracking this listener so it can be cleaned up, or removing any prior listener on this channel before registering:
Proposed tracking fix
// Clean up any stale listener before registering a new one, to ensure only
// one ipcMain listener is active at a time (same pattern as createInternalPickerHandler).
cleanupScreenSharingListener();
videoCallWindow.webContents.send('video-call-window/open-screen-picker');
- ipcMain.once(
+ const relayListener = (_event: Event, sourceId: string | null) => {
+ if (!callerWebContents.isDestroyed()) {
+ callerWebContents.send(
+ 'video-call-window/screen-sharing-source-responded',
+ sourceId
+ );
+ }
+ };
+
+ activeScreenSharingListener = relayListener as any;
+ isScreenSharingRequestPending = true;
+
+ ipcMain.once(
'video-call-window/screen-sharing-source-responded',
- (_event, sourceId: string | null) => {
- if (!callerWebContents.isDestroyed()) {
- callerWebContents.send(
- 'video-call-window/screen-sharing-source-responded',
- sourceId
- );
- }
- }
+ relayListener
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Clean up any stale listener before registering a new one, to ensure only | |
| // one ipcMain listener is active at a time (same pattern as createInternalPickerHandler). | |
| cleanupScreenSharingListener(); | |
| videoCallWindow.webContents.send('video-call-window/open-screen-picker'); | |
| // Forward the picker response back to the calling webContents (e.g. the Jitsi webview | |
| // preload that called ipcRenderer.invoke here). The screenSharePicker renderer sends | |
| // the result via ipcRenderer.send → ipcMain; we relay it to the caller so that | |
| // jitsiBridge's ipcRenderer.on listener fires correctly. | |
| ipcMain.once( | |
| 'video-call-window/screen-sharing-source-responded', | |
| (_event, sourceId: string | null) => { | |
| if (!callerWebContents.isDestroyed()) { | |
| callerWebContents.send( | |
| 'video-call-window/screen-sharing-source-responded', | |
| sourceId | |
| ); | |
| } | |
| } | |
| ); | |
| // Clean up any stale listener before registering a new one, to ensure only | |
| // one ipcMain listener is active at a time (same pattern as createInternalPickerHandler). | |
| cleanupScreenSharingListener(); | |
| videoCallWindow.webContents.send('video-call-window/open-screen-picker'); | |
| // Forward the picker response back to the calling webContents (e.g. the Jitsi webview | |
| // preload that called ipcRenderer.invoke here). The screenSharePicker renderer sends | |
| // the result via ipcRenderer.send → ipcMain; we relay it to the caller so that | |
| // jitsiBridge's ipcRenderer.on listener fires correctly. | |
| const relayListener = (_event: Event, sourceId: string | null) => { | |
| if (!callerWebContents.isDestroyed()) { | |
| callerWebContents.send( | |
| 'video-call-window/screen-sharing-source-responded', | |
| sourceId | |
| ); | |
| } | |
| }; | |
| activeScreenSharingListener = relayListener as any; | |
| isScreenSharingRequestPending = true; | |
| ipcMain.once( | |
| 'video-call-window/screen-sharing-source-responded', | |
| relayListener | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/videoCallWindow/ipc.ts` around lines 516 - 536, The ipcMain.once listener
registered for 'video-call-window/screen-sharing-source-responded' is anonymous
and not tracked, so cleanupScreenSharingListener() can't remove it; update the
registration to create a named/assigned listener (e.g., assign the callback to
activeScreenSharingListener) and use ipcMain.once/handle accordingly so
cleanupScreenSharingListener() can call ipcMain.removeListener or removeOnce on
that stored listener before registering a new one; ensure the code that sends
the picker
(videoCallWindow.webContents.send('video-call-window/open-screen-picker')) still
forwards the response to callerWebContents.send and that you check
callerWebContents.isDestroyed() inside the tracked callback.
jeanfbrito
added a commit
that referenced
this pull request
Feb 23, 2026
* feat: Add Exchange/EWS debugging patches and error classification (#3187) * chore(theme): transparency mode not removing background of server view (#3156) * Language update from Lingohub 🤖 (#3165) Project Name: Rocket.Chat.Electron Project Link: https://app.lingohub.com/project/pr_1Ag2Vlx6MWNt-16038/branches/prb_16rm9BiWK53b-4144 User: Lingohub Robot Easy language translations with Lingohub 🚀 Co-authored-by: lingohub[bot] <69908207+lingohub[bot]@users.noreply.github.com> * feat: Implement user theme preference settings (#3160) * feat: Implement user theme preference settings and remove legacy theme appearance handling - Introduced a new `ThemeAppearance` component to manage user theme preferences, allowing selection between 'auto', 'light', and 'dark' themes. - Updated state management to include `userThemePreference`, replacing the previous `themeAppearance` handling. - Removed deprecated theme appearance logic from various components and files, streamlining the codebase. - Added internationalization support for theme appearance settings across multiple languages. - Enhanced the UI to reflect user-selected theme preferences dynamically. * fix(i18n): Correct Norwegian translation for theme appearance description * fix(theme): Validate theme preference values before dispatching - Updated the `handleChangeTheme` function to include validation for theme preference values, ensuring only 'auto', 'light', or 'dark' are accepted. This change prevents invalid values from being dispatched, enhancing the robustness of the theme management logic. * refactor(DocumentViewer): Update theme management to utilize Redux state for user preferences - Replaced the use of `useDarkMode` with Redux selectors to determine the theme based on user preferences and machine theme. - Enhanced theme logic to support 'auto', 'light', and 'dark' settings, improving the flexibility and responsiveness of the theme management in the DocumentViewer component. * refactor(DocumentViewer): Simplify theme management by removing Redux dependencies - Eliminated the use of Redux selectors for theme management in the DocumentViewer component, replacing it with a static 'tint' background and default color settings. - Streamlined the component's code by removing unnecessary theme logic, enhancing readability and maintainability. * chore: Clean up code by removing unnecessary blank lines in ThemeAppearance, TransparentWindow, and userThemePreference files * fix: Address PR review comments and restore API compatibility - Remove trailing blank lines from ThemeAppearance.tsx, TransparentWindow.tsx, and userThemePreference.ts - Restore setUserThemeAppearance as no-op function for backwards compatibility with @rocket.chat/desktop-api interface * fix: resolve 91 security vulnerabilities in dependencies (#3173) * fix: resolve 91 security vulnerabilities in dependencies - Update axios 1.6.4 -> 1.13.2 (SSRF, DoS, credential leakage) - Update electron-updater 5.3.0 -> 6.3.9 (code signing bypass) - Update rollup 4.9.6 -> 4.32.0 (DOM clobbering XSS) - Update glob 11.0.3 -> 11.1.0 in workspace (command injection) - Add resolutions for transitive dependencies: - cross-spawn, braces, ws, follow-redirects - form-data, tar-fs, undici - Add comprehensive security remediation documentation * docs: fix markdown lint - add language specifier to code block * chore: Remove security documentation from repository Security vulnerability remediation documentation kept locally for reference. * fix: Issues in German translation (#3155) * chore: Upgrade Electron and Node.js versions, update README and packa… (#3179) * chore: Upgrade Electron and Node.js versions, update README and package configurations - Updated Electron dependency from version 39.2.5 to 40.0.0 in package.json and yarn.lock. - Bumped Node.js version requirements in package.json and devEngines to >=24.11.1. - Revised README.md to reflect new supported platforms and minimum version requirements. - Removed deprecated tests related to ELECTRON_OZONE_PLATFORM_HINT in app.main.spec.ts. - Enhanced documentation for development prerequisites and troubleshooting sections. * chore: Bump version numbers in configuration files - Updated the bundle version in electron-builder.json from 26010 to 26011. - Incremented the application version in package.json from 4.11.1 to 4.12.0. * docs: Update README to reflect new platform support and installation formats - Revised the supported platforms section to include additional architectures and installation formats for Windows, macOS, and Linux. - Updated download links for Microsoft Store and Mac App Store, ensuring accurate access to application sources. * docs: Revise README layout for download links - Updated the formatting of download links for Microsoft Store, Mac App Store, and Snap Store to improve visual presentation and accessibility. - Changed from a div-based layout to a paragraph-based layout with adjusted image sizes for better responsiveness. * chore: Update @types/node version in package.json and yarn.lock - Upgraded @types/node from version 16.18.69 to 25.0.10 in both package.json and yarn.lock to ensure compatibility with the latest TypeScript features and improvements. * chore: Enable alpha releases (#3180) * chore: Upgrade Electron and Node.js versions, update README and package configurations - Updated Electron dependency from version 39.2.5 to 40.0.0 in package.json and yarn.lock. - Bumped Node.js version requirements in package.json and devEngines to >=24.11.1. - Revised README.md to reflect new supported platforms and minimum version requirements. - Removed deprecated tests related to ELECTRON_OZONE_PLATFORM_HINT in app.main.spec.ts. - Enhanced documentation for development prerequisites and troubleshooting sections. * chore: Bump version numbers in configuration files - Updated the bundle version in electron-builder.json from 26010 to 26011. - Incremented the application version in package.json from 4.11.1 to 4.12.0. * docs: Update README to reflect new platform support and installation formats - Revised the supported platforms section to include additional architectures and installation formats for Windows, macOS, and Linux. - Updated download links for Microsoft Store and Mac App Store, ensuring accurate access to application sources. * docs: Revise README layout for download links - Updated the formatting of download links for Microsoft Store, Mac App Store, and Snap Store to improve visual presentation and accessibility. - Changed from a div-based layout to a paragraph-based layout with adjusted image sizes for better responsiveness. * docs: Add alpha release process documentation - Introduced a new document detailing the alpha release process for the Rocket.Chat Desktop app, including channel definitions, versioning guidelines, and steps for creating and publishing alpha releases. - Included instructions for users to opt into the alpha channel and troubleshooting tips for common issues. * chore: Update architecture support and Node.js version requirements - Added 'arm64' architecture support to the build targets in electron-builder.json for NSIS, MSI, and ZIP formats. - Lowered the minimum Node.js version requirement in package.json from >=24.11.1 to >=20.0.0 for better compatibility. * chore: Change develop branch to dev for release workflow Update build-release workflow and desktop-release-action to use 'dev' branch instead of 'develop' for development releases. * chore: Update versioning and add release tag script - Bumped version in package.json to 4.12.0.alpha.1. - Added scripts/release-tag.ts for automated release tagging. - Updated .eslintignore to exclude the new scripts directory. * chore: Correct version format in package.json - Updated version format in package.json from "4.12.0.alpha.1" to "4.12.0-alpha.1" for consistency. * chore: Update all workflows to use dev branch instead of develop - validate-pr.yml: Add dev to PR target branches - powershell-lint.yml: Change develop to dev - pull-request-build.yml: Change develop to dev * fix: Normalize tags for consistent comparison in release-tag script Strip leading 'v' prefix when comparing tags to handle both v-prefixed and non-prefixed tag formats consistently. * chore: Increment bundle version in electron-builder.json to 26012 * chore: Address nitpick comments in release-tag script - Add comment explaining why /scripts is excluded from eslint - Return null on exec error to distinguish from empty output - Add warning when git tag list fails - Use -- separator in git commands for safety * fix: Add jsign to GITHUB_PATH in Windows CI setup The jsign tool was being installed but not added to PATH for subsequent steps. This caused the "Verify tools" step to fail with "jsign not found". * chore: Bump version to 4.12.0-alpha.2 - Updated version in package.json to 4.12.0-alpha.2 - Incremented bundleVersion in electron-builder.json to 26013 * docs: Add QA testing guide for alpha channel updates * docs: Rename alpha docs to pre-release and fix workflow concurrency - Rename alpha-release-process.md to pre-release-process.md - Add beta release documentation - Add detailed channel switching instructions - Fix concurrency group using github.ref instead of github.head_ref (github.head_ref is empty for push events, causing tag builds to cancel) * feat(outlook): add @ewsjs/xhr debugging patches Add comprehensive NTLM authentication debugging to @ewsjs/xhr library: - patches-src/ directory structure for maintainable patches - Enhanced ntlmProvider.ts with detailed NTLM handshake logging - Enhanced xhrApi.ts with HTTP request/response debugging - Yarn patch resolution for @ewsjs/[email protected] - apply-patches.sh script for regenerating patches Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-Claude) Co-authored-by: Sisyphus <[email protected]> * feat(outlook): add type definitions for calendar sync Add error-related type definitions to support error classification: - ErrorSource: exchange, rocket_chat, desktop_app, network, authentication, configuration - ErrorSeverity: low, medium, high, critical - OutlookCalendarError: full error object with context - ErrorClassification: pattern matching result type Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-Claude) Co-authored-by: Sisyphus <[email protected]> * feat(outlook): add error classification system Add comprehensive error classification for Outlook calendar sync: - Pattern-based error detection for Exchange, Rocket.Chat, and desktop errors - Automatic severity and source classification - User-friendly error messages with suggested actions - Structured logging format for debugging - Support for NTLM auth, network, SSL, and credential errors Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-Claude) Co-authored-by: Sisyphus <[email protected]> * feat(outlook): enhance calendar sync with debugging and mutex * test(outlook): add tests for getOutlookEvents * feat(outlook): add logging infrastructure for calendar debugging * chore: fix linting issues for Outlook calendar debugging - Exclude patches-src/ from eslint (not part of main build) - Fix has-credentials handler return type to match expected signature Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-Claude) Co-authored-by: Sisyphus <[email protected]> * fix: address CodeRabbit review issues for Outlook calendar - Fix console transport recursion by using originalConsole in writeFn - Fix infinite recursion in redactObject using destructuring - Remove NTLM Type 3 message logging (contains credentials) - Fix queued sync promises never resolving by tracking resolve/reject - Fix unhandled async errors in preload using .then().catch() - Accept HTTP 2xx status codes instead of only 200 - Fix URL validation to check pathname instead of full URL - Update tests to match actual implementation behavior * feat(settings): add Developer tab with verbose Outlook logging toggle - Add Developer tab in Settings (only visible when developer mode enabled) - Add verbose Outlook logging toggle to control [OutlookCalendar] console output - Add colored console output for better visibility on dark themes - Redirect to General tab when developer mode disabled while on Developer tab - Create centralized logger (outlookLog, outlookError, etc.) in src/outlookCalendar/logger.ts - Convert all direct console.log calls to use centralized logger - Fix infinite recursion bug in patches (verboseLog calling itself) - Add AGENTS.md documentation files for knowledge management - Use theme-aware colors for Settings UI text * fix(ci): remove conflicting patch-package patch for @ewsjs/xhr The @ewsjs/xhr package is already patched via Yarn's patch protocol (.yarn/patches/). The patch-package patch was accidentally added and conflicts with the already-applied Yarn patch, causing CI failures. * docs: add patching mechanism documentation to AGENTS.md Clarify that @ewsjs/xhr uses Yarn patch protocol (.yarn/patches/) while patch-package (patches/) is only for other packages. This prevents accidental CI breakage from conflicting patches. * fix: address CodeRabbit review comments - logger.ts: Use shared prefix constants instead of duplicating strings - getOutlookEvents.ts: Replace Promise.reject() with throw statements - getOutlookEvents.ts: Route console.error through outlookError - ipc.ts: Route all console.* through outlookLog/outlookWarn/outlookError - ipc.ts: Replace Promise.reject(e) with throw e - AGENTS.md: Fix markdown formatting and update versions * fix(outlook): address CodeRabbit review issues - Add JSDoc to syncEventsWithRocketChatServer documenting sync coalescing - Remove isSyncInProgress check in initial sync (let queue handle it) - Remove logging implementation details test (tested console.log colors) * chore: remove unused patches-src directory The debugging code in patches-src/ was never applied - only the minimal bug fix in .yarn/patches/ is used. Removing dead code to avoid confusion. * fix: address all code review issues from PR #3187 review CRITICAL fixes: - Support multi-server sync state (Map instead of globals) - Fix Promise<Promise<boolean>> return type - Use JSON.stringify for safe string escaping in executeJavaScript MAJOR fixes: - Add RocketChat calendar event types for type safety - CRUD operations now return {success, error?} instead of swallowing errors - Replace sync fs.appendFileSync with async fs.promises.appendFile - Add useId() and htmlFor for accessibility in ThemeAppearance - Apply privacy redaction to all transports (not just file) MINOR fixes: - Extract magic numbers to named constants - Extract duplicate buildEwsPathname helper function - Remove unused _context parameter from classifyError - Remove fire-and-forget connectivity test calls - Add originalConsole fallback in preload logging - Optimize getComponentContext to skip stack trace for log/info/debug - Fix email regex typo: [A-Z|a-z] -> [A-Za-z] - Fix double timestamp in createClassifiedError - Replace inline style with Fuselage pt prop * fix(outlook): fix race condition in sync queue processing Changed 'if' to 'while' loop to ensure all queued syncs are processed. Previously, syncs queued while lastSync.run() was executing would be lost because the queue was cleared before processing started. * fix: address additional code review issues - Fix pool exhaustion bug in context.ts: add overflow counter fallback when availableServerIds is depleted, emit warning with diagnostics - Fix PII leak in ipc.ts error logging: move sensitive fields (subject, responseData) to verbose-only outlookLog calls at 5 locations - Fix silent failure in performSync: throw error instead of silent return when eventsOnRocketChatServer fetch fails * fix(logging): add captureComponentStack parameter to getLogContext Allows callers to opt into stack-based component detection by passing captureComponentStack=true, while preserving default behavior. --------- Co-authored-by: Rodrigo Nascimento <[email protected]> Co-authored-by: lingohub[bot] <69908207+lingohub[bot]@users.noreply.github.com> Co-authored-by: Max Lee <[email protected]> Co-authored-by: Sisyphus <[email protected]> * feat: Add scoped logging infrastructure and log viewer window (#3186) * chore(theme): transparency mode not removing background of server view (#3156) * Language update from Lingohub 🤖 (#3165) Project Name: Rocket.Chat.Electron Project Link: https://app.lingohub.com/project/pr_1Ag2Vlx6MWNt-16038/branches/prb_16rm9BiWK53b-4144 User: Lingohub Robot Easy language translations with Lingohub 🚀 Co-authored-by: lingohub[bot] <69908207+lingohub[bot]@users.noreply.github.com> * feat: Implement user theme preference settings (#3160) * feat: Implement user theme preference settings and remove legacy theme appearance handling - Introduced a new `ThemeAppearance` component to manage user theme preferences, allowing selection between 'auto', 'light', and 'dark' themes. - Updated state management to include `userThemePreference`, replacing the previous `themeAppearance` handling. - Removed deprecated theme appearance logic from various components and files, streamlining the codebase. - Added internationalization support for theme appearance settings across multiple languages. - Enhanced the UI to reflect user-selected theme preferences dynamically. * fix(i18n): Correct Norwegian translation for theme appearance description * fix(theme): Validate theme preference values before dispatching - Updated the `handleChangeTheme` function to include validation for theme preference values, ensuring only 'auto', 'light', or 'dark' are accepted. This change prevents invalid values from being dispatched, enhancing the robustness of the theme management logic. * refactor(DocumentViewer): Update theme management to utilize Redux state for user preferences - Replaced the use of `useDarkMode` with Redux selectors to determine the theme based on user preferences and machine theme. - Enhanced theme logic to support 'auto', 'light', and 'dark' settings, improving the flexibility and responsiveness of the theme management in the DocumentViewer component. * refactor(DocumentViewer): Simplify theme management by removing Redux dependencies - Eliminated the use of Redux selectors for theme management in the DocumentViewer component, replacing it with a static 'tint' background and default color settings. - Streamlined the component's code by removing unnecessary theme logic, enhancing readability and maintainability. * chore: Clean up code by removing unnecessary blank lines in ThemeAppearance, TransparentWindow, and userThemePreference files * fix: Address PR review comments and restore API compatibility - Remove trailing blank lines from ThemeAppearance.tsx, TransparentWindow.tsx, and userThemePreference.ts - Restore setUserThemeAppearance as no-op function for backwards compatibility with @rocket.chat/desktop-api interface * fix: resolve 91 security vulnerabilities in dependencies (#3173) * fix: resolve 91 security vulnerabilities in dependencies - Update axios 1.6.4 -> 1.13.2 (SSRF, DoS, credential leakage) - Update electron-updater 5.3.0 -> 6.3.9 (code signing bypass) - Update rollup 4.9.6 -> 4.32.0 (DOM clobbering XSS) - Update glob 11.0.3 -> 11.1.0 in workspace (command injection) - Add resolutions for transitive dependencies: - cross-spawn, braces, ws, follow-redirects - form-data, tar-fs, undici - Add comprehensive security remediation documentation * docs: fix markdown lint - add language specifier to code block * chore: Remove security documentation from repository Security vulnerability remediation documentation kept locally for reference. * fix: Issues in German translation (#3155) * chore: Upgrade Electron and Node.js versions, update README and packa… (#3179) * chore: Upgrade Electron and Node.js versions, update README and package configurations - Updated Electron dependency from version 39.2.5 to 40.0.0 in package.json and yarn.lock. - Bumped Node.js version requirements in package.json and devEngines to >=24.11.1. - Revised README.md to reflect new supported platforms and minimum version requirements. - Removed deprecated tests related to ELECTRON_OZONE_PLATFORM_HINT in app.main.spec.ts. - Enhanced documentation for development prerequisites and troubleshooting sections. * chore: Bump version numbers in configuration files - Updated the bundle version in electron-builder.json from 26010 to 26011. - Incremented the application version in package.json from 4.11.1 to 4.12.0. * docs: Update README to reflect new platform support and installation formats - Revised the supported platforms section to include additional architectures and installation formats for Windows, macOS, and Linux. - Updated download links for Microsoft Store and Mac App Store, ensuring accurate access to application sources. * docs: Revise README layout for download links - Updated the formatting of download links for Microsoft Store, Mac App Store, and Snap Store to improve visual presentation and accessibility. - Changed from a div-based layout to a paragraph-based layout with adjusted image sizes for better responsiveness. * chore: Update @types/node version in package.json and yarn.lock - Upgraded @types/node from version 16.18.69 to 25.0.10 in both package.json and yarn.lock to ensure compatibility with the latest TypeScript features and improvements. * chore: Enable alpha releases (#3180) * chore: Upgrade Electron and Node.js versions, update README and package configurations - Updated Electron dependency from version 39.2.5 to 40.0.0 in package.json and yarn.lock. - Bumped Node.js version requirements in package.json and devEngines to >=24.11.1. - Revised README.md to reflect new supported platforms and minimum version requirements. - Removed deprecated tests related to ELECTRON_OZONE_PLATFORM_HINT in app.main.spec.ts. - Enhanced documentation for development prerequisites and troubleshooting sections. * chore: Bump version numbers in configuration files - Updated the bundle version in electron-builder.json from 26010 to 26011. - Incremented the application version in package.json from 4.11.1 to 4.12.0. * docs: Update README to reflect new platform support and installation formats - Revised the supported platforms section to include additional architectures and installation formats for Windows, macOS, and Linux. - Updated download links for Microsoft Store and Mac App Store, ensuring accurate access to application sources. * docs: Revise README layout for download links - Updated the formatting of download links for Microsoft Store, Mac App Store, and Snap Store to improve visual presentation and accessibility. - Changed from a div-based layout to a paragraph-based layout with adjusted image sizes for better responsiveness. * docs: Add alpha release process documentation - Introduced a new document detailing the alpha release process for the Rocket.Chat Desktop app, including channel definitions, versioning guidelines, and steps for creating and publishing alpha releases. - Included instructions for users to opt into the alpha channel and troubleshooting tips for common issues. * chore: Update architecture support and Node.js version requirements - Added 'arm64' architecture support to the build targets in electron-builder.json for NSIS, MSI, and ZIP formats. - Lowered the minimum Node.js version requirement in package.json from >=24.11.1 to >=20.0.0 for better compatibility. * chore: Change develop branch to dev for release workflow Update build-release workflow and desktop-release-action to use 'dev' branch instead of 'develop' for development releases. * chore: Update versioning and add release tag script - Bumped version in package.json to 4.12.0.alpha.1. - Added scripts/release-tag.ts for automated release tagging. - Updated .eslintignore to exclude the new scripts directory. * chore: Correct version format in package.json - Updated version format in package.json from "4.12.0.alpha.1" to "4.12.0-alpha.1" for consistency. * chore: Update all workflows to use dev branch instead of develop - validate-pr.yml: Add dev to PR target branches - powershell-lint.yml: Change develop to dev - pull-request-build.yml: Change develop to dev * fix: Normalize tags for consistent comparison in release-tag script Strip leading 'v' prefix when comparing tags to handle both v-prefixed and non-prefixed tag formats consistently. * chore: Increment bundle version in electron-builder.json to 26012 * chore: Address nitpick comments in release-tag script - Add comment explaining why /scripts is excluded from eslint - Return null on exec error to distinguish from empty output - Add warning when git tag list fails - Use -- separator in git commands for safety * fix: Add jsign to GITHUB_PATH in Windows CI setup The jsign tool was being installed but not added to PATH for subsequent steps. This caused the "Verify tools" step to fail with "jsign not found". * chore: Bump version to 4.12.0-alpha.2 - Updated version in package.json to 4.12.0-alpha.2 - Incremented bundleVersion in electron-builder.json to 26013 * docs: Add QA testing guide for alpha channel updates * docs: Rename alpha docs to pre-release and fix workflow concurrency - Rename alpha-release-process.md to pre-release-process.md - Add beta release documentation - Add detailed channel switching instructions - Fix concurrency group using github.ref instead of github.head_ref (github.head_ref is empty for push events, causing tag builds to cancel) * feat(logging): add scoped logging infrastructure * feat(log-viewer): add log viewer window and components * build: add log viewer window build configuration * feat: integrate logging and log viewer into app lifecycle * feat: add log viewer IPC channels and menu item * feat: add i18n translations and fix UI color tokens * chore: add logging dependencies and fix type error * fix: address code review feedback - Add 'silly' log level to LogLevel type for electron-log compatibility - Fix duplicate server IDs by using overflow counter instead of MAX_SERVER_ID - Reset startInProgress flag when retry count exceeded in preload - Add statLog to log viewer preload API - Use contextIsolation and preload script for log viewer window security - Replace direct ipcRenderer usage with window.logViewerAPI in renderer * revert: restore log viewer window settings and add architecture guidelines - Revert nodeIntegration/contextIsolation changes that broke log viewer - Add CLAUDE.md guidelines to prevent destructive architecture changes - Document that existing code patterns exist for specific reasons * fix: address code review feedback from CodeRabbit This commit addresses three major review comments: 1. Remove unused preload script for log viewer window - The preload.ts was built but never wired to the BrowserWindow - Current implementation uses nodeIntegration: true and contextIsolation: false - Removed unused build entry from rollup.config.mjs - Deleted unused src/logViewerWindow/preload.ts file 2. Guard programmatic scrolls to prevent disabling auto-scroll - Added isAutoScrollingRef to track programmatic vs user-initiated scrolls - Set flag before calling scrollToIndex and reset after - handleScroll now returns early if scroll is programmatic - Prevents auto-scroll from being disabled when virtuosoRef.scrollToIndex triggers onScroll 3. Don't swallow startup failures - exit after logging - Changed start().catch(console.error) to properly log error and exit - Uses logger.error for structured logging - Calls app.exit(1) to prevent partial initialization - Prevents app running in broken state after critical failures 4. Add error handling to log viewer menu item - Wrapped openLogViewer click handler in try-catch - Matches pattern used by videoCallDevTools menu item - Logs errors to console for debugging * fix(log-viewer): guard against non-positive limits in getLastNEntries Return empty content when limit <= 0 to prevent undefined behavior from negative slice indices. --------- Co-authored-by: Rodrigo Nascimento <[email protected]> Co-authored-by: lingohub[bot] <69908207+lingohub[bot]@users.noreply.github.com> Co-authored-by: Max Lee <[email protected]> * fix: Add safe guards to prevent The application GUI just crashed (#3206) * fix: guard store functions against pre-initialization calls on macOS Tahoe On macOS 26.x (Tahoe), the IPC call to retrieve the server URL is slower than on earlier macOS versions, causing the preload to retry with a 1-second delay. During this window the RC webapp loads and calls `window.RocketChatDesktop.setTitle()` and `setUserPresenceDetection()`, which internally invoke `dispatch()` and `listen()` from the Redux store before `createRendererReduxStore()` has completed. Since `reduxStore` is still `undefined`, accessing `.dispatch` or `.subscribe` throws a TypeError that propagates back through contextBridge into the React tree, crashing the app with "The application GUI just crashed". Fix: add null guards to `dispatch`, `dispatchSingle`, `dispatchLocal`, `watch`, and `listen` so they silently no-op instead of throwing when the store is not yet initialized. The webapp reactively re-fires these calls once the app is fully ready, so no state is permanently lost. Also guard `request()` to reject immediately with a clear error rather than returning a hung Promise that never resolves, preventing potential memory leaks if `createNotification()` is called before store init. Simplify the `getInternalVideoChatWindowEnabled` selector as a drive-by. * fix: add safeSelect for preload context and guard getInternalVideoChatWindowEnabled select() has no null guard by design — it crashes loudly if called before store initialization, which is correct for the main process where the store is always ready before any select() call. Add safeSelect() for preload contexts where the store may not yet be initialized. Unlike select(), it returns T | undefined and TypeScript enforces that callers handle the undefined case. Use safeSelect in getInternalVideoChatWindowEnabled() with an explicit ?? false fallback, so early calls before store init return false (safe default) instead of crashing or silently returning undefined-as-boolean. * fix: Screen picker not loading again after closing by clicking outside it (#3205) * fix: improve screen share picker cancellation reliability - Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain * fix: resolve screen share picker stuck after dismissal Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX. * chore: remove outdated Electron 10 comment (#3202) * Language update from Lingohub 🤖 (#3196) Project Name: Rocket.Chat.Electron Project Link: https://app.lingohub.com/project/pr_1Ag2Vlx6MWNt-16038/branches/prb_16rm9BiWK53b-4144 User: Lingohub Robot Easy language translations with Lingohub 🚀 Co-authored-by: lingohub[bot] <69908207+lingohub[bot]@users.noreply.github.com> * chore: Update hu.i18n.json (#3193) * chore: Remove package-lock.json in favor of yarn.lock (#3214) * chore: remove package-lock.json in favor of yarn.lock This project uses Yarn as its package manager. Having both package-lock.json and yarn.lock tracked causes conflicts and breaks npx/npm tooling due to devEngines format differences. * chore: anchor package-lock.json ignore to repository root * fix: address code review feedback - privacy.ts: don't spread message in catch block to avoid leaking sensitive fields; return only safe metadata (level, date). Add one-time stderr warning when redaction fails for observability. - store/index.ts: warn when watch() is called before store is initialized so callers aren't silently left in uninitialized state. - hu.i18n.json: fix 'bolt' (shop) -> 'tároló' (storage) for software store references. Add missing trailing period for consistency. --------- Co-authored-by: Rodrigo Nascimento <[email protected]> Co-authored-by: lingohub[bot] <69908207+lingohub[bot]@users.noreply.github.com> Co-authored-by: Max Lee <[email protected]> Co-authored-by: Sisyphus <[email protected]> Co-authored-by: Santam Roy Choudhury <[email protected]> Co-authored-by: Balázs Úr <[email protected]>
Ram-sah19
pushed a commit
to Ram-sah19/Rocket.Chat.Electron
that referenced
this pull request
Mar 10, 2026
…e it (RocketChat#3205) * fix: improve screen share picker cancellation reliability - Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain * fix: resolve screen share picker stuck after dismissal Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX.
Ram-sah19
pushed a commit
to Ram-sah19/Rocket.Chat.Electron
that referenced
this pull request
Mar 10, 2026
…e it (RocketChat#3205) * fix: improve screen share picker cancellation reliability - Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain * fix: resolve screen share picker stuck after dismissal Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX.
Ram-sah19
pushed a commit
to Ram-sah19/Rocket.Chat.Electron
that referenced
this pull request
Mar 10, 2026
…e it (RocketChat#3205) * fix: improve screen share picker cancellation reliability - Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain * fix: resolve screen share picker stuck after dismissal Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX.
Ram-sah19
pushed a commit
to Ram-sah19/Rocket.Chat.Electron
that referenced
this pull request
Mar 10, 2026
…e it (RocketChat#3205) * fix: improve screen share picker cancellation reliability - Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain * fix: resolve screen share picker stuck after dismissal Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX.
Ram-sah19
pushed a commit
to Ram-sah19/Rocket.Chat.Electron
that referenced
this pull request
Mar 10, 2026
…e it (RocketChat#3205) * fix: improve screen share picker cancellation reliability - Remove redundant dialog.close() call inside onclose handler in Dialog hooks (close event fires after dialog is already closed, making the call a no-op per WHATWG spec) - Add safety-net IPC cancellation in ScreenSharePicker: track whether a response was sent per picker session; if visible transitions false without a response having been sent, send null cancellation as fallback. This covers all dismissal paths (click-outside, ESC, programmatic close) regardless of the Dialog close event chain * fix: resolve screen share picker stuck after dismissal Three compounding bugs caused the screen sharing button to become permanently unresponsive after the user dismissed the picker by clicking outside the dialog: 1. handleClose firing after handleShare — when handleShare called setVisible(false), the useDialog useEffect triggered dialog.close() which synchronously fired onclose → handleClose. Since handleClose had no guard, it sent a null cancellation immediately after the real sourceId, consuming the ipcMain.once listener with null and leaving Jitsi's getDisplayMedia callback unresolved on the next attempt. Fix: added responseSentRef.current guard at the top of handleClose. 2. isScreenSharingRequestPending cleared after cb() — Jitsi calls getDisplayMedia again synchronously inside the setDisplayMediaRequest- Handler callback, re-entering createInternalPickerHandler while isScreenSharingRequestPending was still true, permanently blocking subsequent requests. Fix: moved markScreenSharingComplete() before cb() in both the response listener and the timeout handler. 3. Dual ipcMain.once race in open-screen-picker handler — the jitsiBridge IPC path registered its own relay listener without clearing any active listener from createInternalPickerHandler first. Fix: call cleanupScreenSharingListener() before registering the relay. Also adds "Open System Preferences" link to the screen recording permission denied callout, consistent with the microphone permission UX.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves a frustrating bug where the "Start Screen Sharing" button would become permanently unresponsive after dismissing the screen picker by clicking outside it during a Jitsi video call. Users had to disconnect and rejoin to recover. This PR also adds a quick link to system privacy settings when screen recording permission is denied.
What's New
Added
Fixed
Improved
Platform Notes
How to Test
Screen Sharing Button Recovery
Screen Sharing Cancel → Share Flow
Screen Recording Permission Link (macOS)
Screen Recording Permission Link (Windows)
Related
https://rocketchat.atlassian.net/browse/SUP-991
Summary by CodeRabbit
New Features
Improvements
Localization