Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
yinnho
left a comment
There was a problem hiding this comment.
Verified this against Chrome while closing the same hole in another engine — the paint-half here matches, and the display:none/display:contents filter on the body candidate is the detail most implementations miss (WPT overflow-body-propagation-016: a body that generates no box propagates nothing, and a later body is not a fallback).
Two sibling behaviors from the same spec section that this PR doesn't cover yet, in case you want them in scope (or as a follow-up):
- The scrolling area collapses. When
hidden/cliparrives at the viewport, the page's scrolling area becomes exactly the viewport: Chrome reportsdocument.scrollingElement.scrollHeight === document.scrollingElement.clientHeightandwindow.scrollTo(0, 1000)leavesscrollY === 0on a 5000px-tall page. If the scroll-extent computation doesn't consult the propagated overflow,body { overflow: hidden }stops clipping (with this PR) but the page stays scroll-programmable — half-correct in the other direction. - The body's own extent survives. Although the body's used overflow flips back to
visible, Chrome keepsdocument.body.scrollHeightat the full content extent (e.g. 5000) — useful as a test oracle to distinguish "propagated" from "clipped locally".
overflow: scroll/auto propagated to the viewport must of course keep the viewport scrollable — only hidden/clip pin the range at zero.
… §3.3 (blitz#880)
UAF-style divergence: body{overflow:hidden} never reached the viewport —
the viewport stayed scrollable and body clipped its own descendants.
Now the first box-generating body child propagates its overflow to the
viewport when html is visible, and body's used value flips back to
visible (must not clip, scrollHeight keeps content extent).
- diting_layout: shared effective_viewport_overflow/body_overflow_propagates
helpers; paint collect exempts html + propagating body from clipping
- diting_js: scroll_extent + band_frame collapse the scrolling area to the
viewport for hidden/clip at the propagated viewport overflow
- bootstrap: root scroller scrollTop/scrollLeft setters clamp to the real
scroll range (scrollTo on a collapsed viewport pins at 0, like Chrome)
- video: camera-scroll test pins the output viewport before navigate so
the scroll range exists
Refs DioxusLabs/blitz#880 (paint half landed there; this carries the
scrolling-area collapse + body extent survival halves too).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Split out of #877.
The painter exempted only the root element from overflow clipping. Per css-overflow-3 §3.3, when
<html>hasoverflow: visiblein both axes, the overflow of the first<body>child that generates a box is propagated to the viewport instead, and the body's own used overflow becomesvisible— sobody { overflow: hidden }must not clip the body's content (css/CSS2/visufx/overflow-propagation-001a/b/c).BaseDocument::viewport_overflow_element()returns the element whose overflow is propagated: the root, unless it's an<html>with visible overflow and its first<body>child has a box (adisplay: nonebody propagates nothing →overflow-body-propagation-016).BlitzDomPaintercaches it asviewport_overflow_element_id; inrender_elementthat element skips clipping for overflow reasons only:contain: paint/ image / iframe / text-input clipping; now those still apply (overflow-body-propagation-010).Scrolling already followed these rules (
scrolling.rs); this brings paint clipping in line.WPT (local)
css/CSS2/visufx: +3 (overflow-propagation-001*)css/css-overflow: 227 → 232 (overflow-body-propagation-007/008/009/014/015), no regressionscss/css-position, flexbox, grid, text: unchangedLink to Devin session: https://dioxus.staging.devinenterprise.com/sessions/355437a9e8fd4bebbf536cf039421df2
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/355437a9e8fd4bebbf536cf039421df2?variant=devin-insiders
Requested by: @nicoburns
WPT results
Subtests: 16 newly passing, 12 newly failing (net +4).
Full diff (28 changed tests)
Generated by the WPT workflow.