fix(engine): propagate viewport changes to the live JS context - #133
Open
danperks wants to merge 1 commit into
Open
fix(engine): propagate viewport changes to the live JS context#133danperks wants to merge 1 commit into
danperks wants to merge 1 commit into
Conversation
`set_viewport` updated the process-global metrics that seed a *future* JS context but never touched one that already existed, so a loaded page kept the size it started at forever: `innerWidth`, `innerHeight` and `devicePixelRatio` stayed frozen, width-based media queries never re-evaluated, and no `resize` event was ever dispatched. Any page doing its own responsive work in JS was stuck at its load-time layout. `__viewportChanged` is added to the browser environment — it refreshes the globals, re-evaluates live `MediaQueryList`s and dispatches `resize` — with `Session::notify_viewport_changed` to drive it, mirroring the existing `notify_color_scheme_changed` path. Layout is re-run and the rect table re-pushed *before* dispatch. A resize handler exists in order to measure, and `getBoundingClientRect` reads the pushed rect table rather than the globals, so dispatching first let handlers observe the pre-resize geometry and cache the wrong values. The cache is dropped again afterwards so DOM mutations made by handlers are laid out on the next render. Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
What & why
set_viewportupdated the process-global metrics that seed a future JS context, but never toucheda context that already existed. A loaded page therefore kept the size it started at forever:
window.innerWidth/innerHeight/devicePixelRatiostayed frozen at load-time valuesmatchMedia(...).matcheswent staleresizeevent was ever dispatchedAnything doing its own responsive work in JS was pinned to its load-time layout. I hit this building
a GPUI frontend against the engine: resizing the window resized the framebuffer, but the page inside
it kept laying out for the old width.
__viewportChangedis added to the browser environment — it refreshes the globals, re-evaluateslive
MediaQueryLists and dispatchesresize— withSession::notify_viewport_changedto drive it.This deliberately mirrors the existing
notify_color_scheme_changedpath rather than inventing asecond mechanism.
Ordering: re-layout happens before dispatch
The part worth reviewing closely. A
resizehandler exists in order to measure, andgetBoundingClientRectreads the pushed rect table, not the globals set above. Dispatching firsttherefore let handlers observe pre-resize geometry and cache the wrong values — a stale layout is
arguably worse than no event, because the page acts on it confidently.
So
set_viewportnow drops the layout cache, re-runsensure_layout, and re-pushes the rect tablebefore dispatching. The cache is dropped once more afterwards, so DOM mutations made by handlers
are laid out on the next render (the same reasoning as
set_color_scheme).The regression test pins this ordering specifically. It asserts on a percentage-width div measured
inside the handler, because the root's own
clientWidthfalls back toinnerWidthand so wouldpass even if layout had never re-run. Verified in both directions — with the re-layout removed the
handler sees
500(the pre-resize width) instead of320.Noted but deliberately not fixed here
clientWidth/clientHeightare never populated for non-root elements:set_layout_rectsfeedsgetBoundingClientRectbut not the__elemMetricspadding-box fields, so a non-root element reads0. That is a pre-existing gap unrelated to resize, and folding it in would have muddled thischange — happy to open a separate issue or PR for it if useful.
🤖 How this was built
Checklist
cargo test --workspacepasses — addsresize_relayouts_before_handlers_observe_geometry, which also covers that an unchangedviewport does not re-enter JS at all
cargo fmt --all+cargo clippyclean for this change (clippy reports some pre-existingwarnings in
wurlandnet, untouched here)a fixed viewport; covered by the unit test above instead
Made with Cursor