Background tabs by default: stop stealing the user's OS focus#498
Open
matheus-conin wants to merge 4 commits into
Open
Background tabs by default: stop stealing the user's OS focus#498matheus-conin wants to merge 4 commits into
matheus-conin wants to merge 4 commits into
Conversation
Every switch_tab() called Target.activateTarget unconditionally, which on macOS raises the automation Chrome window and steals the user's focus on every navigation (much worse with two daemons driving one Chrome). Window activation is never needed for automation: attach, screenshot, click and navigation all work on background tabs. - switch_tab(target, activate=False): Target.activateTarget is now opt-in, for when the user explicitly asks to SEE the tab - new_tab() / attach_first_page(): Target.createTarget with background=True - _open_chrome_inspect(): print instructions instead of osascript-activating the user's Chrome on a transient attach failure - docs updated (SKILL.md, interaction-skills/tabs.md, connection.md)
…nsertText With tabs no longer activated, input silently broke: a hidden RenderWidget doesn't process Input.dispatchMouseEvent, clicks didn't fire, and typed text never landed. Fixes validated empirically (16/16 concurrent test matrix across two daemons): - daemon: enable Emulation.setFocusEmulationEnabled on every session attach; without it mouse events to a background tab never even ack - click_at_xy: dispatch mouseMoved before press/release (the click doesn't fire on background tabs without it), then focus the focusable element under the point (compositor clicks don't move DOM focus on background tabs, breaking click-then-type) - press_key: deliver printable characters via Input.insertText between keyDown/keyUp; text on keyDown plus a char event double-inserts under focus emulation, and a char event alone doesn't insert after JS focus - fill_input: clear via element.select() instead of a Cmd/Ctrl+A key dance (the modifier stayed latched, turning every char into a shortcut); only select when the field has content (select() on an empty field drops all subsequent insertions); park the caret at the end for append mode
✅ Skill review passedReviewed 3 file(s) — no findings. |
Contributor
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/browser_harness/helpers.py">
<violation number="1" location="src/browser_harness/helpers.py:220">
P2: CI unit tests still exercise the old Cmd/Ctrl+A clearing behavior, so this helpers.py change will fail `test_fill_input_clear_first_sends_select_all_then_backspace`. Updating that test to assert the new `element.select()`/conditional Backspace behavior would keep the suite aligned with this intentional implementation change.</violation>
</file>
<file name="interaction-skills/tabs.md">
<violation number="1" location="interaction-skills/tabs.md:17">
P2: Documentation contradiction between `interaction-skills/tabs.md` and `SKILL.md`: `tabs.md` says "automation **never** needs `activate=True`" as an absolute rule, but `SKILL.md` (added in the same PR) documents a specific case where automation *does* need it — "Rare visibility-sensitive page (pauses video/animation when hidden): escalate with `switch_tab(tid, activate=True)` for that step only." This leaves an agent reading both files with inconsistent guidance. Recommend softening the `tabs.md` phrasing to align with the SKILL.md edge case, e.g. "Automation almost never needs `activate=True`" or "Automation never needs `activate=True` for normal interaction — screenshots, clicks, and navigation all work on background tabs. The one exception is visibility-sensitive pages (pauses video/animation when hidden); see SKILL.md."</violation>
</file>
<file name="src/browser_harness/daemon.py">
<violation number="1" location="src/browser_harness/daemon.py:280">
P2: If `Emulation.setFocusEmulationEnabled` fails (times out or errors) during `switch_tab`/`new_tab`, the failure is silently logged and the session is activated without focus emulation. On that tab, `click_at_xy` would dispatch mouse events that never get processed by the RenderWidget — clicks silently fail with no error to the caller. Consider adding a lightweight post-condition check after `set_session`: evaluate a small JS snippet (e.g., `document.title`) to verify the session is responsive, and raise/re-attach if focus emulation may not have taken effect on that session.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- Replace the Cmd/Ctrl+A clearing test with tests for the new element.select() behavior, including the empty-field case where Backspace must be skipped - Update the set_session parallelism tests: the gather now carries 4 enables + focus emulation (peak 5 on attach, 6 on switch) - tabs.md: soften 'never needs activate=True' to match the visibility-sensitive-page exception documented in SKILL.md - daemon: retry Emulation.setFocusEmulationEnabled once and log a loud WARNING naming the consequence (background input dropped) and the workaround when it still fails
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/browser_harness/daemon.py">
<violation number="1" location="src/browser_harness/daemon.py:278">
P2: A slow focus-emulation attach path can make switch_tab()/new_tab() time out client-side before the daemon replies. The retry loop gives this single background-tab setup step up to 8s, exceeding the helper IPC socket's 5s timeout despite set_session being on the synchronous path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…off-path The sequential retry gave a single gather member up to 8s, exceeding the helper IPC socket's 5s read timeout on the synchronous set_session path. First attempt stays inline (same 4s budget as the other enables); the retry and the loud failure WARNING move to a background task.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/browser_harness/daemon.py">
<violation number="1" location="src/browser_harness/daemon.py:300">
P2: If the first focus-emulation attempt times out, `set_session` now returns before the retry finishes. `switch_tab()` immediately hands the tab back to callers, so the first click/keypress on a newly attached background tab can still race the retry and be dropped. Keeping the retry in the request path, or exposing a ready state before input is accepted, would preserve the current readiness contract.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
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
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.
Problem
Every
switch_tab()callsTarget.activateTargetunconditionally. On macOS this raises the automation Chrome window to the foreground and steals the user's OS focus on every navigation (new_tab/ensure_real_tabboth go throughswitch_tab). With two daemons driving one Chrome it becomes a constant interruption — the user can't type, their mouse target moves, foreground app changes mid-task.Window activation is never needed for automation: attach, screenshot (
Page.captureScreenshot), click (Input.dispatchMouseEvent) and navigation all work on a background tab.Changes
Stop stealing focus (commit 1)
switch_tab(target, activate=False)—Target.activateTargetis now opt-in, for when the user explicitly wants to see the tab.new_tab()/attach_first_page()create the tab withbackground=True._open_chrome_inspect()prints instructions instead ofosascript-activating the user's Chrome on a transient attach failure.SKILL.md,interaction-skills/tabs.md,connection.md).Keep input working on background tabs (commit 2)
Once tabs are no longer activated, a hidden
RenderWidgetstops processing input, so this is required to not regress clicks/typing:Emulation.setFocusEmulationEnabledon every session attach — without it, mouse events to a background tab never even ack.click_at_xy: dispatchmouseMovedbefore press/release (the click doesn't fire on background tabs otherwise), then focus the focusable element under the point (compositor clicks don't move DOM focus on background tabs, which broke click-then-type).press_key: deliver the printable character viaInput.insertTextbetween keyDown/keyUp (text on keyDown plus a char event double-inserts under focus emulation; a char event alone doesn't insert after JS focus).fill_input: clear viaelement.select()instead of a Cmd/Ctrl+A key dance (the modifier stayed latched, turning every char into a shortcut); only select when the field has content; park the caret at end for append mode.Validation
Local empirical battery, 16/16 PASS: 10 concurrent cases across two daemons (load, clicks in 4 geometries, typing, multi-tab) + 5 typing cases + 1 opt-in
activate=True. A frontmost-app monitor sampling at 0.3s across the whole run (3612 samples) recorded the automation Chrome as frontmost in exactly 3 samples — all inside the intentionalactivate=Truetest — and never during normal automation. Screenshots of background tabs rendered real content (verified the anti-throttling launch flags are needed; those live in the caller's launch script, not this repo).Tested on macOS, Chrome 150, dedicated debugging profile on a non-default port.
Summary by cubic
Tabs now stay in the background by default to stop Chrome from stealing OS focus on navigation. Use
switch_tab(..., activate=True)only when the user wants to see a tab or when a visibility‑sensitive page requires it;new_tab()and first attach create background tabs, and_open_chrome_inspect()only prints instructions.Emulation.setFocusEmulationEnabledon session attach so background tabs accept input; retry once asynchronously to keepset_sessionwithin 4s, and warn with a workaround if it still fails.click_at_xy: sendmouseMovedbefore press/release; then focus the focusable element under the point.press_key: deliver printable characters viaInput.insertTextbetween keyDown/keyUp.fill_input: clear viaelement.select()only when content exists; skip Backspace on empty fields; move caret to end for append mode.Written for commit a1721f7. Summary will update on new commits.