[css-flexbox] Tables as flex items issues

• Component: Layout and Rendering
• Status: NEW ---

[Sergio Villar Senin]

Several failures in newly imported WPT tests:

imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-fixed-min-width-3.html
imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-flex-cross-size.html
imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-percent-width-cell-001.html
imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-specified-width.html
imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-stretch-cross-size.html
imported/w3c/web-platform-tests/css/css-flexbox/table-item-flex-percentage-width.html

[Radar WebKit Bug Importer]

<rdar://problem/74278994>

[Felipe Erias]

A few notes about the failing tests:

— table-as-item-fixed-min-width-3

This test creates a flex layout with a table and a div. The table claims to have a min-width of 50px, but after layout it becomes wider as its only cell contains a div with a width of 100px.
  
The flex algorithm uses the min-width of the table to allocate the available space. After layout, the table takes up more space than expected, causing the second flex child to overflow the container.

— table-as-item-flex-cross-size

The flex container (row direction) sets an overriding logical height on its table child (cross axis from the point of view of the container). However, the table does not actually grow because it ignores that overriding value.

— table-as-item-percent-width-cell-001

The table ignores the overriding logical width placed by its flex container and instead takes all the available space and gives it to its first cell (which has width:100%).

Furthermore, the different values of flex-basis (content, min-content, max-content) are ignored, so the outcome is the same in all cases.

— table-as-item-specified-width

The element has flex-basis:100px with flex-grow and flex-shrink both set to 0. This means that its width should stay at 100px and don't change.

However, RenderTable::updateLogicalWidth() erroneously discards the 100px overriding width set by the flex container and uses the width value from the style instead (500px).

— table-as-item-stretch-cross-size

The flex container (column direction) sets an overriding logical height on its table child (main axis for the container). Like before, the table does not actually grow because it ignores that overriding value.

— table-item-flex-percentage-width

This test creates a series of flex containers with display:table children.

Several of those do not have the expected outcome, mainly because the content's width (set as a percentage) takes precedence over its flex-basis and does not change as a result of flex-shrink or flex-grow.

[Felipe Erias]

Created attachment 426208 [details]
Patch

[Sergio Villar Senin]

Comment on attachment 426208 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426208&action=review

Thanks for the patch!

Every patch to WebKit needs a test. If this is covered by WPT tests then you need to remove them from TestExpectations file.

> Source/WebCore/rendering/RenderTable.cpp:513
> +            for (unsigned i = 0; i < m_captions.size(); ++i) {

You can write this like

for (auto& caption : m_captions)

> Source/WebCore/rendering/RenderTable.cpp:517
> +            LayoutUnit contentOverridingLogicalHeight = overridingLogicalHeight() - captionLogicalHeight;

Not sure why you label this as contentOverridingLogicalHeight. overridingLogicalHeight() refers to the border box, meaning that it includes border and padding so if you really want the content height you have to use overridingContentLogicalHeight()

[Sergio Villar Senin]

Comment on attachment 426208 [details]
Patch

Moved to bug 224665