Skip to content

fix: only open screen-share picker on a user-initiated request#3360

Open
Shevilll wants to merge 1 commit into
RocketChat:masterfrom
Shevilll:fix/screen-share-picker-on-launch
Open

fix: only open screen-share picker on a user-initiated request#3360
Shevilll wants to merge 1 commit into
RocketChat:masterfrom
Shevilll:fix/screen-share-picker-on-launch

Conversation

@Shevilll

@Shevilll Shevilll commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Closes #3308

What changed

The desktop-capturer source picker opened on every app launch (regression since 4.14.0, introduced by #3266 which added screen-sharing support for the server view).

Root cause: the web app calls navigator.mediaDevices.getDisplayMedia() during page load to probe capabilities. Because the app runs with --autoplay-policy=no-user-gesture-required, that automatic, gesture-less probe is not rejected as it would be in a browser — it reaches the setDisplayMediaRequestHandler that #3266 now registers on every server-view webview, which opens the picker.

This adds a guard so the handler ignores requests that carry no user gesture (request.userGesture === false), and only delegates to the provider for a genuine, user-initiated screen share. Real shares always originate from a click, so they are unaffected.

This complements the earlier partial fix #3320, which only gated the Linux/portal cache-prewarm path — the picker still opened on every launch on all platforms via the handler itself, which is why this issue remained open.

Why this approach

request.userGesture is the documented Electron signal for whether a display-media request came from a user interaction; gating on it is the minimal change that restores the pre-4.14.0 behaviour (no picker on launch) without reverting #3266's feature. The denial uses the same cb({ video: false }) shape as the other deny paths in this module for consistency.

Tests

Added two cases to src/screenSharing/main/serverViewScreenSharing.spec.ts:

  • a gesture-less request is denied with { video: false } and the provider's handleDisplayMediaRequest is not invoked;
  • a request with a user gesture is delegated to the provider.

Run with yarn test.

Manual verification

  1. Launch the app, connect to a workspace, then close and relaunch → no picker appears on startup (the reported regression).
  2. Start a screen share from a server-view share button → the picker still opens and a source can be selected.
  3. Start a screen share inside a video call → the picker still works (the call-window handler is separate and untouched).

Summary by CodeRabbit

  • Bug Fixes

    • Fixed screen-picker incorrectly opening from automatic probe calls without user gestures. The handler now detects and rejects such requests immediately.
  • Tests

    • Added test coverage for user gesture validation, verifying correct handling of both user-initiated and automatic request scenarios.

Since server-view screen sharing support was added (4.14.0), the desktop-capturer source picker opened on every app launch. The web app calls navigator.mediaDevices.getDisplayMedia() during page load to probe capabilities; because the app runs with autoplay-policy no-user-gesture-required, that gesture-less probe reached the newly registered setDisplayMediaRequestHandler and opened the picker.

Ignore display-media requests that carry no user gesture, so the picker only opens for a genuine, user-initiated screen share. Real shares (which always originate from a click) are unaffected. This complements the earlier partial fix (RocketChat#3320), which only gated the Linux/portal cache-prewarm path and left the issue open.

Closes RocketChat#3308
@CLAassistant

CLAassistant commented Jun 20, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8d084b78-857f-495d-b72a-36e02e4a1fa0

📥 Commits

Reviewing files that changed from the base of the PR and between 4856b6b and e2ed930.

📒 Files selected for processing (2)
  • src/screenSharing/main/serverViewScreenSharing.spec.ts
  • src/screenSharing/serverViewScreenSharing.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.ts: Use TypeScript for all new code unless explicitly told otherwise
Use optional chaining with fallbacks for platform-specific APIs instead of mocking when possible. Example: const uid = process.getuid?.() ?? 1000;

Files:

  • src/screenSharing/serverViewScreenSharing.ts
  • src/screenSharing/main/serverViewScreenSharing.spec.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{tsx,ts}: MANDATORY: Use Fuselage components for all UI work. Only create custom components when Fuselage doesn't provide what's needed
Import UI components from @rocket.chat/fuselage and check Theme.d.ts for valid color tokens
Use React functional components with hooks
Use PascalCase for component file names

Files:

  • src/screenSharing/serverViewScreenSharing.ts
  • src/screenSharing/main/serverViewScreenSharing.spec.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Redux actions must follow FSA (Flux Standard Action) pattern
Avoid unnecessary comments — write self-documenting code through clear naming
Always verify libraries by checking official docs and .d.ts files in node_modules/. Never assume props, tokens, or APIs work without verification
Avoid subjective descriptors ('smart', 'excellent', 'dumb') in documentation and comments
Use measurable descriptions in code documentation: 'reduced memory usage', 'improved by X%' instead of subjective claims
NEVER invent metrics — don't include estimated time spent or speculated user counts. Only include numbers from actual logs, error messages, or documented sources

Files:

  • src/screenSharing/serverViewScreenSharing.ts
  • src/screenSharing/main/serverViewScreenSharing.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use *.spec.ts file naming for Renderer process tests

Files:

  • src/screenSharing/main/serverViewScreenSharing.spec.ts
**/*.{spec.ts,main.spec.ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Only mock platform-specific APIs when defensive coding isn't possible. Linux-only APIs requiring mocks: process.getuid(), process.getgid(), process.geteuid(), process.getegid()

Files:

  • src/screenSharing/main/serverViewScreenSharing.spec.ts
🔇 Additional comments (4)
src/screenSharing/serverViewScreenSharing.ts (2)

72-78: LGTM!


72-78: The request.userGesture property is documented and available in Electron 40.8.5.

The setDisplayMediaRequestHandler callback's request parameter includes the userGesture property as documented in the official Electron API. This property has been part of the API since its introduction, and Electron 40 remains a supported version as of June 2026. The code correctly relies on this property.

src/screenSharing/main/serverViewScreenSharing.spec.ts (2)

167-183: LGTM!


185-201: LGTM!


Walkthrough

setupServerViewDisplayMedia in serverViewScreenSharing.ts now reads the request object in the setDisplayMediaRequestHandler callback and short-circuits with { video: false } when request.userGesture is false. Two new test cases in the spec file cover both the blocked and delegated paths.

Changes

User gesture gate for screen-share handler

Layer / File(s) Summary
userGesture guard and tests
src/screenSharing/serverViewScreenSharing.ts, src/screenSharing/main/serverViewScreenSharing.spec.ts
The setDisplayMediaRequestHandler callback accepts the request argument and returns { video: false } immediately when request.userGesture is false, leaving the existing provider-delegation path intact for genuine user-initiated requests. Two new test cases assert the blocked path calls the callback directly without invoking internalProvider.handleDisplayMediaRequest, and the user-gesture path delegates to the provider.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a user-gesture gate to prevent opening the screen-share picker on non-user-initiated requests.
Linked Issues check ✅ Passed The PR implements the fix for #3308 by adding a user-gesture check to ignore gesture-less display-media requests, directly addressing the regression where the picker opened on every app launch.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the user-gesture gate in setupServerViewDisplayMedia and corresponding test coverage, with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Shevilll

Copy link
Copy Markdown
Contributor Author

Hi team! Gated the screen-share capturer picker with a user gesture check (request.userGesture === true) to prevent launch-time popups, and verified all Jest test specs are green. Please let me know if there are any adjustments needed! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Screen share picker opens on every app launch (regression since 4.14.0, likely PR #3266)

2 participants