Never emit bare text nodes as block/flex/grid layout children - #657
Open
nicoburns wants to merge 1 commit into
Open
Never emit bare text nodes as block/flex/grid layout children#657nicoburns wants to merge 1 commit into
nicoburns wants to merge 1 commit into
Conversation
Text nodes carry no style, so pushing one as a direct layout child of a block/flex/grid container panics when Taffy asks for the child's style. Two construction paths leaked bare text nodes: - Internal table displays (table-row, table-row-group, etc.) occurring outside a table fell into a catch-all arm that pushed text children raw. They now lay out as flow containers (no anonymous table wrapper generation yet), wrapping text/inline children as usual. - display:contents hoisting pushed hoisted text nodes raw into the ancestor container. Hoisted children are now wrapped in anonymous blocks per the nearest non-contents ancestor's wrapping policy. Also detect display:contents ::before/::after pseudos on flex/grid containers so their hoisted text content is wrapped as an item.
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 91 WPT crashes:
`style` is not available on this node kind(panic inuniversal_accessors!when Taffy asks for a text node's style during block/flex/grid layout). Box construction leaked bare text nodes as layout children of non-inline containers via three paths:Internal table displays outside a table (
display: table-row-groupetc. with no table ancestor, common in the CSS2 anonymous-table and generated-content tests): these fell intocollect_layout_children's catch-all arm, which pushed text children raw. They now fall into the flow-container arm (Blitz does not yet generate anonymous table wrapper boxes), so text/inline children get anonymous-block wrapping or an inline root as usual.display: contentshoisting:push_hoisted_children_and_pseudospushed hoisted children raw, so text inside a contents node became a bare layout child of the ancestor block/flex/grid container. Hoisting now threads aWrapContext { container_node_id, needs_wrap }describing the nearest non-contents ancestor's wrapping policy, and wraps hoisted children withpush_wrappedexactly as if they were direct children of that container:The all-out-of-flow path uses
text_item_needs_wrap(text only): inline elements were previously pushed raw there and are safe (they carry styles), and wrapping them regressedcss/css-images/cross-fade-natural-size.html.display:contentspseudo (.flex::before { display: contents; content: "A" }): thehas_text_node_or_contentscheck only looked at DOM children, so the pseudo was pushed raw. The check now includes::before/::after.Results (full
wpt cssrun)cargo test --workspace,cargo fmt,cargo clippy --workspacecleanLink to Devin session: https://dioxus.staging.devinenterprise.com/sessions/62b3de4395af4598a23a5918918f3239
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.