Skip to content

test(telephony): stabilize shortcut notification click - #3333

Merged
jeanfbrito merged 1 commit into
feat/telephony-shortcut-main-processfrom
fix/telephony-shortcut-electron-test-crash
May 14, 2026
Merged

test(telephony): stabilize shortcut notification click#3333
jeanfbrito merged 1 commit into
feat/telephony-shortcut-main-processfrom
fix/telephony-shortcut-electron-test-crash

Conversation

@jeanfbrito

@jeanfbrito jeanfbrito commented May 14, 2026

Copy link
Copy Markdown
Member

Closes #ISSUE_NUMBER

Summary by CodeRabbit

  • Bug Fixes
    • Improved the reliability of the settings window activation when registration failure notifications are clicked. The app now properly logs window focus errors and ensures the settings action is always triggered regardless of focus status.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d413813c-acd4-4ad2-955a-11e42fa9c811

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb8388 and 77edf36.

📒 Files selected for processing (2)
  • src/telephony/main.spec.ts
  • src/telephony/main.ts

Walkthrough

The PR refactors the registration failure notification's click handler to improve async reliability: the handler now explicitly logs errors from window focus attempts and guarantees dispatch of the sidebar settings action via a .finally() block. The test is updated to treat the extracted click listener as a possibly-async function and await it directly.

Changes

Notification click handler refactor

Layer / File(s) Summary
Notification click handler with explicit error logging and finalized dispatch
src/telephony/main.ts, src/telephony/main.spec.ts
The notifyRegistrationFailure notification click handler now awaits focusRootWindow() with explicit error logging and dispatches SIDE_BAR_SETTINGS_BUTTON_CLICKED in a .finally() block to ensure the action is always dispatched. The corresponding test extracts and awaits the handler directly as a possibly-async function.

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • RocketChat/Rocket.Chat.Electron#3331: The main PR's updates to notifyRegistrationFailure's Electron Notification click handler refactor and test stabilization directly mirror the changes in PR #3331.

Suggested labels

type: chore


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.

@jeanfbrito
jeanfbrito merged commit 1ec2d3b into feat/telephony-shortcut-main-process May 14, 2026
2 of 3 checks passed
jeanfbrito added a commit that referenced this pull request May 14, 2026
* feat(telephony): add global shortcut to dial clipboard number

* fix(telephony): harden global shortcut handling

* refactor(telephony): share dialpad opener

* test(telephony): stabilize shortcut notification click (#3331)

* test(telephony): stabilize shortcut notification click (#3333)
@jeanfbrito
jeanfbrito deleted the fix/telephony-shortcut-electron-test-crash branch May 14, 2026 11:07
jeanfbrito added a commit that referenced this pull request Jul 2, 2026
…3370)

* feat: register callto:/tel: deep link handlers (DAMOVO-1)

Register Rocket.Chat as OS handler for callto: and tel: URL schemes
on Windows, macOS, and Linux. When a telephony link is clicked in any
app, RC launches or focuses and dispatches a typed IPC event to the
server webview with the parsed phone number.

- Register callto/tel schemes in electron-builder.json (all platforms)
- Add parseTelephonyLink() with number normalization and callto:// support
- Add performTelephonyCall() with multi-server dialog + remember choice
- Expose onTelephonyCallRequested callback on RocketChatDesktop API
- Persist telephonyPreferredServer via selectPersistableValues
- IPC listener registered before onReady to avoid cold-start race

* test: add unit tests for telephony deep link parsing and routing

Tests cover parseTelephonyLink (tel:/callto: protocols, number
normalization, callto:// double-slash format, extension syntax,
edge cases) and performTelephonyCall (0/1/2+ servers, preferred
server persistence, dialog remember checkbox).

* fix: attach server selection dialog to root window and guard against duplicate IPC listener

- Pass getRootWindow() as first argument to dialog.showMessageBox so
  the server selection prompt appears as a modal sheet attached to the
  main window (consistent with all other dialogs in the codebase)
- Add idempotency guard to listenToTelephonyRequests to prevent
  duplicate IPC handler registration during hot-reload dev cycles

* chore: add GitNexus code intelligence config to CLAUDE.md

Add GitNexus section with impact analysis, query, and context tools.
Gitignore .gitnexus index directory.

* feat: support sha-prefixed exception versions by git commit hash

Dispatch WEBVIEW_GIT_COMMIT_HASH_CHANGED from server info response.
Match supportedVersions exceptions using sha:<hash> prefix against
the server's git commit hash for per-build version overrides.

* feat: add telephony preferred server settings UI

Add TelephonyServer component to Settings > General tab with a
Select dropdown to choose which server handles tel:/callto: links.
Hidden when only one server exists. "Auto (ask each time)" option
clears the preference and reverts to dialog behavior.

* fix: move MimeType into desktop.entry for electron-builder v26 compat

electron-builder v26 rejects MimeType as a direct child of
linux.desktop — only desktopActions and entry are valid properties.
Move it inside desktop.entry where it belongs.

* i18n: add translations for telephony server selection dialog

Replace hardcoded English strings in the telephony dialog with i18n
t() calls and add telephonySelectServer translation keys to all 22
locale files.

* feat: replace native dialog with Fuselage modal for telephony server selection

Replace dialog.showMessageBox with an in-app modal that shows server
favicons, names and hostnames — matching the sidebar appearance. Scales
to many servers via a scrollable list and includes a "remember this
choice" checkbox.

Also hardens the telephony flow:
- Mutex prevents concurrent tel: links from opening duplicate modals
- 120s timeout on modal promise prevents hanging if renderer crashes
- 10s timeout on webContents polling prevents infinite loop if server
  is removed between selection and view creation

* refactor(telephony-modal): tighten vertical spacing

Drop Margins wrapper from the modal and the Tile container from rows.
Title→message margin x8→x4, message→list x16→x12, rows now use
paddingBlock x6 / paddingInline x8 instead of Tile padding x12.

* test(telephony): add coverage for preload, settings dropdown, and server modal

Adds 20 tests across three new spec files covering the telephony deep-link
runtime path that was previously only validated by deepLinks/main.spec.ts.

- src/telephony/renderer/preload.spec.ts (6 tests):
  IPC bridge state machine — listenToTelephonyRequests guard, pendingPayload
  buffer/replay, callback replacement, ipcRenderer.on registration.

- src/ui/components/SettingsView/features/TelephonyServer.spec.tsx (8 tests):
  Settings dropdown — hide when servers.length <= 1, option generation
  (auto + per-server), value binding to telephonyPreferredServer, dispatch
  of TELEPHONY_PREFERRED_SERVER_SET (null for auto, URL string otherwise),
  hostname fallback when server title missing.

- src/ui/components/TelephonyServerSelectModal/index.spec.tsx (6 tests):
  Modal flow — visibility gating on dialogs.telephonyServerSelect.isOpen,
  ServerItem rendering per server, dispatch payload shape on click with
  rememberChoice on/off, close dispatch with null payload, rememberChoice
  reset after close.

Adds @testing-library/react, @testing-library/jest-dom, and
@testing-library/dom (peer) as devDependencies. Fuselage Select and Dialog
are mocked at module level since they rely on React-Aria and native
<dialog>.showModal() respectively, which don't drive cleanly in
@kayahr/jest-electron-runner's renderer environment.

Spec paths follow the existing renderer testMatch convention:
src/<module>/<subdir>/<file>.spec.tsx — a flat src/telephony/preload.spec.ts
would be silently dropped by jest's testMatch globs.

* fix(telephony): decode percent-encoded URI before sanitization

tel:%2B15551234 left %2B encoded, producing phoneNumber '%2B15551234'
instead of '+15551234'. decodeURIComponent runs before strip pass;
malformed escapes return null (treated same as other invalid input).

* fix(supportedVersions): make sha- exception prefix check case-insensitive

Git commit hashes are conventionally case-insensitive. SHA-bb83777
should match same as sha-bb83777.

* fix(telephony-ui): harden URL parsing and improve modal accessibility

- TelephonyServer: extract hostname via safeHostname helper to prevent
  settings page crash on malformed server URLs (new URL() throws).
- TelephonyServerSelectModal: associate 'Remember this choice' label
  with checkbox via htmlFor/id for assistive tech.
- ServerItem: render Tile as native button (is='button' type='button')
  so keyboard users get Tab focus and Enter/Space activation.

* Feat/telephony shortcut main process (#3334)

* feat(telephony): add global shortcut to dial clipboard number

* fix(telephony): harden global shortcut handling

* refactor(telephony): share dialpad opener

* test(telephony): stabilize shortcut notification click (#3331)

* test(telephony): stabilize shortcut notification click (#3333)

* Add telephony clipboard dial shortcut (#3330)

* feat(telephony): add global shortcut to dial clipboard number

* fix(telephony): harden global shortcut handling

* refactor(telephony): share dialpad opener

* test(telephony): stabilize shortcut notification click

* fix telephony deeplink edge cases

* chore: format telephony PR lint fixes

* refactor: add marginBlock to Field components in SettingsView features

Updated the AvailableBrowsers, TelephonyGlobalShortcut, TelephonyServer, and ThemeAppearance components to include a marginBlock of 'x16' on the Field components for improved spacing and layout consistency.

* chore: polish telephony settings copy

* chore: add telephony settings translations

* feat(telephony): add master toggle and gate runtime registration

Add `isTelephonyEnabled` setting (default off) to gate the telephony
feature end-to-end:

- New persisted `isTelephonyEnabled` boolean with action, reducer, and
  selector entry; surfaces as a master toggle in Settings > General.
- `TelephonyServer` and `TelephonyGlobalShortcut` controls remain
  visible but disabled while the master toggle is off.
- Global shortcut config selector returns the disabled config when the
  master toggle is off, so the existing watcher auto-unregisters any
  active accelerator on toggle-off.
- `tel:`/`callto:` deep links short-circuit when the master toggle is
  off.
- OS-level protocol registration for `tel`/`callto` moves out of the
  unconditional startup loop into a new reactive
  `setupTelephonyProtocolHandlers`, which calls
  `setAsDefaultProtocolClient` / `removeAsDefaultProtocolClient` in
  response to toggle changes. `rocketchat:` continues to register at
  startup unchanged.

The macOS `Info.plist` and Linux `.desktop` files declared by
electron-builder will still list the app as a candidate handler for
`tel`/`callto`, but it will never be set as default unless the user
opts in at runtime.

* feat(telephony): prompt user about default handler conflicts on opt-in

DMV-1 calls for a first-run prompt warning the user that Teams, Zoom,
or Skype may already own the tel:/callto: handler and that they must
confirm Rocket.Chat as default in OS settings themselves. Windows 10/11
hash-protects UserChoice so `setAsDefaultProtocolClient` only registers
the app as a candidate; without the prompt, users have no way to know
the OS silently kept the prior default.

Trigger every off->on transition of the master `isTelephonyEnabled`
toggle (not literal first run — the toggle is opt-in and is the natural
moment of user intent). Seed the transition tracker via a synchronous
`select` before subscribing, so returning users who reopen the app
with telephony already enabled are not re-prompted.

- New `TelephonyDefaultHandlerPromptModal` Fuselage modal (title, two
  body paragraphs, "Open System Settings" + "Got it" buttons), mounted
  in Shell next to the existing telephony modal.
- Three new void actions (`_OPEN`, `_CLOSE`, `_OPEN_SETTINGS_CLICKED`)
  and a `telephonyDefaultHandlerPrompt` sub-reducer in `dialogs.ts`.
- Main-process `setupTelephonyDefaultHandlerPrompt` watches the master
  toggle and dispatches OPEN on each off->on flip. Listens for the
  settings-button click and routes per platform:
  - Windows: `shell.openExternal('ms-settings:defaultapps')`
  - macOS: opens FaceTime preferences (where tel: default lives)
  - Linux: spawns `gnome-control-center default-apps` or
    `kcmshell5/6 componentchooser` based on `XDG_CURRENT_DESKTOP`;
    unknown DEs log a tip and rely on the modal's verbal instructions.
- i18n keys under `telephony.defaultHandlerPrompt`.
- 13 new tests covering transition detection, idempotency, teardown,
  and each platform branch.

* test(app): nest PersistableValues spec into __tests__

The renderer Jest project's `testMatch` requires at least one
subdirectory between `src/<module>/` and the spec, so
`src/app/PersistableValues.spec.ts` was silently skipped. Moved the
file under `src/app/__tests__/`, fixed the relative import, and
expanded the migration assertion to cover `isTelephonyEnabled`.
Documented the constraint in `CLAUDE.md` so future renderer specs are
placed correctly.

* feat(telephony): add Voice & Video settings tab and polish diagnostics UI

Split telephony, video-call and screen-capture controls out of the General
settings tab into a dedicated Voice & Video tab so the telephony stack has
room to grow without crowding the rest of the settings.

Diagnostics UI now collapses by default behind an Accordion with a status
Tag summary in the title (pass/issues/warnings/checking). Per-check rows
use Tag variants for status (primary/danger/warning) with flexShrink
guards so the badge does not collapse to an ellipsis on narrow widths.
Check labels were rewritten in user-facing terms (Click-to-call,
Click-to-conference) and platform names are humanized (darwin -> macOS).
Long handler paths are stripped from the inline details (full path still
ships in the copy-diagnostics JSON) so the row layout stays clean.

* feat(telephony): expose diagnostics IPC and Windows capabilities registration

Adds telephony/get-diagnostics IPC channel and runtime module, wires
TelephonyDiagnostics into the settings panel, and registers Rocket.Chat
in the Windows RegisteredApplications/Capabilities surface so Default
Apps exposes it for tel and callto.

* fix(telephony): target app-scoped Windows default-apps deep link and skip darwin

Detects per-user vs per-machine installs from process.execPath and opens
ms-settings:defaultapps?registeredApp{User,Machine}=Rocket.Chat so the
Default Apps page lands on the app-specific surface. macOS has no
equivalent settings pane, so the open-settings handler is now a no-op
and the modal hides body2 plus the Open Settings button on darwin.

* fix(store): snapshot prev before invoking watcher to avoid stale re-entry

If a watcher synchronously dispatches an action that re-triggers the
same subscription, the recursive call previously saw a stale prev
value. Capture prev into a local before assigning curr to it, so the
recursive watcher invocation observes the freshly applied state.

* refactor(telephony): localize shortcut display and tighten telephony copy

Introduces formatAcceleratorForDisplay so the shortcut input and
validation error render Cmd/Ctrl labels (with macOS-aware overrides)
instead of leaking the raw Electron accelerator syntax. The input
becomes capture-only (readOnly) so manual typing cannot desync from
the stored value, and the reserved-accelerator key is renamed to
reservedByApp with a new reservedByOS sibling. Several telephony
strings (modal, settings descriptions, diagnostics labels, select
server dialog) are rewritten for clarity and consistency.

* chore: ignore local OpenWolf tooling state

* chore: apply prettier formatting to VoiceVideoTab accordion items

* fix(telephony): use Fuselage default color token for server title in select modal

* fix(telephony): switch active workspace to resolved server before placing call

openTelephonyDialpad sent telephony/call-requested to the resolved
server's webContents but never updated currentView, so the call landed
in a workspace the user was not looking at. Dispatch
DEEP_LINKS_SERVER_FOCUSED (same action the rocketchat:// deep-link path
uses) with the resolved URL before contacting the webview so the
visible view follows the call across the single-server, preferred-server,
and modal-selection paths.

* fix(telephony): check Windows UserChoice ProgId for isDefault diagnostic

app.isDefaultProtocolClient on Windows reports true when the
RocketChat.tel / RocketChat.callto ProgIDs are registered, regardless
of which handler the user actually picked via Default Apps. This made
isDefault.tel pass even when Windows Settings still showed "Choose a
default" for tel. Read the authoritative
HKCU\\Software\\Microsoft\\Windows\\Shell\\Associations\\URLAssociations\\<scheme>\\UserChoice
ProgId and compare it to RocketChat.<scheme> instead. Non-Windows
platforms keep using isDefaultProtocolClient.

* docs(telephony): clarify Windows default-handler prompt and diagnostic messages

Windows blocks apps from writing the UserChoice ProgId, so the user
has to pick Rocket.Chat per scheme on the Default Apps page. Reword
the default-handler modal to spell out that each link type (tel and
callto) must be picked individually, mention that Windows itself
prevents apps from setting it, and rename the action button to point
at the Rocket.Chat default-apps page.

Diagnostic details for the isDefault check now read as user-facing
guidance instead of registry jargon: "Windows has not been told which
app to use..." when UserChoice is missing, and "Currently handled by
<app>. Open default apps to switch to Rocket.Chat." when another
handler is set.

* docs(telephony): split default-handler modal copy by platform

body2 and the action button render on both Windows and Linux, so the
prior Windows-specific text leaked onto Linux installs. Split into
bodyWindows / bodyLinux and openSettingsWindows / openSettingsLinux
keys and pick the right pair in the modal based on process.platform.

* fix(telephony): widen body margin in default-handler modal so buttons do not crowd the text

* feat(telephony): ship Windows default-app associations XML + MSI opt-in policy flag

Windows blocks user-mode writes to UserChoice (UCPD since March 2024) so
the installer cannot make Rocket.Chat the default tel:/callto: handler
on its own. Ship the canonical DefaultAssociations XML alongside the
app and expose a new MSI public property SET_DEFAULT_ASSOCIATIONS=1
that, when explicitly passed, writes the GPO-equivalent registry value
(HKLM\\SOFTWARE\\Policies\\Microsoft\\Windows\\System!DefaultAssociationsConfiguration)
pointing at the bundled XML. A sentinel under HKLM\\SOFTWARE\\Rocket.Chat\\InstallState
lets uninstall remove just the value we wrote without touching other
policies in that key.

Documents GPO / Intune / DISM paths so admins who already manage default
associations centrally use those channels instead of the installer flag
(real AD GPOs win at the next gpupdate cycle anyway).

A small spec guards against the XML and installer ProgIds drifting apart.

* fix(installer): preserve default-associations policy across MSI major upgrades

The cleanup CA also fires during RemoveExistingProducts on a major upgrade,
which would wipe HKLM\\SOFTWARE\\Policies\\Microsoft\\Windows\\System!DefaultAssociationsConfiguration
and the sentinel before the new MSI installs. The new MSI only rewrites
when SET_DEFAULT_ASSOCIATIONS=1 is re-passed, and admins typically forget
that on routine upgrades — so policy would silently disappear after a
version bump. Gate the uninstall condition on UPGRADINGPRODUCTCODE="" so
cleanup runs only on real uninstalls.

Add an automated WiX-injection spec covering the property declaration,
deferred + Impersonate="no" CA attributes, type-51 immediate setters,
install/uninstall scheduling conditions (including the new upgrade
guard), placement of CustomAction/Property elements as children of
<Product>, and a regression check that backslashes in VBScript registry
paths render as single backslashes after JS template-literal expansion.

* docs(windows): split default-app-associations into its own shareable file

* fix(telephony): tighten default handler diagnostics

Read Windows UserChoiceLatest when UserChoice is absent and treat explicit Windows handler choices as authoritative so another app cannot be reported as pass through protocol registration fallback.

Hide the default-app CTA when diagnostics are healthy, add per-check settings actions for actionable failures, and cover the Windows/Linux diagnostic flows with focused tests.

* fix(telephony): keep server selector text readable

Keep the telephony server selector aligned with the settings select width while preventing mid-word wrapping, and rename the prompt option to describe the ask-each-time behavior.

* fix(i18n): complete Brazilian Portuguese translations

* fix(i18n): complete German translations

* docs(telephony): clarify default handler diagnostics

* fix(installer): register telephony associations in MSI

* Fix telephony release blockers

* Add telephony QA flows

* Make telephony QA flows Qase-ready

* Harden QA flow authoring guidance

* Add reusable Desktop QA flow skill

* Tighten telephony QA coverage

* chore: add telephony picker diagnostics

* fix: filter deep link process arguments

* fix(telephony): reset rememberChoice on state-driven modal close

The TelephonyServerSelectModal kept `rememberChoice` local state alive
across close/reopen cycles when the modal closed via a Redux state
update (e.g., external dispatch) rather than the local close handlers,
leaking the prior `true` value into the next dispatched payload.

Add a useEffect keyed on `isVisible` that resets `rememberChoice` when
the modal becomes hidden. The existing in-handler resets stay in place
for stores that do not propagate state changes (notably the stub
reducers in unit tests).

Fixes the failing `rememberChoice resets when the dialog is closed by
state update` spec that blocked all 6 PR #3325 CI jobs.

* fix(telephony): expire buffered deeplink after 120s TTL

A deeplink targeting a workspace without VoIP never registers an
onTelephonyCallRequested callback, so the buffered pendingPayload would
sit in the frame indefinitely and could surface a stale number on a
later unrelated remount. Drop the payload silently after a 120s TTL;
the timer is cleared on flush so a consumed payload never re-fires.

* fix(telephony): strip non-phone debris from clipboard dial shortcut

extractClipboardPhoneNumber returned the raw trimmed clipboard text, so
pasted content like "Call (800) 555-0199 now" reached the dial pad with
surrounding words and formatting intact. Strip everything that is not a
dialable character ([^\d+*#]) and keep + only as a leading prefix; still
require at least 3 digits.

* chore: bump version to 4.15.0

* CORE-2201 Rewrite App settings panel copy and group into sections

Rename the panel to "App settings" and restructure the General tab
from a flat toggle list into five sections (App UI, System UI, System
behavior, Calling, Other & technical), reordered per platform.

Rewrite every label to sentence case and add a plain-language
description to each setting, using "workspace" instead of "server"
in user-facing strings. Add a macOS "Menu bar extra" variant for the
tray icon, a "Bounce dock icon" label for flash frame, and a
disabled-state hint for Minimize on close. Strings only; every toggle
and input keeps the same preference key and behavior.

* Rename Settings menu entries to App settings

Update the workspace-bar overflow menu and the native app menu item
that open the settings panel so their labels match the renamed
"App settings" panel.

* Hide telephony preferred server setting from App settings

Remove the TelephonyServer picker from the settings panel so the
unreleased click-to-call feature stays hidden on master. The full
feature, including this UI, lands via PR #3325. Backend telephony
code is left in place but dormant.

* Translate App settings rewrite into 15 locales

Apply the CORE-2201 settings copy rewrite to de-DE, es, fi, fr, hu, ja,
no, pl, pt-BR, ru, sv, tr-TR, uk-UA, zh-CN and zh-TW: rename the panel
to App settings, add the five section headings, retranslate changed
labels and descriptions (workspace terminology, sentence case), and add
the macOS menu-bar-extra and bounce-dock-icon variants plus the
minimize-on-close hint where the parent key exists.

Sparse stub locales (ar, it-IT, nb-NO, nn, se, zh) are left to fall back
to en-US, matching their existing settings coverage.

* fix: share main webview session with internal video chat window

Internal conferences opened via openInternalVideoChatWindow ran in a
webview hardcoded to the isolated `persist:jitsi-session` partition, so
they did not share cookies or localStorage with the server webview they
were opened from. Electron scopes cookies/localStorage to the
(partition, origin) pair, so a same-origin conference loaded
unauthenticated (the login token lives in localStorage under the server
origin).

Load the call webview in the originating server's partition
(`persist:<serverUrl>`, resolved from the caller webContents) so the
call shares the main webview's session.

Because session-level handlers are per-session, sharing the session means
the call window's handlers would otherwise clobber the main webview's:
- Screen sharing now uses a single unified display-media handler that
  routes by originating frame (in-call requests open the picker in the
  call window; main-app requests fall back to the server-view picker),
  and the plain server-view handler is restored when the call closes.
- The teardown permission-handler reset is skipped on a shared session
  so it can't disable permissions on the live main webview.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: harden video call window session sharing for production

Make the POC session-share fix correct for all users.

- Thread per-window state (ActiveCall record) instead of reading the
  mutable pendingVideoCallPartition global at lifecycle points that span
  async ticks; narrow the global to renderer-handshake reads only.
- Restore the server-view display-media AND permission handlers from
  every teardown path (closed, cleanup, render-process-gone) so a shared
  session can't leave the main webview's screen sharing or permission
  prompts disabled after a call ends. Restore is idempotent.
- Fix isSharedSession staleness: snapshot the per-window record at
  webview attach and evaluate routing at display-media request time.
- Guard terminal state null-out with identity (=== capturedCall) so a
  stale prior-window teardown can't wipe a newer call's state.
- Serialize the open-window handler via a promise-chain mutex to close a
  rapid-double-open race that orphaned a window with leaked handlers.
- Retain persist:jitsi-session as the unresolved-server fallback so such
  calls keep stable isolated storage.

Add ipc.main.spec.ts covering restore (shared/fallback/destroyed/
idempotent), partition assignment, render-process-gone, the null-out
guard, and the serialization race.

* fix: address CodeRabbit review feedback on #3370

Hardening fixes from CodeRabbit review (verified against code, false
positives rejected):

Security / stability:
- videoCallWindow/ipc.ts: validate URL protocol (http/https) before the
  g.co external-open escape hatch (was reachable via ftp://g.co/...);
  tighten host match to exact g.co / *.g.co (was overmatching evilg.co);
  guard getRootWindow() rejection in restoreServerViewHandler so teardown
  can't produce an unhandled rejection (display-media restore still runs).
- serverView/index.ts: gate the 'Permission request' debug log behind
  NODE_ENV==='development' (was logging payloads in production); wrap
  isProtocolAllowed() in try/catch so the openExternal permission callback
  always fires even on malformed URLs.
- telephony/main.ts: handle shell.openExternal() promise rejection with
  .catch(); attach 'error' listeners to gnome/kcmshell spawns before
  unref() so a missing executable can't throw unhandled.
- screenSharing/serverViewScreenSharing.ts: reset cached init state on
  provider init failure so later requests can retry instead of reusing a
  rejected promise until restart.
- telephony/dialpad.ts: guard webContents.send against a destroyed handle.

Data integrity:
- PersistableValues.ts: preserve persisted telephonyGlobalShortcutConfig
  in the >=4.14.0 migration instead of resetting it to defaults on upgrade.

UI / i18n:
- TelephonyDiagnostics.tsx: catch rejected get-diagnostics IPC and set a
  controlled empty state instead of leaking an unhandled rejection.
- i18n/ar, i18n/es: split reservedAccelerator into reservedByApp /
  reservedByOS to match the runtime keys (other locales fall back to en).

QA tooling / installer:
- validate-flows.mjs: enforce qase_id PRESENCE (key may be null per the
  flow contract) rather than truthiness, which would have rejected every
  existing flow; handle CRLF frontmatter delimiters.
- export-qase-csv.mjs: normalize CRLF on read for cross-platform parsing.
- package.json: declare yaml as a devDependency (used by export-qase-csv).
- msiProjectCreated.js: fail fast on telephony RegWrite errors in the
  WriteTelephonyCapabilities custom action.

Tests:
- main.spec.ts: mock shell.openExternal as a resolved promise.
- dialpad.spec.ts / deepLinks/main.spec.ts: add isDestroyed() to
  webContents mocks for the new destroyed-handle guard.
- TelephonyGlobalShortcut.spec.tsx: add configurable:true so the
  process.platform restore in afterAll doesn't throw.

Rejected:
- Rename telephony/main.spec.ts -> main.main.spec.ts: false positive.
  jest testMatch already routes src/**/main.spec.ts to the main-process
  project; the rename would break discovery.

Verified: tsc clean, lint clean, full suite 456 passed / 2 skipped / 0
failed, validate-flows passes all 14 telephony flows.

* fix(i18n): complete settings option keys across locales

CodeRabbit flagged locales missing settings.options keys introduced by
the settings UX rewrite, causing controls to fall back to English.

- Add missing settings.options keys (debugLogging, e2ePdfPreviewSizeLimit,
  detailedEventsLogging, outlookCalendarSyncInterval, verboseOutlookLogging,
  telephonyServer, and more) to de-DE, fi, hu, no, pt-BR, sv, uk-UA
- Add missing settings.general tab label to ja and zh-CN
- Translate settings.general no: General -> Generelt

All 15 locales now match en.i18n.json (22 settings.options keys).

* feat: openInMainWindow bridge to navigate the main window from the video call window

The standalone internal video-chat window is its own BrowserWindow with no
window.opener, so the web app's window.open/opener trick can't reach the main
window — it just spawns another window. Add an IPC path so the conference page
can ask the main app window to focus itself and navigate to an in-app route.

Caller (video-chat window):
- window.videoCallWindow.openInMainWindow(path) — validates that path is an
  in-app relative route ("/..."), rejecting absolute/protocol-relative/scheme
  URLs, then invokes 'video-call-window/open-in-main-window'.

Main process:
- New handler resolves the target server webview (caller's own server, else the
  active currentView.url), shows/restores/focuses the main window, and emits
  'navigate-to-route' (payload: path) to that server webview's webContents. It
  intentionally does NOT loadURL — that would hard-reload the SPA. No-ops safely
  with a warning when no window/webview is found, and re-validates the path.

Receiver (server webview is contextIsolated, so a raw send lands in the preload,
not the page):
- New navigateToRoute preload relay listens on 'navigate-to-route' and forwards
  to a RocketChatDesktop.onNavigateToRoute(callback) the web client registers,
  buffering the latest path if it arrives before registration (mirrors the
  telephony relay).

Also: Cmd/Ctrl+Shift+D ("Toggle Developer Tools") now targets the focused window
(falling back to the main window), so it can open DevTools for the video call
window instead of always the main one.

Web-repo follow-up (out of scope here): the web client must call
RocketChatDesktop.onNavigateToRoute(path => router.navigate(path)) for the route
change to take effect.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: route open-in-main-window to the call's origin server

When the standalone video call window asks the main window to navigate,
the handler resolved the target server from the caller webContents and
fell back to whichever server was active in the main window. In a
multi-workspace setup that could navigate a *different* server than the
one the call belongs to.

Resolve in priority order: caller's own server, then the active call's
origin server (authoritative, via activeCall.serverWebContentsId), then
the active view as a last-resort guess (now logged as ambiguous). Also
log in the preload bridge when a non-relative path is rejected, for
parity with the main-process handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat: add videoCallWindow.close() bridge to close the video call window

The internal video-chat window is a BrowserWindow created by the main process,
so the renderer's own window.close() can't close it. Expose a close() method on
the window.videoCallWindow bridge that asks the main process to do it.

- Preload: close: () => ipcRenderer.send('video-call-window/close') (no payload).
- Main: ipcMain.on('video-call-window/close') resolves the window from the sender
  (with a hostWebContents fallback for the webview-guest sender) and calls
  win.close() when it's live. Resolving from the sender means a renderer can only
  close its own window.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: focus the existing video call window when reopening the same conference

Clicking "join" again from the main window while the video call window was
already open tore the window down and recreated it. When the requested
conference URL matches the one already open, focus the existing window instead
(restore if minimized, show, focus) and return early, leaving activeCall,
provider, credentials and partition untouched. A different URL still
closes + recreates as before.

Track the conference URL on activeCall so the decision uses lifecycle state
rather than the renderer-handshake globals.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: open external links from the video call window in the system browser

The internal video-chat window didn't route external links to the system
browser like the main window does, so target="_blank" / window.open links from
the conference chat (which run in the webview guest, whose setWindowOpenHandler
was unset) spawned a new Electron window instead.

Set the guest webview's window-open handler on attach: http(s) popups return
{ action: 'deny' } and open via the system browser (openExternal), smb:// is
denied, anything else stays in-app. Also add a will-navigate handler that sends
external-scheme target="_self" navigations (mailto:, tel:, custom schemes) to
the browser, while leaving http(s) self-navigations in the webview so the
conference's own flows (auth redirects, etc.) keep working.

Extract the shared deny/openExternal policy into a helper reused by the host
window's existing handler so host and guest behave identically.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address review feedback on video call session sharing

- media/openExternal permission branches in serverView now catch
  rejections and deny instead of leaving the request hanging
- video call window-open policy denies popups by default, allowing only
  about:/blob: in-app schemes (closes javascript:/data:/file:/smb:)
- install the media permission handler on the conference webview's
  partition session (isolated fallback only) so mic/cam requests route
  through handleMediaPermissionRequest instead of Electron's default
- resolve the partition / set activeCall only inside the
  window-creating branch, after URL validation and the g.co redirect,
  so a bailed-out open can't leave stale state for teardown to misread
- swallow the fire-and-forget open-in-main-window invoke rejection
- drop no-op awaits on synchronous getNormalBounds()/getURL()
- add regression tests for popup scheme policy and the fallback-session
  permission handler

* CORE-2201 Apply UXDQA feedback to App settings panel

Address designer review on the General tab:
- Hide section heading labels (keep the grouped layout/order)
- Stack select/input controls below their description, full-width
- Move per-control caveats ("Requires app restart", "Reloads app on
  change") into dim c1 hint sublines instead of inline sentences
- Drop the redundant "System default uses..." browser sentence
- Render the video-calls description without bold product names
- Make Clear screen capture permissions a secondary danger button
- Use smart quotes around "do not ask again" in calling copy
- Group the PDF preview size limit next to Hardware acceleration

Full-width controls: SettingField wraps the control in a flex row so
Fuselage Select/InputBox (flex-grow:1) fill the column; drop the
maxWidth caps. Caveat sublines use fontScale c1 (regular) not micro
(which is bold). Strings updated across all populated locales.

* CORE-2201 Use Fuselage 3-tier field layout for App settings

Route every settings row through Fuselage's canonical
FieldLabel / FieldDescription / FieldHint stack instead of
misusing FieldHint for description text and hand-rolling hints
as <Box fontScale='c1'>.

- Add shared ToggleField component (label+toggle row, then
  FieldDescription, optional FieldHint, children escape hatch)
- Add description prop to SettingField (Select/Input rows)
- Migrate all 13 toggle and 5 select feature components
- Split the "App restarts when this option is changed" sentence
  out of videoCallScreenCaptureFallback.description into a
  dedicated hint key, across all 22 locales
- Backfill option keys missing from ar, it-IT, nb-NO, nn, se, zh
- ScreenCaptureFallback: replace Math.random id with useId

* fix: use bare 'default' Fuselage color token instead of 'font-default'

Box color= prepends the font- prefix internally, so passing
color='font-default' produced an invalid token and logged
'invalid color: font-default' on every render. Use the bare
'default' token at all 5 call sites (MarkdownContent, DocumentViewer,
PdfContent, TopBar).

* fix: memoize DownloadsManagerView selectors and label back buttons

- Wrap serverFilterOptions and the filtered downloads list in
  useMemo over a stable downloads slice, eliminating the react-redux
  'Selector returned a different result' rerender warning
- Add aria-label to the icon-only arrow-back IconButton in
  DownloadsManagerView and SettingsView for screen-reader access

* fix: remove voice/video settings duplicated in General tab

The master merge added ScreenCaptureFallback, InternalVideoChatWindow, and
VideoCallWindowPersistence to GeneralTab while the new Voice & Video tab
already rendered them alongside TelephonyServer and
ClearPermittedScreenCaptureServers. Remove all five from GeneralTab so the
Voice & Video tab is their single canonical home.

---------

Co-authored-by: Rodrigo Nascimento <rodrigoknascimento@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant