Skip to content

fix(web): preserve composer focus during browser automation - #4118

Closed
mwolfeio wants to merge 1 commit into
pingdotgg:mainfrom
mwolfeio:codex/preserve-composer-focus-browser-automation
Closed

fix(web): preserve composer focus during browser automation#4118
mwolfeio wants to merge 1 commit into
pingdotgg:mainfrom
mwolfeio:codex/preserve-composer-focus-browser-automation

Conversation

@mwolfeio

@mwolfeio mwolfeio commented Jul 18, 2026

Copy link
Copy Markdown

What Changed

  • Preserve the renderer DOM focus around agent browser press operations.
  • Restore the previously focused element in a finally block, including when automation fails.
  • Reuse the same best-effort focus helper in the existing element-picker flow.
  • Add coverage for success, failure, and detached-element cleanup.

Why

Native browser key dispatch temporarily focuses Electron’s guest WebContents. The main process restores the outer renderer afterward, but not its active DOM element, so the composer stays blurred and interrupts users typing queued follow-ups while the agent works. This restores the prompt editor without changing the guest-focus behavior required by browser automation.

UI Changes

Behavior-only fix; there are no visual changes. Before this change, an agent browser key press leaves the composer unfocused. Afterward, the same editor regains focus without scrolling.

Validation

  • vp check (0 errors; 10 pre-existing warnings)
  • vp run typecheck
  • Web unit suite: 152 files / 1,321 tests passed
  • Focus regression test: 3 tests passed

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Screenshots are not applicable because there is no visual change
  • Interaction video not included

Note

Low Risk
Small, localized renderer UX fix with best-effort focus restoration and no changes to auth, data, or automation semantics beyond wrapping press.

Overview
Adds shared documentFocus helpers that snapshot the renderer’s active element and restore it with focus({ preventScroll: true }) in a finally block, skipping detached or invalid targets.

Agent browser press in PreviewAutomationHosts now runs inside runPreservingDocumentFocus so guest WebContents key dispatch no longer leaves the chat composer blurred while the user types follow-ups.

The element-picker flow in PreviewView uses the same capture/restore helpers instead of inline focus logic. Unit tests cover success, failure, and detached-element cases.

Reviewed by Cursor Bugbot for commit 6ae4382. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Preserve composer focus during browser automation press actions

  • Extracts focus capture/restore logic into a new documentFocus utility module (documentFocus.ts) with captureDocumentFocus, restoreDocumentFocus, and runPreservingDocumentFocus helpers.
  • Wraps the press automation action in PreviewAutomationHosts.tsx with runPreservingDocumentFocus so the previously focused element is restored after the operation completes or fails.
  • Replaces manual focus snapshot/restore in PreviewView.tsx with the new shared helpers.
  • Adds a unit test suite in documentFocus.test.ts covering success, failure, and detached-element cases.

Macroscope summarized 6ae4382.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7ac8dd69-2453-400a-ad1d-da2796926650

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 18, 2026
@macroscopeapp

macroscopeapp Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Straightforward refactor extracting existing focus-preservation logic into a reusable utility, with unit tests. Limited scope, self-contained UI behavior fix.

You can customize Macroscope's approvability policy. Learn more.

@t3dotgg

t3dotgg commented Sep 4, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6 Astra (preview) responding on behalf of Theo

This note is part of an automated cleanup pass.

Carryover from #5898 at 15b2a4bd29: Electron's performAutomationPress restores the previous WebContents, but that alone does not restore the composer DOM element. Check a preview press in another thread while the composer is focused, on both success and failure, with no scroll jump or thread switch. Keep the detached-element guard. #5792 also reports focus loss from other automation actions, so this press fix must not be treated as proof that every reported interaction is fixed.

@juliusmarminge

Copy link
Copy Markdown
Member

Fix first at 6ae43825.

P2: runPreservingDocumentFocus unconditionally restores its captured element after the asynchronous operation. If the person focuses another control while the request is pending, this moves focus away from their newer selection when the request completes or rejects. Extracting the same helper into the longer element-picker flow also keeps that behavior there.

I executed the exact proposed helper in an isolated Chromium DOM with two synthetic inputs and actual browser clicks. No T3 or Electron behavior was mocked into the helper:

Sequence while operation is pending Expected focus after settlement Observed
Stay in Composer, resolve Composer Composer
Click Search, resolve Search Composer
Click Search, reject Search Composer
Click Search, detach old Composer, resolve Search Search

Preserve the person's newer focus choice rather than always reclaiming the original element. Add success and failure regressions for an intentional focus change, alongside the detached-element control. The existing tests only verify the captured element's focus callback and do not cover that transition.

This establishes a flaw in the proposed helper, not reproduction of native guest focus loss or keyboard delivery. The same-thread/cross-thread Electron checks from the earlier maintainer comment remain required. #5792 also reports other automation actions, so a press-only fix must retain that limitation. Current main 4f782bed still restores only the prior WebContents in the desktop press cleanup; source inspection alone does not establish native runtime behavior.

Audited by GPT 6 Astra via Codex in T3 Code.

@juliusmarminge

Copy link
Copy Markdown
Member

Native follow-up for #5792: the focus-only proposal still needs a correct-key-target check.

I ran the unchanged PreviewManager from freshly fetched main a4953855 in Electron 43.4.1 on Linux, with a real <webview>, synthetic Composer/Search inputs, a private Xvfb display and disposable Electron state. No providers, live database, or external page requests were involved.

Actual semantic clicks established the initial focus. After calling the production automationPress(tabId, {key: "Enter"}), DOM keydown receipts showed:

Initial focus Preview visibility Enter in preview Enter in outer Composer Action result
Preview input Visible Yes No Success
Composer Visible No Yes Success
Composer Hidden No Yes Success

The final run at 06:20:31 UTC used the actual unwrapped Electron debugger methods; an earlier instrumented run had the same result. Direct semantic-click/Enter controls also delivered correctly to each intended field. The loaded source hashes were captured. In the failing controls, the outer document remained focused while getFocusedWebContents() identified the guest, so the current cleanup did not restore an outer WebContents.

This is a minimal native preview-manager reproduction, not the full T3 composer, original Arch/AppImage/window-manager setup, or proof that a real draft was submitted. The original reported focus-loss symptom was not reproduced here. Nevertheless, preserving DOM focus alone is insufficient evidence for shipping: native tests must establish that keys reach the target preview and never reach the foreground composer, as well as preserving the person's newer focus choice from the earlier review. The original issue remains open; no fix or closure claim.

Audited by GPT 6 Astra via Codex in T3 Code.

@juliusmarminge

Copy link
Copy Markdown
Member

Superseded by #11354, which fixed #5792 (preview automation no longer steals/sends composer focus across threads). Closing this earlier focus-restore approach as leftover.

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

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants