Repository navigation
Conversation
Adapt pingdotgg#8469 to current main. Preserve profiles and configured defaults, use CSS dimensions directly at fractional zoom, and bound guest readiness by the host response deadline. Co-authored-by: Guilherme Barros <gbarros1095@gmail.com> Co-authored-by: Theo Browne <me@t3.gg>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is a 25-file, 2,599-line change that substantially alters viewport application, automation deadlines, rollback, and cross-layer state ordering across desktop, web, and server paths. The unresolved findings also identify stale-control and mixed-version rollback risks that require human assessment. Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
Note This comment is posted by Julius' dot Closing for missing UI verification. This changes the visible preview's toolbar resizing and return to fill mode, but the PR explicitly omits before/after screenshots and a recording. The native probe checks guest dimensions without exercising that UI flow. Add before/after screenshots and a short recording showing fixed or preset resizing and return to fill at non-default zoom, then request reconsideration. |
|
@juliusmarminge The updated PR body contains seven before/after screenshots and a 53-second recording from the packaged Browser panel. The recording shows preset selection, freeform sizing and return to fill at host zoom level The follow-up branch is pushed at Could you reconsider and reopen this PR for review of the updated branch? |
Problem
Follow-up fixes are pushed to
fix/preview-viewport-upstreamat720a677b945b571c9681aba820be3fe9eb886a44. This closed PR still recordsc3dd8ae9, pending maintainer reconsideration and reopening.preview_resizecan time out because the stored viewport, rendered layout and native guest disagree. The toolbar has the same mismatch: at host zoom level0.5, the installed Nightly displays427x925after selecting the390x844iPhone preset, and460x658after entering420x600.The native guest now uses the selected viewport. Returning to fill clears the override. Failed or late requests preserve newer viewport state.
Closes #3712
Closes #12319
Closes #10525
Related: #13540 and #13610. The fix removes the resize-timeout trigger, without claiming to fix general host failover or the separate GPU-memory lifecycle problem.
UI verification
These captures exercise the real packaged Browser panel with a local HTTP page showing
innerWidthandinnerHeight. Both versions use separate temporary profiles, host zoom level0.5(about109.5%), and guest zoom100%. Before: Nightly99e08526e5ec. After:720a677b945b571c9681aba820be3fe9eb886a44.390x844420x600The additional capture and recording (53 seconds) use guest zoom
80%with the same non-default host zoom. The recording shows preset selection, freeform sizing and return to fill through the toolbar.toolbar-resize.mp4
Change
executeJavaScript. At the minimum supported guest zoom25%, integer DIP rounding can produce392x844for390x844. Fixed-mode readiness therefore allows at most2CSS pixels while still checking the exact setting and declared size, and returns the actual measurement. Fill keeps its1pixel tolerance.Based on Theo Browne's #8469 and Guilherme Barros's #7303, credited in the original commit. These links establish provenance, not approval of this branch.
Verification
On
720a677b945b571c9681aba820be3fe9eb886a44:277focused tests in12suites passed, along with scoped web/desktop typechecks, lint/format, the desktop build and local packaging. Independent follow-up review has no remaining actionable findings.Native Electron probes and raw measurements:
132records (12baselines and120checked post-operation measurements) cover actual HTTP navigation andabout:blank, host zoom levels0and0.5, guest factors1,0.8and1.25, resizing, clearing, zoom changes, navigation and reload. A separate low-zoom run records the rounding above. The earlier blank-only probe did not exercise effective HTTP-page zoom, so its results are limited to that document.The unchanged
PreviewAutomationHosts.test.tsxstatus test has5passes and1failure on the original PR headc3dd8ae9too: its empty mocked list clearsautomation-tab. Linux/Windows runtime checks and a full collaborative-browser pass on an installed patched build remain unverified. The running T3 installation was not modified. External review/CI approval is pending maintainer reconsideration of this closed PR.Made with GPT-6.1-Sol using the Codex harness in T3 Code.