Commit 8bf47bb4 authored by robhogan's avatar robhogan Committed by Commit bot

Don't repeat headers if at least one row of content doesn't fit

The spec (https://drafts.csswg.org/css-tables-3/#repeated-headers) tells us that
we shouldn't bother repeating headers if a row of content doesn't fit. It's not
clear what to do in the situation where it's just the first page that the row
doesn't fit. I think we ultimately want to do the same as Edge/IE and still
repeat the headers on subsequent pages.

That would require a second layout pass to achieve so for now just drop the
repeating headers if we can't fit a row on the first page.

BUG=624814
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2326303002
Cr-Commit-Position: refs/heads/master@{#418838}
parent 5dc5867a
<!DOCTYPE html>
<style>
table {
border-collapse: collapse;
}
td, th {
background-color: #ddd;
border: 1px solid black;
}
tr {
break-inside: avoid;
}
</style>
<p>crbug.com/624814: A table header group should not repeat on each page if we can't fit at least one content row below it.</p>
<div style="-webkit-columns:3; line-height: 20px; column-fill: auto; height:190px; background-color: yellow;">
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<table>
<thead>
<tr>
<th>Col 1</th>
<th>Col 2</th>
</tr>
</thead>
<tbody>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
</tbody>
</table>
</div>
<!DOCTYPE html>
<style>
table {
border-collapse: collapse;
}
td, th {
background-color: #ddd;
border: 1px solid black;
}
thead, tr {
break-inside: avoid;
}
thead {
break-after: avoid;
}
</style>
<p>crbug.com/624814: A table header group should not repeat on each page if we can't fit at least one content row below it.</p>
<div style="-webkit-columns:3; line-height: 20px; column-fill: auto; height:190px; background-color: yellow;">
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<table>
<thead>
<tr>
<th>Col 1</th>
<th>Col 2</th>
</tr>
</thead>
<tbody>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
<tr><td>Te</td><td>xt</td></tr>
</tbody>
</table>
</div>
......@@ -1002,10 +1002,11 @@ void LayoutTableSection::layoutRows()
rowLayoutObject->updateLayerTransformAfterLayout();
if (isPaginated) {
paginationStrutOnRow = paginationStrutForRow(rowLayoutObject, LayoutUnit(m_rowPos[r]));
rowLayoutObject->setPaginationStrut(LayoutUnit(paginationStrutOnRow));
if (paginationStrutOnRow) {
// If there isn't room for at least one content row on a page with a header group, then
// we won't repeat the header on each page.
if (!r && table()->header() && table()->sectionAbove(this) == table()->header())
if (!r && table()->header() && table()->sectionAbove(this) == table()->header() && table()->header()->getPaginationBreakability() != AllowAnyBreaks)
state.setHeightOffsetForTableHeaders(state.heightOffsetForTableHeaders() - table()->header()->logicalHeight());
// If we have a header group we will paint it at the top of each page, move the rows
// down to accomodate it.
......@@ -1718,7 +1719,7 @@ void LayoutTableSection::setLogicalPositionForCell(LayoutTableCell* cell, unsign
cell->setLogicalLocation(cellLocation);
}
bool LayoutTableSection::hasRepeatingHeaderGroup() const
bool LayoutTableSection::isRepeatingHeaderGroup() const
{
if (getPaginationBreakability() == LayoutBox::AllowAnyBreaks)
return false;
......@@ -1735,7 +1736,7 @@ bool LayoutTableSection::hasRepeatingHeaderGroup() const
// If the first row of the section after the header group doesn't fit on the page, then
// don't repeat the header on each page. See https://drafts.csswg.org/css-tables-3/#repeated-headers
LayoutTableSection* sectionBelow = table()->sectionBelow(this);
if (sectionBelow && sectionBelow->firstRow() && sectionBelow->paginationStrutForRow(sectionBelow->firstRow(), sectionBelow->logicalTop()))
if (sectionBelow && sectionBelow->firstRow() && sectionBelow->firstRow()->paginationStrut())
return false;
return true;
......@@ -1748,7 +1749,7 @@ bool LayoutTableSection::mapToVisualRectInAncestorSpace(const LayoutBoxModelObje
// Repeating table headers are painted once per fragmentation page/column. This does not go through the regular fragmentation machinery,
// so we need special code to expand the invalidation rect to contain all positions of the header in all columns.
// Note that this is in flow thread coordinates, not visual coordinates. The enclosing LayoutFlowThread will convert to visual coordinates.
if (table()->header() == this && hasRepeatingHeaderGroup())
if (table()->header() == this && isRepeatingHeaderGroup())
rect.setHeight(table()->logicalHeight());
return LayoutTableBoxComponent::mapToVisualRectInAncestorSpace(ancestor, rect, flags);
}
......
......@@ -311,7 +311,7 @@ public:
bool mapToVisualRectInAncestorSpace(const LayoutBoxModelObject* ancestor, LayoutRect&, VisualRectFlags = DefaultVisualRectFlags) const override;
bool hasRepeatingHeaderGroup() const;
bool isRepeatingHeaderGroup() const;
protected:
void styleDidChange(StyleDifference, const ComputedStyle* oldStyle) override;
......
......@@ -37,7 +37,7 @@ inline const LayoutTableCell* TableSectionPainter::primaryCellToPaint(unsigned r
void TableSectionPainter::paintRepeatingHeaderGroup(const PaintInfo& paintInfo, const LayoutPoint& paintOffset, const CollapsedBorderValue& currentBorderValue, ItemToPaint itemToPaint)
{
if (!m_layoutTableSection.hasRepeatingHeaderGroup())
if (!m_layoutTableSection.isRepeatingHeaderGroup())
return;
LayoutTable* table = m_layoutTableSection.table();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment