Fix attached Web Inspector resize: bottom-dock divider, small-pane minimum-size policy, WebKit size sync#7771
Fix attached Web Inspector resize: bottom-dock divider, small-pane minimum-size policy, WebKit size sync#7771austinywang wants to merge 9 commits into
Conversation
…, #663) Regression tests only — expected RED on this commit: - portal host does not claim the bottom-dock page/inspector seam in hitTest - portal host mouseDown/mouseDragged at the seam does not resize the inspector - local-inline host does not claim the bottom-dock seam smallPaneSideDockDragMovesIntoDegradedRange is a guard test for the small-pane clamp policy and may already pass on this commit. The fix lands in the next commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…licy, WebKit size sync Fixes #7762. Fixes #663. The attached Web Inspector divider was intermittently dead and glitchy, worst in small panes: bottom dock (which cmux forces when a pane is too narrow for a side dock) had no cmux-owned divider at all, side-dock candidate detection used brittle 1pt epsilons, and cmux never told WebKit the user-chosen size so WebKit's attachment relayout snapped it back. - Extract HostedInspectorDockSide into HostedInspectorResizeGeometry.swift, add .bottom, make the geometry axis-generic, raise resolve epsilon to 4, and scale the cross-axis overlap guard with the container. - HostedInspectorMinimumSizePolicy: proportional-degrade clamp whose divider range is never empty, so small panes degrade to a half/half split instead of a locked divider. - Wire bottom-dock hit-testing, resizeUpDown cursor, Y-axis drag, preferred-height record/reapply (fraction semantics) through both the window-portal host and the local-inline host. - On mouseUp, push the final size into WebKit via InspectorFrontendHost.setAttachedWindowHeight/Width on the inspector frontend webview so WebKit stops fighting the layout. - Cache the hitTest divider candidate briefly so mouseDown cannot lose the drag to layout churn between events. - Never run adaptive bottom-dock or portal bottom-dock frame repair while a divider drag is active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHosted Web Inspector resizing now supports leading, trailing, and bottom dock orientations through extent-based geometry. Divider hit caching, preferred width/height persistence, attached-size synchronization, lifecycle refreshes, and geometry and interaction tests are added. ChangesHosted Inspector resize flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserPanelView
participant HostedInspectorResizeGeometry
participant WindowBrowserSlotView
participant HostedInspectorAttachedSizeSync
participant WKWebView
User->>BrowserPanelView: Drag hosted inspector divider
BrowserPanelView->>HostedInspectorResizeGeometry: Resolve dock side and clamp extent
HostedInspectorResizeGeometry-->>BrowserPanelView: Updated page and inspector frames
BrowserPanelView->>WindowBrowserSlotView: Record preferred extent
BrowserPanelView->>HostedInspectorAttachedSizeSync: Sync final inspector extent
HostedInspectorAttachedSizeSync->>WKWebView: Evaluate attached-size JavaScript
Possibly related issues
Possibly related PRs
Suggested reviewers: 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📝 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 |
Greptile SummaryThis PR reworks attached Web Inspector resizing for side and bottom docks. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (8): Last reviewed commit: "Portal: capture cross-axis preference wh..." | Re-trigger Greptile |
| struct HostedInspectorAttachedSizeSync { | ||
| static func sync(frontendWebView: WKWebView?, dockSide: HostedInspectorDockSide, extent: CGFloat) { |
There was a problem hiding this comment.
Static Inspector Sync Namespace
This new production type is only a static helper surface, so the WebKit size-sync behavior has no owning host instance to construct or inject. The repo rule asks production Swift to keep behavior on the scoped owner; here the operation is always called from the inspector-hosting views after a drag, so it should live behind that owner instead of a new ambient namespace.
Rule Used: Flag new ambient global state in production Swift:... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Intentional: the sync is invoked from two independent hosts (window-portal WindowBrowserHostView and local-inline HostContainerView) after their drag loops, and per the repo's shared-behavior policy that must be one shared mutation path, not per-host copies. It is stateless (pure JS construction + fire-and-forget evaluate), takes the frontend webview explicitly rather than reaching into ambient state, and follows the existing pattern of BrowserDeveloperToolsDockControlNormalizer/HostedInspectorDockControlScript. Happy to fold it into a host-owned instance if a future refactor gives the two hosts a common owner.
…est fixture - Local-inline host hitTest now skips the native-hit branch for horizontal (bottom-dock) dividers, mirroring the portal host, so the inspector's own web content cannot swallow the seam band. - testBrowserPanelHostAllowsRightDockedInspectorToExpandLeftAfterPromotion used a 240pt host, which the new proportional-degrade clamp pins to a single divider position (120/120); widen the fixture to 420pt so the expand-left drag has real range ([120, 300], 148 -> 188). - Clear the cached hitTest divider candidate on mouseUp in both hosts so it cannot outlive the interaction holding view references. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ttom redock With .bottom now resolving as a divider candidate, the side-dock width math in shouldForceHostedInspectorBottomDock computed ~0 effective page width for bottom-docked layouts, so layout() kept requesting WI._dockBottom() (0.25s cooldown) and skipped the stored-extent reapply path. Bottom-dock hits can never need a redock to bottom; bail out. Reported by Cursor Bugbot on #7771. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three targeted fixes for PR #7771 dogfood findings (resize could still go under minimum sometimes; drag still glitched intermittently): - Neutralize the inspector frontend's own docked resizer (#docked-resizer gets pointer-events: none via the dock-control script). In side dock, cmux's divider band deliberately lets clicks through to inspector content, so WebKit's 3px in-content resizer was still reachable and resized with WebKit's window-based clamps — bypassing cmux's pane minimums entirely, then fighting the reapply pass. All attached resizes now go through cmux's clamped divider. - Sync the attachment size to WebKit on every drag event (deduped on integer extent), not just mouseUp. WebKit's inspectedViewFrameDidChange relayout re-applied the stale pre-drag size between cmux's frame writes, making the divider visibly fight the cursor mid-drag. - Decide adaptive bottom-dock from container capacity instead of live frame widths, so a user drag that leaves the page under 240pt no longer yanks the inspector to the bottom right after mouseUp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Panels/BrowserPanelView.swift (1)
6520-6557: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winResync after the final reapply.
mouseUpsends the attached size beforereapplyHostedInspectorDividerToStoredExtentIfNeeded(reason: "drag.end"), butapplyHostedInspectorDividerExtentonly updates frames. If the reapply changes the layout,InspectorFrontendHoststays on the pre-reapply extent. Sync from the applied frames, or clearlastSyncedHostedInspectorDragExtentbefore the final sync so the browser state matches the last layout.🤖 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/BrowserPanelView.swift` around lines 6520 - 6557, Update mouseUp and the final drag-end reapply flow so the attached inspector size is synchronized after reapplyHostedInspectorDividerToStoredExtentIfNeeded, using the resulting applied frames rather than only the pre-reapply extent. Ensure the final sync is not skipped by lastSyncedHostedInspectorDragExtent, either by deriving the extent from the applied layout or clearing that cache before syncing; use syncAttachedSizeDuringDrag and the hosted inspector frame/dock-side state.
🤖 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 `@cmuxTests/HostedInspectorDockControlScriptTests.swift`:
- Around line 129-156: In disablesFrontendDockedResizerSoCmuxOwnsAttachedResize,
simplify the mock DOM setup by declaring the docked resizer element upfront and
making document.getElementById directly return it for "docked-resizer", removing
the initial function assignment, lookup, and reassignment sequence.
---
Outside diff comments:
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 6520-6557: Update mouseUp and the final drag-end reapply flow so
the attached inspector size is synchronized after
reapplyHostedInspectorDividerToStoredExtentIfNeeded, using the resulting applied
frames rather than only the pre-reapply extent. Ensure the final sync is not
skipped by lastSyncedHostedInspectorDragExtent, either by deriving the extent
from the applied layout or clearing that cache before syncing; use
syncAttachedSizeDuringDrag and the hosted inspector frame/dock-side state.
🪄 Autofix (Beta)
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
Run ID: 111ed9fd-175f-42d7-89ba-295df7c9485f
📒 Files selected for processing (4)
Sources/App/HostedInspectorDockControlScript.swiftSources/BrowserWindowPortal.swiftSources/Panels/BrowserPanelView.swiftcmuxTests/HostedInspectorDockControlScriptTests.swift
…ess review Debug-log evidence from dogfood (browser.portal.manualInspectorDrag) showed WebKit's WebInspectorUIProxy asynchronously re-applying its stored attachment size between almost every pair of drag events — oldPageFrame kept reverting to the exact pre-drag layout, and when its reset landed last the whole drag ended where it started (the dead-feeling drag). It observes the inspected webview's frame-change notifications, which our per-event frame writes were posting. - HostedInspectorDragFrameNotificationSilencer: turn off the inspected view's postsFrameChangedNotifications for exactly the drag's duration (both hosts, all drag-exit paths), so cmux is mechanically the only layout writer while the mouse is down. mouseUp restores notifications and the existing InspectorFrontendHost size sync reconciles WebKit. - Instrument HostedInspectorAttachedSizeSync completions under DEBUG so the debug log records whether setAttachedWindowHeight/Width lands. - Review: a stored side-dock width no longer blocks adopting the current bottom-dock layout as the height preference (capture fallback when the stored extent is for the other dock axis; Cursor finding). - Review: simplify the mock document in the docked-resizer script test (CodeRabbit finding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nit-safe Dogfood log analysis of the previous build showed the per-drag-event InspectorFrontendHost.setAttachedWindow* calls were themselves the remaining glitch: during fast scrubs the inspector frontend's JS queue backs up and each queued call echoes back as a WebKit frame apply for a stale extent (befores of consecutive drag applies matched integer extents synced hundreds of milliseconds earlier), yanking the divider backwards. cmux now writes frames alone during the drag — the frame-notification silencer keeps WebKit's observer passive — and WebKit gets exactly one setAttachedWindow* reconciliation on mouseUp (instrumentation confirmed the call lands: result=1 for both axes). Also address review: the silencer now restores through a deinit-safe token, so an owner torn down mid-drag can never leave the inspected view's frame notifications permanently off (Cursor finding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Panels/BrowserPanelView.swift (1)
6521-6550: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSynchronize WebKit only after applying the final stored extent.
Line 6533 queues
finalExtent, but line 6550 may synchronously change the inspector frame. The queued JavaScript can therefore reapply a stale dimension and recreate the mouse-up snap. Reapply while notifications remain silenced, measure the resulting frame, restore notifications, and then synchronize.Apply the same ordering through a shared finalization path for
BrowserWindowPortal, whose Lines 517-533 repeat this sequence.Proposed sequencing fix
isHostedInspectorDividerDragActive = false - hostedInspectorDragFrameSilencer.end() updateDividerCursor(at: convert(event.locationInWindow, from: nil)) if let finalDragState { + reapplyHostedInspectorDividerToStoredExtentIfNeeded(reason: "drag.end") let finalExtent = finalDragState.dockSide.inspectorExtent( inspectorFrame: finalDragState.inspectorView.frame, in: finalDragState.containerView.bounds ) + hostedInspectorDragFrameSilencer.end() HostedInspectorAttachedSizeSync.sync( frontendWebView: hostedInspectorFrontendWebView, dockSide: finalDragState.dockSide, extent: finalExtent ) - reapplyHostedInspectorDividerToStoredExtentIfNeeded(reason: "drag.end") + } else { + hostedInspectorDragFrameSilencer.end() }As per coding guidelines, “Do not wire the same behavior separately through multiple surfaces; use one shared action path.”
🤖 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/BrowserPanelView.swift` around lines 6521 - 6550, Synchronize the frontend only after the final stored extent has been reapplied: in the hosted inspector mouse-up finalization, move extent measurement and HostedInspectorAttachedSizeSync.sync after reapplyHostedInspectorDividerToStoredExtentIfNeeded, while keeping drag-frame notifications silenced until reapplication completes, then measure the resulting inspector frame and restore notifications before syncing. Extract this ordering into a shared finalization helper and use it from both BrowserPanelView’s mouseUp path and BrowserWindowPortal’s equivalent sequence instead of maintaining separate implementations.Source: Coding guidelines
🤖 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 `@Sources/Panels/HostedInspectorResizeGeometry.swift`:
- Around line 47-64: Replace Restore.run’s delayed DispatchQueue.main fallback
with synchronous MainActor-owned teardown. Add explicit MainActor
drag-cancellation/teardown methods in both host views that restore
postsFrameChangedNotifications immediately and clear the active Restore token;
invoke them from all drag-ending and lifecycle paths. Keep Restore.deinit as
assertion-only fallback, remove its `@unchecked` Sendable ownership of NSView
state, and ensure restoration is managed by the single MainActor lifecycle
owner.
---
Outside diff comments:
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 6521-6550: Synchronize the frontend only after the final stored
extent has been reapplied: in the hosted inspector mouse-up finalization, move
extent measurement and HostedInspectorAttachedSizeSync.sync after
reapplyHostedInspectorDividerToStoredExtentIfNeeded, while keeping drag-frame
notifications silenced until reapplication completes, then measure the resulting
inspector frame and restore notifications before syncing. Extract this ordering
into a shared finalization helper and use it from both BrowserPanelView’s
mouseUp path and BrowserWindowPortal’s equivalent sequence instead of
maintaining separate implementations.
🪄 Autofix (Beta)
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
Run ID: 775f146f-2180-4a7e-8869-657b739be5d8
📒 Files selected for processing (3)
Sources/BrowserWindowPortal.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/HostedInspectorResizeGeometry.swift
Review follow-up: restore the drag-frame silencer through an explicit MainActor teardown path — both hosts now cancel an in-flight divider drag (drag state, cached hit, drag-active flag, silencer) in their viewDidMoveToWindow nil branch, so the deinit token is a last-resort fallback rather than the restoration path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ther dock axis Mirror of the inline host's capture fallback (Cursor finding): the portal's reapplyHostedInspectorDividerIfNeeded returned early when the resolved dock side had no stored extent (e.g. bottom dock right after a side-dock width was stored), so bottom-dock height was never adopted and later relayout could fight the divider. Adopt the current layout as that axis's preference instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 61aea05. Configure here.
| HostedInspectorAttachedSizeSync.sync( | ||
| pageWebView: dragState.slotView.hostedInspectorPageWebViewForAttachedSizeSync, | ||
| dockSide: dragState.dockSide, extent: finalExtent | ||
| ) |
There was a problem hiding this comment.
Silencer ends before size sync
High Severity
On divider mouseUp, both hosts call hostedInspectorDragFrameSilencer.end() before HostedInspectorAttachedSizeSync.sync. The silencer exists specifically to stop WebKit from reacting to frame notifications with a stale attachment size, but lifting it first re-enables that observer while the sync is still an async evaluateJavaScript round-trip. Any frame write or deferred notification in that window can snap the divider back—the same frame-fighting this change set out to close.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 61aea05. Configure here.


Fixes #7762. Fixes #663. Related: #2933 (item 2 tracks this same symptom), #7037 (same hit-swallowing failure family — prior art for the pass-through/claim approach), #617.
Problem
Resizing the attached Web Inspector was unreliable: the divider was intermittently dead to drag, glitchy when it did work, and much worse in small panes / on small displays.
Root causes:
HostedInspectorDockSideonly modeled.leading/.trailing. Bottom-dock resize relied on WebKit's tiny in-content resizer inside the inspector frontend, which cmux's portal hit-testing routes around/swallows → Cannot resize devtools console panel in built-in browser #663's completely dead divider.overlap > 8guard, andmouseDownre-resolved the divider independently ofhitTest, so transient WebKit layout churn silently dropped drags.Fix
Sources/Panels/HostedInspectorResizeGeometry.swift(pure, unit-tested):HostedInspectorDockSidemoved out ofBrowserWindowPortal.swift, gains.bottom, axis-generic divider position/hit-rect/clamp/frame math, tolerant resolve epsilon (4pt), container-scaled cross-axis overlap guard.HostedInspectorMinimumSizePolicy): side dock keeps a 120pt inspector / 120pt page minimum; bottom dock 100pt inspector / 80pt page. When the pane can't fit both minimums, each minimum degrades to half the container, so the allowed divider range is never empty — the divider clamps gracefully instead of locking up.WindowBrowserHostViewand local-inlineWebViewRepresentable.HostContainerView): hitTest claims the seam band (±6pt portal / ±10pt inline) so neither the page WKWebView nor the inspector's own web content swallows the drag;resizeUpDowncursor; Y-axis drag; preferred-height record/reapply with the same fraction-of-container semantics as width.Sources/Panels/HostedInspectorAttachedSizeSync.swift: on mouseUp, pushes the final size into WebKit viaInspectorFrontendHost.setAttachedWindowHeight/Widthon the inspector frontend webview (typeof-guarded JS, nil-safe), so WebKit's attachment manager agrees with the user-chosen size and stops fighting cmux's layout.mouseDownfalls back to the hit cached byhitTest(0.1s lifetime, ±hit-band distance) so event-to-event layout churn can't lose the drag.Commit structure (red/green)
4275c014ab) addscmuxTests/HostedInspectorBottomDockDividerTests.swiftonly — expected red: both hosts fail to claim the bottom-dock seam and the portal drag doesn't resize. Proof run on that SHA: test-e2e run — failed on exactly the three bottom-dock expectations. (smallPaneSideDockDragMovesIntoDegradedRangeis a guard test for the clamp policy and may already pass on commit 1.)5efa87918c) is the fix pluscmuxTests/HostedInspectorResizeGeometryTests.swift(pure geometry:.bottomresolve, hit-rect bands, never-empty clamp range, frame complementarity, size-sync JS shape). Same filter on the fixed code: test-e2e run — passed (run @ e793f50, which also carries the review fixes: bottom-dock seam guard in the local-inline host, widened pinned test fixture; c690206 additionally stops already-bottom-docked layouts from re-triggering the adaptiveWI._dockBottom()redock).Test coverage honesty
Deterministic coverage: divider hit-testing/claiming in both hosts, drag→frame application, small-pane clamp policy, and the pure geometry — all via cmuxTests. Not covered by automated tests: the live feel of a real drag against a real WKWebView-hosted inspector (WebKit's actual attachment relayout timing,
InspectorFrontendHostsync round-trip) — that needs manual dogfood: open a browser pane → open DevTools (attached, bottom and side dock) → drag the divider, including with the pane narrowed below ~240pt so the adaptive bottom dock engages.Localization audit
No user-facing strings were added or changed (divider/cursor interaction only); no
Localizable.xcstringschanges needed.Swift file budgets:
BrowserWindowPortal.swift3960/3981,BrowserPanelView.swift7924/7959 — both shrank; no TSV changes; new files are 260/34/158/98 lines.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches pointer routing, live NSView frame layout, and WebKit inspector attachment JS across portal and browser panel hosts; behavior is heavily unit-tested but real WKWebView drag feel still needs manual verification.
Overview
Attached Web Inspector resizing is reworked so cmux owns the divider in bottom dock as well as side dock, with clamping that still works in narrow panes and without fighting WebKit during drags.
Geometry and policy —
HostedInspectorDockSidemoves intoHostedInspectorResizeGeometry.swiftand gains.bottom, axis-generic divider math, andHostedInspectorMinimumSizePolicyso side/bottom docks clamp inspector vs page size without an empty drag range on tiny containers.Hosts (portal + local inline) — Divider hit-testing claims the seam (vertical or horizontal cursor), drags use extent (width or height) with stored fractions, a short-lived hit cache bridges
hitTest→mouseDown, and adaptive bottom-dock / frame repair are skipped while dragging.HostedInspectorDragFrameNotificationSilencersuppresses inspected-view frame notifications during drag; oneInspectorFrontendHost.setAttachedWindowWidth/Heightsync runs on mouseUp via newHostedInspectorAttachedSizeSync.Frontend —
HostedInspectorDockControlScriptdisables the in-page#docked-resizerso WebKit’s built-in handle doesn’t compete with cmux.Tests cover bottom-dock claiming/drag, geometry, dock-control script, and a widened side-dock panel test fixture.
Reviewed by Cursor Bugbot for commit 61aea05. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit