Bug 221473

<rdar://problem/74278994>

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.

Created attachment 426208 [details] Patch

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()

Comment on attachment 426208 [details] Patch Moved to bug 224665

All `table-as-item` are passing on Safari Technology Preview 207 - https://wpt.fyi/results/css/css-flexbox?label=master&label=experimental&aligned&q=css-flexbox%2Ftable-as-item and this is also passing `table-item-flex-percentage-width.html` - https://wpt.fyi/results/css/css-flexbox/table-item-flex-percentage-width.html?label=master&label=experimental&aligned&q=%2Ftable-item-flex-percentage-width.html