Skip to content

Commit

Permalink
Add support for align-content on table cells.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=266453
rdar://119701629

Reviewed by Sammy Gill.

Updates vertical-align conditionals in table rendering
to first check align-content, and map behavior accordingly.

* LayoutTests/imported/w3c/web-platform-tests/css/css-align/blocks/align-content-table-cell-expected.txt: Update to pass
* LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/css-align/blocks/align-content-table-cell-expected.txt: Removed.
* LayoutTests/platform/wpe/imported/w3c/web-platform-tests/css/css-align/blocks/align-content-table-cell-expected.txt: Removed.
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::layoutBlock): Skip alignment if we're a table cell
* Source/WebCore/rendering/RenderTableCell.cpp:
(WebCore::RenderTableCell::computeIntrinsicPadding): Map align-content values to corresponding vertical-align behavior.
(WebCore::RenderTableCell::styleDidChange): Handle align-content style changes same as vertical-align.
(WebCore::RenderTableCell::scrollbarsChanged): Handle align-content centering same as vertical-align.
* Source/WebCore/rendering/RenderTableCell.h:
(WebCore::RenderTableCell::isBaselineAligned const): Update to check align-content first.

Canonical link: https://commits.webkit.org/272373@main
  • Loading branch information
fantasai authored and sammygill committed Dec 20, 2023
1 parent fb97bd0 commit c822a6a
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ first
second
third

FAIL vertical-align:top and align-content:start are equivalent assert_equals: The first cell content top expected 0 but got 15
PASS vertical-align:top and align-content:start are equivalent
PASS vertical-align:middle and `align-content:unsafe center` are equivalent
FAIL vertical-align:bottom and `align-content:unsafe end` are equivalent assert_equals: The first cell content top expected 30 but got 15
FAIL vertical-align:baseline and align-content:baseline are equivalent assert_equals: The first cell content top expected 8 but got 15
PASS vertical-align:bottom and `align-content:unsafe end` are equivalent
PASS vertical-align:baseline and align-content:baseline are equivalent
PASS vertical-align:middle and `align-content:safe center` are equivalent if the container is tall
FAIL vertical-align:bottom and `align-content:safe end` are equivalent if the container is tall assert_equals: The first cell content top expected 30 but got 15
PASS vertical-align:bottom and `align-content:safe end` are equivalent if the container is tall
PASS `align-content:safe center` is equivalent to `unsafe center` even if the specified `height` is short
PASS `align-content:safe end` is equivalent to `unsafe end` even if the specified `height` is short

This file was deleted.

This file was deleted.

4 changes: 3 additions & 1 deletion Source/WebCore/rendering/RenderBlockFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,9 @@ void RenderBlockFlow::layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalH
LayoutUnit newHeight = logicalHeight();

LayoutUnit alignContentShift = 0_lu;
if ((!isPaginated || pageRemaining > newHeight) && (settings().alignContentOnBlocksEnabled()))
// Alignment isn't supported when fragmenting.
// Table cell alignment is handled in RenderTableCell::computeIntrinsicPadding.
if ((!isPaginated || pageRemaining > newHeight) && (settings().alignContentOnBlocksEnabled()) && !isRenderTableCell())
alignContentShift = shiftForAlignContent(oldHeight, repaintLogicalTop, repaintLogicalBottom);

{
Expand Down
19 changes: 16 additions & 3 deletions Source/WebCore/rendering/RenderTableCell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,19 @@ void RenderTableCell::computeIntrinsicPadding(LayoutUnit rowHeight)
LayoutUnit logicalHeightWithoutIntrinsicPadding = logicalHeight() - oldIntrinsicPaddingBefore - oldIntrinsicPaddingAfter;

auto intrinsicPaddingBefore = oldIntrinsicPaddingBefore;
switch (style().verticalAlign()) {
VerticalAlign alignment = style().verticalAlign();
if (auto alignContent = style().alignContent(); !alignContent.isNormal()) {
// align-content overrides vertical-align
if (alignContent.position() == ContentPosition::Baseline)
alignment = VerticalAlign::Baseline;
else if (alignContent.isCentered())
alignment = VerticalAlign::Middle;
else if (alignContent.isStartward())
alignment = VerticalAlign::Top;
else if (alignContent.isEndward())
alignment = VerticalAlign::Bottom;
}
switch (alignment) {
case VerticalAlign::Sub:
case VerticalAlign::Super:
case VerticalAlign::TextTop:
Expand Down Expand Up @@ -460,7 +472,7 @@ void RenderTableCell::styleDidChange(StyleDifference diff, const RenderStyle* ol

// Our intrinsic padding pushes us down to align with the baseline of other cells on the row. If our vertical-align
// has changed then so will the padding needed to align with other cells - clear it so we can recalculate it from scratch.
if (oldStyle && style().verticalAlign() != oldStyle->verticalAlign())
if (oldStyle && (style().verticalAlign() != oldStyle->verticalAlign() || style().alignContent() != oldStyle->alignContent()))
clearIntrinsicPadding();

// If border was changed, notify table.
Expand Down Expand Up @@ -1387,7 +1399,8 @@ void RenderTableCell::scrollbarsChanged(bool horizontalScrollbarChanged, bool ve
return;

// Shrink our intrinsic padding as much as possible to accommodate the scrollbar.
if (style().verticalAlign() == VerticalAlign::Middle) {
if ((style().verticalAlign() == VerticalAlign::Middle && style().alignContent().isNormal())
|| style().alignContent().isCentered()) {
LayoutUnit totalHeight = logicalHeight();
LayoutUnit heightWithoutIntrinsicPadding = totalHeight - intrinsicPaddingBefore() - intrinsicPaddingAfter();
totalHeight -= scrollbarHeight;
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/rendering/RenderTableCell.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ inline unsigned RenderTableCell::rowIndex() const

inline bool RenderTableCell::isBaselineAligned() const
{
if (auto alignContent = style().alignContent(); !alignContent.isNormal())
return alignContent.position() == ContentPosition::Baseline;

VerticalAlign va = style().verticalAlign();
return va == VerticalAlign::Baseline || va == VerticalAlign::TextBottom || va == VerticalAlign::TextTop || va == VerticalAlign::Super || va == VerticalAlign::Sub || va == VerticalAlign::Length;
}
Expand Down

0 comments on commit c822a6a

Please sign in to comment.