Skip to content

fix(tab-bar): unstick Ctrl+Tab switcher opened from a focused browser guest - #9954

Closed
Jingyue-Wu wants to merge 1 commit into
stablyai:mainfrom
Jingyue-Wu:Jingyue-Wu/switch-tab
Closed

Jingyue-Wu wants to merge 1 commit into
stablyai:mainfrom
Jingyue-Wu:Jingyue-Wu/switch-tab

Conversation

@Jingyue-Wu

@Jingyue-Wu Jingyue-Wu commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #9937.

Fixed issue where the Ctrl+Tab tab switching overlay gets stuck when activated from within a browser window.

Screen.Recording.2026-07-22.at.12.36.06.PM.mov

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • pnpm build
  • Added regression tests:
    • New E2E spec tests/e2e/browser-ctrl-tab-switcher.spec.ts — reproduced the stuck overlay pre-fix (drives the real guest before-input-event path via webview.sendInputEvent), green post-fix: (1) opens from the guest and commits on modifier release; (2) advances while held, commits back to the browser tab, and returns focus to the guest page.
    • Unit: RecentTabSwitcher.test.tsx (webview blur on IPC open; focus-request restore on cancel), browser-guest-ui.test.ts (guest blur clears held-chord state).

AI Review Report

Reviewed with Claude Code. The root cause was established empirically before writing the fix: an instrumented E2E run showed guest keydowns reaching before-input-event while every keyup after the prevented Ctrl+Tab keydown was suppressed, ruling out IPC wiring and renderer-listener explanations.

Risks checked:

  • Gesture correctness — repeat keydowns, Shift-reversed direction, Tab-release-while-Ctrl-held (must not commit), Escape cancel, and window blur cancel all flow through the existing renderer handlers after the webview blur; no chord logic changed.
  • Focus lifecycle — blur only fires when the switcher actually opened (guard on switcherRef) and the active element is a <webview>; restore targets the browser page id resolved from the tab (activePageId), matching what BrowserPane consumes. An earlier draft passed the tab id — the E2E focus-restore test caught it.
  • Stale state — guest blur now clears ctrlTabSwitching so a later unrelated keyup can't emit a phantom ui:ctrlTabKeyUp; a phantom commit is additionally a no-op in the renderer (null switcher guard).
  • Non-browser guests — simulator/emulator webviews still get the blur (commit works); source-restore is intentionally skipped when activeTabType !== 'browser'.

Cross-platform: explicitly checked for macOS, Linux, and Windows. No new chords or labels; matching stays behind matchesRecentTabSwitcherChord/the keybinding registry (Ctrl+Tab default on all platforms), the Chromium keyup-suppression behavior is platform-independent (bug reported on Linux, reproduced on macOS), and the change touches no paths, shell behavior, or platform-conditional Electron APIs. SSH use case unaffected — UI keyboard routing only.

Security Audit

  • IPC: no new channels; the fix consumes the existing main→renderer ui:ctrlTabKeyDown/ui:ctrlTabKeyUp events, which originate only from the main process's guest shortcut forwarding — guest page content cannot forge them and no guest-supplied data flows into the new code paths.
  • Input handling: the renderer acts only on trusted store state (tab/page ids) plus document.activeElement tag checks; the guest keydown remains preventDefault()-ed, so pages still never observe the Ctrl+Tab chord.
  • Focus requests: reuse the existing TTL-bounded queueBrowserFocusRequest path already used by the palette; ids come from the renderer store, not from guest content.

ELI5

Ctrl+Tab from inside a browser tab could leave the tab switcher stuck on screen. Input is handled so the overlay closes when you release the keys, even when focus was in the browser guest.

… guest

When the Ctrl+Tab gesture starts inside a focused webview guest, main
preventDefault-s the chord keydown, and Chromium then suppresses every
guest keyup until the next keydown — so the ui:ctrlTabKeyUp commit could
never arrive and the switcher overlay stayed stranded open (stablyai#9937).

Hand the rest of the held gesture to the renderer window instead: opening
the switcher from a guest blurs the webview so advance, release-commit,
and Escape all flow through the window key handlers, and commit/cancel
return focus to the browser page when appropriate. Also clear the guest's
held-chord flag on blur so stale state cannot emit a phantom commit.

Fixes stablyai#9937
@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The changes reset held Ctrl+Tab and double-tap state when a browser guest blurs. RecentTabSwitcher now transfers focus from the guest webview, tracks the source page, and requests focus on commit or cancellation. Unit and end-to-end tests cover blur cleanup, focus handoff, tab switching, and webview focus restoration.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code and tests address #9937 by handling Ctrl+Tab from a focused browser guest, restoring focus, and clearing stale guest state.
Out of Scope Changes check ✅ Passed The changes appear scoped to the Ctrl+Tab browser-guest fix and associated regression tests, with no unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed Clear and specific: it describes the Ctrl+Tab switcher bug in browser guests, which matches the main change.
Description check ✅ Passed The core template sections are present and substantive; screenshots and notes are the only missing items.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/e2e/browser-ctrl-tab-switcher.spec.ts (1)

64-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Dedupe webview lookup between focusGuestPage and sendGuestCtrlTabKeyDown.

Both helpers repeat the identical [data-browser-overlay-tab-id] lookup and "Missing webview" throw. Extracting a shared getBrowserGuestWebview(page, browserTabId) helper avoids drift if the selector or error message changes.

♻️ Proposed refactor
+async function getBrowserGuestWebview(
+  page: OrcaPage,
+  browserTabId: string
+): Promise<void> {
+  // shared lookup logic used by focusGuestPage and sendGuestCtrlTabKeyDown
+}

Extract the document.querySelectorAll('[data-browser-overlay-tab-id]') + webview lookup body from both page.evaluate callbacks into one exported evaluate-able helper string/function reused by both call sites.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7bbf7080-8f93-4350-8b29-37363fc0423a

📥 Commits

Reviewing files that changed from the base of the PR and between 334027c and 07e5364.

📒 Files selected for processing (5)
  • src/main/browser/browser-guest-ui.test.ts
  • src/main/browser/browser-guest-ui.ts
  • src/renderer/src/components/tab-bar/RecentTabSwitcher.test.tsx
  • src/renderer/src/components/tab-bar/RecentTabSwitcher.tsx
  • tests/e2e/browser-ctrl-tab-switcher.spec.ts

@nwparker

Copy link
Copy Markdown
Contributor

Closing as superseded by merged PR #9966. The issue’s maintainer update says #9966 fixed the stuck Ctrl+Tab overlay from the focused browser guest, and its scope matches this PR.

@nwparker nwparker closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Ctrl+Tab Switch Tab overlay gets stuck open when focus is on the in-app browser tab

3 participants