Skip to content

fix(desktop): capture the whole oversized preview viewport - #36286

Open
shantanuojha wants to merge 3 commits into
pingdotgg:mainfrom
shantanuojha:fix/ultrawide-preview-capture
Open

shantanuojha wants to merge 3 commits into
pingdotgg:mainfrom
shantanuojha:fix/ultrawide-preview-capture

Conversation

@shantanuojha

@shantanuojha shantanuojha commented Nov 13, 2026 •

Copy link
Copy Markdown

An oversized background preview can report a 3440×1440 DOM viewport while its screenshot contains only the top-left of the page. Reproduced on public main 9efd960925: Electron 44.10.1 returned 1300×870 from a guest extending beyond a 1000×668 host window, missing the right and bottom corner sentinels.

Fit the background guest inside the host window with a CSS transform, preserving its logical viewport. Reuse the window-size subscription to refit on resize. Visible previews and suspended guests retain their existing placement; capture ownership, serialization, and image downscaling stay unchanged. This is desktop hosting geometry, including screenshots requested by remote clients or agents; headless web hosting and mobile rendering do not need changes.

Validation:

  • Current table regressions cover unscaled guests, width/height constrained fits, and the zero-sized-window guard added by the upstream pipeline.
  • Real Electron fixture: 390×844, 1440×900, 1920×1080, 2560×1440, 3440×1440, then 640×480. All four sentinel pixels pass after the fix at DPR 1 and 2. DPR 2 uses native page zoom on a DPR-1 display. The ultrawide PNGs are 3440×1440 and 6880×2880, respectively. DOM/layout metrics are read after resize settling, not copied from the request.
  • Running development desktop: the real HostedBrowserWebview and native screenshot IPC returned a complete 3440×1440 image. Background presentation/activity was driven through the browser surface store.
  • vp run check:changed passes. The existing ElectronBrowserHost, ServerBrowserPage, and ServerBrowser suites pass 130 tests; the current geometry suite passes 4 tests. check:changed was rerun successfully on dfcfd1a.

Latest-head proof (dfcfd1a versus original base 9efd960): the macOS Electron fixture also includes the requested 1440×1700 band capture. All 14 head captures across seven viewport sizes and DPR 1/2 retain all four corners and all four bands. Base reproduces clipping. Exact metrics and pixel checks · Reproduction bundle. This uses the actual base/head style resolver outputs in native Electron; it is separate from the original integrated desktop pass.

Limits: macOS only; no physical Retina display tested. Covered-window CDP capture can still time out, as tracked separately in #33719. This fixes successful-but-cropped captures, not that stall. Upstream's existing asynchronous desktop resize measurement remains intact. Explicit CDP clips were rejected as a solution because they produced repeated top-left pixels despite returning the requested dimensions.

Generic fixture evidence is hosted on the contributor fork, without committing screenshot assets:

Before:
Clipped ultrawide viewport

After:
Complete ultrawide viewport with four corner sentinels

Capture geometry and pixel verification · Generic HTML fixture

This is a small reproducible capture bug under CONTRIBUTING.md’s obvious-bug exception; no feature or architecture change is included.

Implemented and verified with GPT-6-Astra (ultra) through the Codex harness in T3 Code. The upstream pipeline added the zero-viewport guard in dfcfd1a, co-authored by Claude Opus 5.5.

@github-actions github-actions Bot added vouch:unvouched Author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 13, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 98ba0ce

Macroscope's review found this PR approvable — Oversized background desktop previews are scaled to fit the host window so captures include the whole viewport. Visible previews remain unchanged, and reported Electron capture checks support the limited scope of the correction.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between dfcfd1a and a493616.

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

No actionable comments were generated in the recent review.

Walkthrough

The hosted browser style resolver now accepts the window size and scales inactive rendering captures when needed to fit. The component supplies the current window size. Tests cover capture dimensions and scaling styles across window sizes, including a zero-sized window.

Changes: Hosted browser capture scaling

Layer / File(s) Summary
Capture sizing and scaling
apps/web/src/browser/hostedBrowserWebviewStyle.ts, apps/web/src/browser/hostedBrowserWebviewStyle.test.ts
The resolver accepts the window size and applies a top-left scale capped at 1 to inactive rendering captures. Tests check capture dimensions and scaling styles across window sizes, including a zero-sized window.
Window-size wiring
apps/web/src/browser/HostedBrowserWebview.tsx
The component reads the current window size with useViewportSize and passes it to the style resolver.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · previous review dfcfd

This change fits an oversized background preview into the host window so screenshots capture the whole viewport. Visible and suspended previews keep their existing placement. No actionable merge-blocking risk was found.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the fix (capturing the whole oversized preview viewport) and matches the main change. It uses the conventional commit form with a desktop scope. The change fits the background guest wi…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description supplies a clear problem statement, a concrete change, scope justification under the obvious-bug exception, and detailed verification with reproduction evidence, covering each template…
Full details: Title check

Explanation

The title names the fix (capturing the whole oversized preview viewport) and matches the main change. It uses the conventional commit form with a desktop scope. The change fits the background guest with a CSS transform so screenshots include the full viewport, which the title reflects.

Full details: Description check

Explanation

The description supplies a clear problem statement, a concrete change, scope justification under the obvious-bug exception, and detailed verification with reproduction evidence, covering each template section. It also names the model and harness used, as the template asks. Minor gaps: the Problem section mixes reproduction data into the first sentence, and the template asks for a conventional commit title, which the PR title (not shown in the description) is expected to supply. Verification relies on external links, and the description states that screenshot assets are hosted on a contributor fork rather than committed, which is compliant. Overall the description is mostly complete.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 98ba0ce and dfcfd1a.

Files selected for processing: 2

  • apps/web/src/browser/hostedBrowserWebviewStyle.test.ts
  • apps/web/src/browser/hostedBrowserWebviewStyle.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@juliusmarminge juliusmarminge added the takeover:in-progress A takeover lead is working on this PR label Nov 13, 2026
A zero viewport would scale the guest to nothing. Trim the geometry
matrix to a table of exact fits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juliusmarminge

Copy link
Copy Markdown
Member

The automated pipeline stopped on this pull request and will not merge it:

the fix is written and reviewed, but proving it needs something no worker has:

  • Permanent blocker: the reproduction requires the macOS Electron desktop app outside the mobile device pool. This VM runs Linux, and the brief prohibits reaching another machine or substituting for unavailable macOS conditions. Both the 1440×1700 band capture and the 3440×1440 corner-marker capture remain unverified on base and head.

It stays open for a person to finish or close.

@juliusmarminge juliusmarminge added takeover:blocked Takeover blocked; reason in the PR body and removed takeover:in-progress A takeover lead is working on this PR labels Nov 13, 2026
@shantanuojha

Copy link
Copy Markdown
Author

I reviewed the published agent feedback and the pipeline's dfcfd1a change. The zero-sized-window guard is valid and retained. CodeRabbit has no actionable findings, and there are no unresolved inline review threads. The PR description now reflects the pipeline's smaller four-case test table and credits its change.

The macOS proof requested in the pipeline stop comment is now available for current head dfcfd1a26622643714e00119a3d3122dadd96e15 against original base 9efd960925f0899c0a6d045d128557c5d82818e8. This was run locally on macOS with Electron 44.10.1 / Chromium 152.0.7977.169, using actual base/head style-resolver outputs in a native webview:

  • 1440×1700 band fixture: base returns 1300×870, missing the cyan/white bands and right/bottom corners. Head returns 1440×1700 with all four bands and corner sentinels.
  • 3440×1440 corner fixture: base returns 1300×870 with only the top-left sentinel. Head returns 3440×1440 with all four sentinels and bands.
  • Full matrix: 390×844 → 1440×900 → 1440×1700 → 1920×1080 → 2560×1440 → 3440×1440 → 640×480, at DPR 1 and 2. All 14 head captures pass PNG dimensions and decoded sentinel/band pixels. DPR 2 uses native Electron page zoom on a DPR-1 display, not physical Retina hardware.
  • vp run check:changed passes on this exact head. GitHub CI is green.

Raw metrics and pixel results · Runnable generic reproduction bundle

1440×1700 before · after

3440×1440 before · after

This is native Electron capture proof, separate from the original integrated development-client pass; it does not claim a new end-to-end MCP run or resolve the covered-window CDP stall in #33719. All assets are generic, and owned test windows/server were cleaned up. Please reassess the takeover:blocked proof blocker with this evidence. No further source changes were necessary.

@juliusmarminge juliusmarminge added takeover:in-progress A takeover lead is working on this PR and removed takeover:blocked Takeover blocked; reason in the PR body labels Nov 14, 2026
@juliusmarminge juliusmarminge added takeover:blocked Takeover blocked; reason in the PR body and removed takeover:in-progress A takeover lead is working on this PR labels Nov 14, 2026

This branch has not been deployed

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

Labels

size:S 10-29 changed lines (additions + deletions). takeover:blocked Takeover blocked; reason in the PR body vouch:unvouched Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants