Repository navigation
feat: support resizing WKWebView browser viewports - #8072
Conversation
|
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:
📝 WalkthroughWalkthroughAdds exact logical viewport emulation for WKWebView browser surfaces, including native reset, aspect-fit layout, CLI/API handling, hosting integration, screenshot support, documentation, and end-to-end tests. ChangesWKWebView viewport emulation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant TerminalController
participant BrowserPanel
participant WKWebView
CLI->>TerminalController: Set or reset browser viewport
TerminalController->>BrowserPanel: Apply requested viewport
BrowserPanel->>WKWebView: Compute and apply layout
WKWebView-->>BrowserPanel: Return applied layout
BrowserPanel-->>TerminalController: Return mode and dimensions
TerminalController-->>CLI: Output viewport result
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches🧪 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 adds logical viewport resizing for WKWebView browser surfaces. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (26): Last reviewed commit: "fix: keep viewport limit errors browser-..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests_v2/test_browser_cli_agent_port.py`:
- Around line 320-328: The screenshot tests around the initial and zoomed
viewport paths create temporary directories with tempfile.mkdtemp() without
cleanup. Replace both temporary-directory allocations with
tempfile.TemporaryDirectory() context-managed usage, and construct each
screenshot path from the managed directory so cleanup occurs automatically after
the test.
🪄 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: 48af6c2b-0e69-49bc-9e75-9d27b7026b13
📒 Files selected for processing (16)
CLI/CMUXCLI+BrowserViewport.swiftCLI/cmux.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserViewportLayout.swiftPackages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserViewportLayoutTests.swiftResources/Localizable.xcstringsSources/Panels/BrowserPanel+AutomationViewport.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserScreenshotSnapshotter.swiftSources/Panels/CmuxWebView.swiftSources/Panels/WKWebView+BrowserViewport.swiftSources/TerminalController+BrowserViewport.swiftcmux.xcodeproj/project.pbxprojdocs/cli-contract.mdskills/cmux-browser/SKILL.mdskills/cmux-browser/references/commands.mdtests_v2/test_browser_cli_agent_port.py
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
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 (1)
Sources/Panels/BrowserPanel+AutomationViewport.swift (1)
18-24: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep viewport geometry on one authoritative source of truth.
These paths guess dimensions from WebView/window geometry or hardcoded defaults when reliable layout state is unavailable. That can apply the wrong layout and misreport screenshot/API viewport dimensions.
Sources/Panels/BrowserPanel+AutomationViewport.swift#L18-L24: defer layout until a real host container exists, or fail closed.Sources/Panels/BrowserPanel+AutomationViewport.swift#L40-L55: derive the visual size from the viewport/layout model, not window bounds or1280x720.Sources/Panels/BrowserPanel+AutomationViewport.swift#L58-L63: remove the arbitrary800x600container fallback.🤖 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`+AutomationViewport.swift around lines 18 - 24, Keep viewport geometry authoritative across Sources/Panels/BrowserPanel+AutomationViewport.swift:18-24, deferring layout until a real host container exists or failing closed; at Sources/Panels/BrowserPanel+AutomationViewport.swift:40-55, derive visual dimensions from the viewport/layout model instead of window bounds or 1280x720; and at Sources/Panels/BrowserPanel+AutomationViewport.swift:58-63, remove the arbitrary 800x600 fallback. Update the affected automation viewport layout and sizing flow without introducing alternate geometry sources.Source: Path instructions
🤖 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`+AutomationViewport.swift:
- Around line 18-24: Keep viewport geometry authoritative across
Sources/Panels/BrowserPanel+AutomationViewport.swift:18-24, deferring layout
until a real host container exists or failing closed; at
Sources/Panels/BrowserPanel+AutomationViewport.swift:40-55, derive visual
dimensions from the viewport/layout model instead of window bounds or 1280x720;
and at Sources/Panels/BrowserPanel+AutomationViewport.swift:58-63, remove the
arbitrary 800x600 fallback. Update the affected automation viewport layout and
sizing flow without introducing alternate geometry sources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 975f4c97-95e9-4d97-bf58-50402af60efc
📒 Files selected for processing (2)
Sources/Panels/BrowserPanel+AutomationViewport.swifttests_v2/test_browser_cli_agent_port.py
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
…viewport-resize # Conflicts: # cmux.xcodeproj/project.pbxproj
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
* test(browser): cover WKWebView viewport emulation * feat(browser): emulate WKWebView viewports * docs(browser): explain WKWebView viewport sizing * fix(browser): preserve viewport across zoom and capture * fix(browser): close viewport review findings * test(browser): preserve reconciled host after capture * fix(browser): preserve current host after capture * test(browser): clean up viewport screenshots * fix(browser): import viewport geometry types * test(browser): cover viewport scale boundaries * fix(browser): bound and normalize viewport rendering * test(browser): cover viewport safety policies * fix(browser): bound viewport capture and inspector transitions * test(browser): cover inspector and zoomed full-page transitions * fix(browser): reconcile inspector and snapshot coordinates * test(browser): cover fullscreen and native zoom capture * fix(browser): reconcile fullscreen and native zoom capture * test(browser): assert runtime viewport dimensions * test(browser): gate runtime viewport regression * test(browser): await runtime viewport metrics * test: cover native viewport rounding at page zoom * fix: match native viewport rounding to WebKit * fix: preserve browser viewport presentation geometry * test: cover emulated viewport autoresizing mask * fix: disable raw viewport autoresizing while emulated * test: cover emulated viewport precision at page zoom * fix: preserve emulated viewport precision at page zoom * test: cover emulated viewport precision at default zoom * fix: preserve emulated viewport precision at default zoom * test: reject stale detached browser host restoration * fix: leave detached browser hosts unowned after capture * refactor: split browser viewport support types * test: report browser zoom render limits * fix: report browser zoom render limits * docs: keep viewport mode docs on public symbol * test: keep native browsers outside viewport host * fix: activate browser viewport host lazily * test: keep native portal layout passive * fix: keep native browser portal layout passive * test: preserve fractional zoom viewport dimensions * fix: preserve exact viewport dimensions at page zoom * test: cover near-integer zoom viewport rounding * ci: run CmuxBrowser package tests * fix: preserve near-integer zoom viewport dimensions * fix: keep viewport limit errors browser-generic (cherry picked from commit 08c9f5d)
Summary
cmux browser viewport resetplus structured mode, logical/display size, scale, presentation, exactness, andpane_resized: falseresultsinvalid_stateguidance when an attached browser inspector owns the split layoutReview fixes
Validation
arch -arm64 swift test --package-path Packages/macOS/CmuxBrowser --filter BrowserViewport- 17 tests passedaf32e630f5fails locally because the bounded tile plan and attached-inspector reset are absent; fix commit854d59506cpasses. GitHub Actions is disabled for this repository, so the red commit could not be recorded in CI.CmuxBrowserpackage suite - 146 tests passed earlier in this branchxcrun swiftc -frontend -parsepython3 -m py_compile tests_v2/test_browser_cli_agent_port.py tests_v2/test_browser_api_unsupported_matrix.pypython3 scripts/swift_file_length_budget.py --base-ref origin/main- blocked by the currentorigin/mainbaseline:Sources/CmuxEventBus.swiftis 619 lines and has no budget entry; this PR does not modify that file or either budget TSV./scripts/check-pbxproj.sh./scripts/lint-pbxproj-test-wiring.shpython3 scripts/check-workspace-package-groups.py --checkpython3 scripts/check-package-resolved-policy.pygit diff --checkNeither
.github/swift-file-length-budget.tsvnor.github/swift-warning-budget.tsvis changed.Fixes #8056
Summary by CodeRabbit
browser viewport <width> <height>plusbrowser viewport reset.