Bug 205386 – [css-flex][css-grid] Out-of-flow boxes should not be children of anonymous items

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Manuel Rego Casasnovas]

This is causing that WebKit fails the following WPT tests
(they pass in Chromium and Firefox):
* http://w3c-test.org/css/css-flexbox/anonymous-flex-item-004.html
* http://w3c-test.org/css/css-grid/grid-items/anonymous-grid-item-001.html

This was already fixed in Chromium:
https://chromium-review.googlesource.com/c/chromium/src/+/895844/
But the code in WebKit regarding tree construction is different now,
so it cannot just be ported directly into WebKit.

There's also a related test that only passes in Firefox right now:
http://w3c-test.org/css/css-flexbox/anonymous-flex-item-005.html

[Oriol Brufau]

Created attachment 399898 [details]
Patch

[Oriol Brufau]

https://crrev.com/533825 has another change in LayoutBlock::AddChildBeforeDescendant which amends https://codereview.chromium.org/120603002

I'm not including that since WebKit doesn't have the earlier change (maybe it should, but that's not relevant to this bug).

[Manuel Rego Casasnovas]

Comment on attachment 399898 [details]
Patch

r=me

[alan]

Do you mind holding off of cq+? I wanted to take a look at it.

[alan]

(In reply to zalan from comment #4)
> Do you mind holding off of cq+? I wanted to take a look at it.
I'll cq+ it for you.

[Oriol Brufau]

Sure, there's no hurry

[alan]

(In reply to Oriol Brufau from comment #6)
> Sure, there's no hurry
Cool. Thanks!

[alan]

Oh I see. A flex container's (which establishes the flex formatting context) in flow children are all flex items and since they establish BFCs they all need to be blockified one way or another.
Thanks to the setChildrenInline(false) hack in the flex container's c'tor, the first inline level box gets forced into an anonymous block level container and we start using this anon block as parent for the sibling inline level boxes, while the subsequent block level boxes are added as siblings. Now since an out-of-flow box does not trigger tree normalization, meaning that we can have an out-of-flow block box next to an inline level box, it gets pushed inside the anon container and the subsequent inline level boxes are added as siblings. At the end we get a tree like this:
flex container
  anonymous block
    anonymous inline level box ([first])
    out-of-flow block level box
    anonymous inline level box ([text])
and since the anonymous block (as a flex item) establishes a BFC, the content is laid out like this "[fist][second]".

But instead what we want is
flex container
  anonymous block 
    anonymous inline level box ([first])
  out-of-flow block level box
  anonymous block 
    anonymous inline level box ([text])
to properly layout the content like this:
[first]
[second]

I think the patch is fine for now, but in a long run (probably in next-gen timeframe) the flex container should have its own builder to be able to properly set up/blockify its direct children. This is a very simple problem and we should not need to implement hacks like this (or that setChildrenInline(false)).

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 399898 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Oriol Brufau]

Created attachment 400005 [details]
Patch

[Oriol Brufau]

Created attachment 400044 [details]
Patch

[EWS]

Committed r262061: <https://trac.webkit.org/changeset/262061>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400044 [details].

[Radar WebKit Bug Importer]

<rdar://problem/63543311>