Title: [286593] trunk
- Revision
- 286593
- Author
- [email protected]
- Date
- 2021-12-07 02:20:24 -0800 (Tue, 07 Dec 2021)
Log Message
[css-flexbox] Account for captions when flexing tables with specified sizes
https://bugs.webkit.org/show_bug.cgi?id=233814
Reviewed by Darin Adler.
Source/WebCore:
Flexing tables is complex because of the specifics of the table layout algorithms. There are
two interrelated issues that were causing some failures in flexbox WPT tests.
The first one is that tables interpret overriding sizes (flexed sizes) as the sizes of rows + captions.
However the specified height of a table only accounts for the heights of the rows, not the captions. That's
why when setting the flexed height of a table we must add up the specified height of the table and the height
of the captions. The table algorithm will properly substract the captions' size in order to compute the
available height for rows.
The second issue is that the table layout algorithm adds the size of the bottom border at the very end
of its execution after performing all the computations and it does it unconditionally. The thing is that
the flexbox code already takes into account the borders and paddings so we basically have to substract that
border when setting the height of the table, because the table layout will add it later.
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computeMainAxisExtentForChild):
* rendering/RenderTable.cpp:
(WebCore::RenderTable::computeCaptionsLogicalHeight const): New method refactored from current code.
(WebCore::RenderTable::layout):
* rendering/RenderTable.h: Expose computeCaptionsLogicalHeight.
LayoutTests:
* TestExpectations: Unskipped two tests that are now passing
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (286592 => 286593)
--- trunk/LayoutTests/ChangeLog 2021-12-07 10:01:22 UTC (rev 286592)
+++ trunk/LayoutTests/ChangeLog 2021-12-07 10:20:24 UTC (rev 286593)
@@ -1,3 +1,12 @@
+2021-12-03 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] Account for captions when flexing tables with specified sizes
+ https://bugs.webkit.org/show_bug.cgi?id=233814
+
+ Reviewed by Darin Adler.
+
+ * TestExpectations: Unskipped two tests that are now passing
+
2021-12-07 Martin Robinson <[email protected]>
perspective() <= 1px should be clamped to 1px
Modified: trunk/LayoutTests/TestExpectations (286592 => 286593)
--- trunk/LayoutTests/TestExpectations 2021-12-07 10:01:22 UTC (rev 286592)
+++ trunk/LayoutTests/TestExpectations 2021-12-07 10:20:24 UTC (rev 286593)
@@ -4224,8 +4224,6 @@
webkit.org/b/221472 imported/w3c/web-platform-tests/css/css-flexbox/abspos/flex-abspos-staticpos-margin-002.html [ ImageOnlyFailure ]
# Tables as flex items.
-webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-1.html [ ImageOnlyFailure ]
-webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-2.html [ ImageOnlyFailure ]
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-min-height-1.html [ ImageOnlyFailure ]
# SVGs as flex items.
Modified: trunk/Source/WebCore/ChangeLog (286592 => 286593)
--- trunk/Source/WebCore/ChangeLog 2021-12-07 10:01:22 UTC (rev 286592)
+++ trunk/Source/WebCore/ChangeLog 2021-12-07 10:20:24 UTC (rev 286593)
@@ -1,3 +1,31 @@
+2021-12-03 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] Account for captions when flexing tables with specified sizes
+ https://bugs.webkit.org/show_bug.cgi?id=233814
+
+ Reviewed by Darin Adler.
+
+ Flexing tables is complex because of the specifics of the table layout algorithms. There are
+ two interrelated issues that were causing some failures in flexbox WPT tests.
+
+ The first one is that tables interpret overriding sizes (flexed sizes) as the sizes of rows + captions.
+ However the specified height of a table only accounts for the heights of the rows, not the captions. That's
+ why when setting the flexed height of a table we must add up the specified height of the table and the height
+ of the captions. The table algorithm will properly substract the captions' size in order to compute the
+ available height for rows.
+
+ The second issue is that the table layout algorithm adds the size of the bottom border at the very end
+ of its execution after performing all the computations and it does it unconditionally. The thing is that
+ the flexbox code already takes into account the borders and paddings so we basically have to substract that
+ border when setting the height of the table, because the table layout will add it later.
+
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::computeMainAxisExtentForChild):
+ * rendering/RenderTable.cpp:
+ (WebCore::RenderTable::computeCaptionsLogicalHeight const): New method refactored from current code.
+ (WebCore::RenderTable::layout):
+ * rendering/RenderTable.h: Expose computeCaptionsLogicalHeight.
+
2021-12-07 Martin Robinson <[email protected]>
perspective() <= 1px should be clamped to 1px
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (286592 => 286593)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2021-12-07 10:01:22 UTC (rev 286592)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2021-12-07 10:20:24 UTC (rev 286593)
@@ -40,6 +40,7 @@
#include "RenderObjectEnums.h"
#include "RenderReplaced.h"
#include "RenderStyleConstants.h"
+#include "RenderTable.h"
#include "RenderView.h"
#include "WritingMode.h"
#include <limits>
@@ -652,7 +653,13 @@
std::optional<LayoutUnit> height = child.computeContentLogicalHeight(sizeType, size, cachedChildIntrinsicContentLogicalHeight(child));
if (!height)
return height;
- return height.value() + child.scrollbarLogicalHeight();
+ // Tables interpret overriding sizes as the size of captions + rows. However the specified height of a table
+ // only includes the size of the rows. That's why we need to add the size of the captions here so that the table
+ // layout algorithm behaves appropiately.
+ LayoutUnit captionsHeight;
+ if (is<RenderTable>(child) && childMainSizeIsDefinite(child, size))
+ captionsHeight = downcast<RenderTable>(child).sumCaptionsLogicalHeight();
+ return *height + child.scrollbarLogicalHeight() + captionsHeight;
}
// computeLogicalWidth always re-computes the intrinsic widths. However, when
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (286592 => 286593)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2021-12-07 10:01:22 UTC (rev 286592)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2021-12-07 10:20:24 UTC (rev 286593)
@@ -419,6 +419,14 @@
}
}
+LayoutUnit RenderTable::sumCaptionsLogicalHeight() const
+{
+ LayoutUnit height;
+ for (auto& caption : m_captions)
+ height += caption->logicalHeight() + caption->marginBefore() + caption->marginAfter();
+ return height;
+}
+
void RenderTable::layout()
{
StackStats::LayoutCheckPoint layoutCheckPoint;
@@ -505,12 +513,8 @@
if (logicalHeightLength.isIntrinsic() || (logicalHeightLength.isSpecified() && logicalHeightLength.isPositive()))
computedLogicalHeight = convertStyleLogicalHeightToComputedHeight(logicalHeightLength);
- if (hasOverridingLogicalHeight()) {
- LayoutUnit captionLogicalHeight;
- for (auto& caption : m_captions)
- captionLogicalHeight += caption->logicalHeight() + caption->marginBefore() + caption->marginAfter();
- computedLogicalHeight = std::max(computedLogicalHeight, overridingLogicalHeight() - captionLogicalHeight);
- }
+ if (hasOverridingLogicalHeight())
+ computedLogicalHeight = std::max(computedLogicalHeight, overridingLogicalHeight() - borderAndPaddingAfter - sumCaptionsLogicalHeight());
Length logicalMaxHeightLength = style().logicalMaxHeight();
if (logicalMaxHeightLength.isIntrinsic() || (logicalMaxHeightLength.isSpecified() && !logicalMaxHeightLength.isNegative())) {
@@ -533,7 +537,7 @@
// Completely empty tables (with no sections or anything) should at least honor their
// overriding or specified height in strict mode, but this value will not be cached.
shouldCacheIntrinsicContentLogicalHeightForFlexItem = false;
- setLogicalHeight(hasOverridingLogicalHeight() ? overridingLogicalHeight() : logicalHeight() + computedLogicalHeight);
+ setLogicalHeight(hasOverridingLogicalHeight() ? overridingLogicalHeight() - borderAndPaddingAfter : logicalHeight() + computedLogicalHeight);
}
LayoutUnit sectionLogicalLeft = style().isLeftToRightDirection() ? borderStart() : borderEnd();
Modified: trunk/Source/WebCore/rendering/RenderTable.h (286592 => 286593)
--- trunk/Source/WebCore/rendering/RenderTable.h 2021-12-07 10:01:22 UTC (rev 286592)
+++ trunk/Source/WebCore/rendering/RenderTable.h 2021-12-07 10:20:24 UTC (rev 286593)
@@ -271,6 +271,8 @@
void willInsertTableColumn(RenderTableCol& child, RenderObject* beforeChild);
void willInsertTableSection(RenderTableSection& child, RenderObject* beforeChild);
+ LayoutUnit sumCaptionsLogicalHeight() const;
+
protected:
void styleDidChange(StyleDifference, const RenderStyle* oldStyle) final;
void simplifiedNormalFlowLayout() final;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
