Don't let display:none children force an inline formatting context - #656
Open
nicoburns wants to merge 1 commit into
Open
Don't let display:none children force an inline formatting context#656nicoburns wants to merge 1 commit into
nicoburns wants to merge 1 commit 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.
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 a panic (
Option::unwrap()onNoneatresolve.rs:375) hit by 6 WPT tests where the root element isdisplay: none(e.g.css/CSS2/box-display/root-box-003.xht).classify_flow_childrencounteddisplay: nonechildren as in-flow inline content (they clearedall_out_of_flowwhile leavingall_inlineset). A container whose only non-whitespace children aredisplay: none— such as the Document node when<html>isdisplay: none— was therefore classified as all-inline and queued as an inline-layout construction task. The deferred-task loop then calledelement_data_mut().unwrap()on the Document node, which has noElementData, and panicked.Fix: skip
display: nonechildren in the classification (they generate no boxes, so they cast no vote). Such containers now take theall_out_of_flowpath and their children are pushed as normal layout children, where Taffy already handlesDisplay::None.WPT: the 6 crashing tests now run (4 PASS, 2 FAIL for unrelated reasons); full
cssrun shows no previously-passing test regressed and no new crashes.Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/dde09f37a79d43a2ac7993911b21d308
Requested by: @nicoburns
WPT results
9 newly passing, 0 newly failing (net +9), 2 other status changes.
Full diff (11 changed tests)
Generated by the WPT workflow.