Fix bare text nodes becoming layout children through display:contents - #658
Open
nicoburns wants to merge 2 commits into
Open
Fix bare text nodes becoming layout children through display:contents#658nicoburns wants to merge 2 commits into
nicoburns wants to merge 2 commits into
Conversation
display:none children generate no boxes, but classify_flow_children was counting them as in-flow inline content. A container whose only children are display:none (e.g. an <html> root with display:none) was classified as all-inline and queued for inline layout construction -- panicking when that container was the Document node, which has no element data.
display:contents is transparent for box generation, but several construction paths were pushing the text children of contents nodes (or the contents node itself) directly as layout children. Taffy then walked into a bare text node during round_layout and panicked with "unrounded_layout is not available on this node kind". - collect_complex_layout_children now recurses transparently through display:contents children itself, so hoisted text goes through the same anonymous-block wrapping as any other text child (e.g. in flex containers). Replaced elements and form controls keep their special handling since display:contents does not apply to them. - Flow classification now also classifies the children of display:contents ::before/::after pseudos, so generated text makes the container an inline root instead of leaving the contents pseudo to be pushed as a box. - The flex/grid text-or-contents check now includes ::before/::after pseudos. - push_hoisted_children_and_pseudos now applies the display:contents check to the pseudos as well as regular children. - Table-internal boxes laid out outside a table context (e.g. a standalone display:table-row) now use the complex path, wrapping text and recursing through contents instead of pushing them raw.
Contributor
🤖 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:
|
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
1 participant
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.
Summary
Fixes the
`unrounded_layout` is not available on this node kindpanic (node/node.rs:160) hit by 3css/css-display/display-contents-*WPT tests. Root cause: several box-construction paths pushed the text children ofdisplay: contentsnodes (or a contents node itself) directly as layout children, so Taffy'sround_layoutwalked into a bareTextnode, which has no layout fields.Changes in
construct.rs:collect_complex_layout_childrennow recurses transparently throughdisplay: contentschildren itself (newvisit_childhelper) instead of delegating tocollect_layout_children, so hoisted text goes through the sameneeds_wrapanonymous-block wrapping as any other text child (e.g. in flex containers). Replaced elements and form controls (input/textarea) keep their special handling, sincedisplay: contentsdoes not apply to them (contents are suppressed, not hoisted).display: contents::before/::afterpseudos, so generated text makes the container an inline root instead of leaving the contents pseudo to be pushed as a box whose text child leaks out unwrapped.has_text_node_or_contentscheck now includes::before/::afterpseudos.push_hoisted_children_and_pseudosnow applies the contents check to pseudos as well as regular children.display: table-row) now use the complex path, wrapping text and recursing through contents instead of pushing raw text/contents nodes.WPT: the 3 crashing tests now run without panicking (they FAIL for unrelated reasons); together with #656 the full
cssrun goes from 244 to 147 crashes (many otherfirst-letter/display:contentscrashes share this root cause), +29 passing, no previously-passing test regressed, no new crashes.Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/dde09f37a79d43a2ac7993911b21d308
Requested by: @nicoburns
WPT results
20 newly passing, 0 newly failing (net +20), 71 other status changes.
Full diff (91 changed tests)
Generated by the WPT workflow.