iOS browser stream: focus editables under replayed taps, suppress stray backspace, verbose diagnostics - #9729
Merged
Merged
Conversation
Replayed clicks reach the streamed page as DOM events, but WebKit refuses to move field focus for clicks in a window that is never key (the offscreen render host). A tapped text field never focuses, the phone keyboard never rises, and backspace falls through as page-level history back-navigation instead of deleting. Red test plus the bare panel replay seam the fix will hang off.
…iagnostics Programmatic JS focus is exempt from WebKit's key-window rule, so a replayed click now hit-tests the tap point (descending one shadow-root level) and focuses the editable it finds. The phone keyboard rises via the existing editable_focused beacon, and typing lands in the field. A bare backspace with no focused editable is suppressed instead of falling through as WebKit history back-navigation, which lost page state when users tried to delete text. Verbose browser-stream diagnostics for debugging user reports: the Mac host ring (Sentry-attached) records stream lifecycle (start, replace, stop, first frame), input replay outcomes (kind, click count, suppressed backspace, text length), focus-assist results, beacon editable transitions, and create resolutions; the phone debug log gains browser.create and browser.stream lifecycle lines.
📝 WalkthroughWalkthroughThe change adds four browser-stream diagnostic events and records lifecycle, creation, capture, focus, and input outcomes. Browser pointer and key replay now use asynchronous panel APIs. Clicks can focus editable elements, and unmodified backspace or delete is suppressed without editable focus. ChangesBrowser streaming observability and input
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalController
participant BrowserPanel
participant WebView
participant DiagnosticLog
TerminalController->>BrowserPanel: Replay pointer or key input asynchronously
BrowserPanel->>WebView: Detect or assist editable focus
BrowserPanel->>WebView: Deliver allowed input
BrowserPanel-->>TerminalController: Return focus outcome or delivery status
TerminalController->>DiagnosticLog: Record input and focus diagnostics
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 2 warnings)
✅ Passed checks (20 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swift`:
- Around line 356-363: The new diagnostic codes need semantic payload decoding
instead of generic detail fields. Update decodeA, decodeB, and decodeC to map
browserStreamLifecycle, browserInputReplayed, browserEditableFocus, and
browserPanelCreateResolved payload positions to their documented lifecycle,
input-kind, focus-assist, and panel-correlation field names and decoded values,
then include all four codes in decodesEveryStructuredPayloadIntoSemanticFields.
In `@Sources/Mobile/MobileBrowserStreamSession.swift`:
- Around line 264-268: Move the stage-4 DiagnosticEvent recording from before
frame encoding and delivery to immediately after the first successful
connection.sendEvent(topic: "browser.frame", ...) in the browser stream
lifecycle flow. Ensure it is emitted only once per stream, and do not record it
for later synchronized PNG captures or failed sends.
In `@Sources/Panels/BrowserPanel`+MobileBrowserStreaming.swift:
- Around line 159-175: The editable-focus predicate in
mobileBrowserEditableHasFocus must traverse the active element through shadow
roots instead of relying only on document.activeElement. Reuse the same
shadow-aware predicate in mobileBrowserDirtyBeaconScript so both focus checks
behave consistently, and add a regression test that focuses an input inside a
shadow root and verifies a bare backspace is delivered.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c9e6430d-db50-4f86-8806-b0f6cedc0e17
📒 Files selected for processing (9)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventCode.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+BrowserStream.swiftSources/Mobile/MobileBrowserStreamCoordinator.swiftSources/Mobile/MobileBrowserStreamSession.swiftSources/Panels/BrowserPanel+MobileBrowserStreaming.swiftSources/TerminalController+MobileBrowser.swiftcmuxTests/BrowserPanelTests.swift
…ry, shadow-aware focus checks The four browser diagnostic codes now decode into named fields (stage, input kind, count, focus outcome, panel correlation) instead of falling through to detail_1/2/3, with a suppressed backspace becoming its own input kind so counts stay unambiguous. The first-frame lifecycle stage records once per session after the first successful browser.frame delivery, never for a capture whose send failed. Editable-focus detection descends shadow roots in the suppression check, the focus assist's already-focused check, and the beacon, so widget-wrapped inputs receive backspace and raise the keyboard; regression test covers backspace delivery to a shadow-root input.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/TerminalController+MobileBrowser.swift (1)
215-224: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecord the actual editable-focus state.
At Line 222,
focusAssist == 0means that no editable exists at the tap point. It does not mean that no editable remains focused. A click on a non-focusable element can retain focus in an existing input, but this recordsa: 0.After replay, call
await panel.mobileBrowserEditableHasFocus()and use that result fora. KeepfocusAssistinb.Proposed fix
if input.kind == .click { mobileBrowserRecordDiagnostic( .browserEditableFocus, panel: panel, - a: focusAssist == 0 ? 0 : 1, + a: await panel.mobileBrowserEditableHasFocus() ? 1 : 0, b: focusAssist ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/TerminalController`+MobileBrowser.swift around lines 215 - 224, Update the .browserEditableFocus diagnostic in the input.click branch to await panel.mobileBrowserEditableHasFocus() and use its result for field a, preserving focusAssist as field b; do not infer the actual focused-editable state from focusAssist.Sources/Panels/BrowserPanel+MobileBrowserStreaming.swift (1)
12-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one editable classification in the dirty beacon.
The dirty-beacon predicate excludes
select, butmobileBrowserEditableHasFocus()andassistMobileBrowserEditableFocus(...)include it. After focus assist selects a<select>, the beacon can publisheditable_focused: falseand overwrite the stream state.Include
tag === 'select'in this predicate, or removeSELECTfrom the other two predicates. Keep all three paths consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Panels/BrowserPanel`+MobileBrowserStreaming.swift around lines 12 - 19, Update the dirty-beacon editable predicate in the JavaScript around mobileBrowserEditableHasFocus to classify select elements consistently with mobileBrowserEditableHasFocus() and assistMobileBrowserEditableFocus(...). Prefer adding the select tag to this predicate while preserving the existing input, textarea, and contenteditable checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/Panels/BrowserPanel`+MobileBrowserStreaming.swift:
- Around line 12-19: Update the dirty-beacon editable predicate in the
JavaScript around mobileBrowserEditableHasFocus to classify select elements
consistently with mobileBrowserEditableHasFocus() and
assistMobileBrowserEditableFocus(...). Prefer adding the select tag to this
predicate while preserving the existing input, textarea, and contenteditable
checks.
In `@Sources/TerminalController`+MobileBrowser.swift:
- Around line 215-224: Update the .browserEditableFocus diagnostic in the
input.click branch to await panel.mobileBrowserEditableHasFocus() and use its
result for field a, preserving focusAssist as field b; do not infer the actual
focused-editable state from focusAssist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a8fdad91-5925-4212-b63d-4848da5c72b1
📒 Files selected for processing (7)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventCode.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftSources/Mobile/MobileBrowserStreamSession.swiftSources/Panels/BrowserPanel+MobileBrowserStreaming.swiftSources/TerminalController+MobileBrowser.swiftcmuxTests/BrowserPanelTests.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b584cc9. Configure here.
| if (tag !== 'INPUT') return false; | ||
| const type = String(el.type || 'text').toLowerCase(); | ||
| return !['button','checkbox','color','file','hidden','image','radio','range','reset','submit'].includes(type); | ||
| }; |
There was a problem hiding this comment.
Select treated as text editable
Medium Severity
isEditable in the new focus and backspace helpers treats SELECT as editable, but the dirty-beacon check does not. With a focused select, bare backspace is delivered and WebKit can navigate history—the failure this PR aims to stop—especially while the phone keyboard is still up after leaving a text field or via manual keyboard request.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit b584cc9. Configure here.
| if (inner) el = inner; | ||
| } | ||
| while (el && !isEditable(el)) el = el.parentElement || (el.getRootNode && el.getRootNode().host) || null; | ||
| if (!el) return 0; |
There was a problem hiding this comment.
Label taps skip focus assist
Medium Severity
Focus assist only walks from the hit element through parents and shadow hosts for an editable. Taps on a label (via for or wrapping text) never resolve the associated control, so those common form targets still leave document.activeElement on BODY and the phone keyboard does not rise.
Reviewed by Cursor Bugbot for commit b584cc9. Configure here.
| if (el && el.shadowRoot) { | ||
| const inner = el.shadowRoot.elementFromPoint(\(x), \(y)); | ||
| if (inner) el = inner; | ||
| } |
There was a problem hiding this comment.
Shadow text insert still shallow
Medium Severity
Focus assist can focus an input inside a shadow root, and the beacon/backspace paths now pierce shadow trees, but insertTextWithJavaScript still uses shallow document.activeElement (the host). Non-ASCII phone text to widget-wrapped fields then fails insertion even though the field is focused and the keyboard is up.
Reviewed by Cursor Bugbot for commit b584cc9. Configure here.
6 of 8 tasks
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
1 participant
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.
Summary
Fixes the streamed-browser input bugs Aziz hit dogfooding #9577, and adds the verbose browser-stream diagnostics he asked for.
Symptoms (all one root cause): tapping a page text field does nothing (no caret, no keyboard); typed text lands nowhere on most pages; backspace with "highlighted" text navigates the page back in history instead of deleting. Diagnosis: replayed phone clicks reach the page as DOM events (verified:
mousedown/clickwith correct coordinates and target), but WebKit refuses to move field focus for clicks in a window that is never key, and the streamed webview lives in the offscreen render host whose window is deliberately non-key. Withdocument.activeElementstuck onBODY,editable_focusednever fires (no phone keyboard), text insertion has no target, and backspace falls through as WebKit's page-level history back-navigation.Fix
BrowserPanel.replayMobileBrowserPointerreplays the click and then runs a focus assist: programmatic JS focus is exempt from the key-window rule, so it hit-tests the tap point (elementFromPoint, descending one shadow-root level for widget-wrapped inputs, then walking ancestors) and callsfocus({preventScroll: true})on the editable it finds. The existingeditable_focusedbeacon then raises the phone keyboard, and both text-insertion paths (key events and JS insertion) have a real target.BrowserPanel.replayMobileBrowserKeysuppresses a bare backspace when no editable has focus: the phone keyboard's backspace is a text-editing key, and letting it through navigated history and lost page state. Modified combinations pass through unchanged.testReplayedClickFocusesEditableUnderTap(red on the bare replay seam) then the assist;testBareBackspaceOutsideEditableIsSuppressedcovers the suppression plus delivery-while-editing.Verbose browser-stream diagnostics
Mac host ring (Sentry-attached via the existing diagnostics tap): new
DiagnosticEventCodes record stream lifecycle (start / replace / stop / first committed frame), input replay outcomes (kind, click count, suppressed backspace, text length), focus-assist results, beacon editable-focus transitions, andmobile.browser.createresolutions, all correlated by a stable panel ID. Phone debug log (Copy Debug Logs, feedback bundles):browser.createoutcomes andbrowser.streamstart / start-deferred / start-failed / stop / force-restart / closed-by-mac lines.Localization audit: no user-facing UI strings changed; the four new diagnostics titles follow the package's existing
localized(_:defaultValue:)pattern used by all 55 prior event codes.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes iOS browser stream input so tapped fields focus, typing goes into the page, and bare backspace no longer navigates history. Adds shadow‑root‑aware focus detection and richer, semantic diagnostics (including first delivered frame) to debug lifecycle, input, and focus.
Bug Fixes
elementFromPoint, descend one shadow root) and focus the editable via JS to bypass WebKit’s key-window rule.testReplayedClickFocusesEditableUnderTap,testBareBackspaceOutsideEditableIsSuppressed,testBackspaceDeliversToShadowRootInput.New Features
browserStreamLifecycle,browserInputReplayed(now includes a distinct “suppressed backspace” kind),browserEditableFocus,browserPanelCreateResolved— with stage/input/count/focus-outcome and a stable panel correlation ID.browser.createoutcomes andbrowser.streamstart/start-deferred/start-failed/stop/force-restart/closed-by-mac.Written for commit b584cc9. Summary will update on new commits.
Note
Medium Risk
Changes Mac-side WebKit input replay and RPC key handling (async,
deliveredflag); user-visible browser streaming behavior but confined to the mobile browser stream path with new test coverage.Overview
Fixes iOS streamed-browser input when the Mac web view lives in a non-key offscreen host: replayed taps did not focus fields (no keyboard, typing went nowhere), and bare backspace could trigger history navigation.
Input behavior: After pointer replay, click focus assist hit-tests the tap (
elementFromPoint, shadow-root descent) and calls programmaticfocus()so editables can receive keys. Bare backspace is dropped when no editable is focused; shadow-root–aware checks align the dirty beacon and suppression logic with widget-wrapped inputs.Diagnostics: Four new
DiagnosticEventCodevalues (56–59) with presentation/decoding record stream lifecycle, input replay outcomes, editable focus, andmobile.browser.createresults on the Mac host ring (panel correlation IDs). The phone shell addsMobileDebugLog.anchormuxlines for browser create and stream start/stop/restart paths.Tests:
MobileBrowserStreamInputFocusTestscovers click-to-focus, backspace suppression, and shadow-root backspace delivery.Reviewed by Cursor Bugbot for commit b584cc9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes