Respect break-inside:avoid on table rows

third_party/WebKit/Source/core/layout/LayoutTableCell.cpp

Index: third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
index bc62253984249840a2d113bff033d44650651947..0a038436f51203c325ac63f977a3b614a53f7ec8 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
@@ -1003,4 +1003,14 @@ bool LayoutTableCell::backgroundIsKnownToBeOpaqueInRect(const LayoutRect& localR
     return LayoutBlockFlow::backgroundIsKnownToBeOpaqueInRect(localRect);
 }
 
+LayoutUnit LayoutTableCell::calculatePaginationStrutToFitContent(LayoutUnit offset, LayoutUnit strutToNextPage, LayoutUnit contentLogicalHeight) const
+{
+    // If break-inside:avoid/forbid is set on a cell we should avoid setting a strut on lines unless the cell is taller than the page,

[mstensho (USE GERRIT), 2016/03/29 09:57:46]

"forbid" is not a valid value.
Also, you're checking if its set on a row, not on a cell.
But I really don't understand why we want this at all. This method is called
when we have already decided that we need to break (either before a block or
before a line), and someone might be upset by the fact that we don't respect it.
Furthermore, no tests fail if I remove this piece of code.

[rhogan, 2016/03/29 19:04:36]

Reflecting my loose-grasp of the break-inside value. :/
Yes.
We're still going to break before the block or line - if break-inside:avoid is
set we'll break before the cell/row, if not then we'll break before the line.
The one exception is where the row/cell is taller than the page, in that
situation we can only break before the line because there's nowhere sensible to
move the row/cell to.

So I don't think this function is overturning anything - it's just deciding how
best to implement the required break on the cell.
My printing/ tests catch this.

[mstensho (USE GERRIT), 2016/03/31 09:55:04]

OK, which test? Should be fairly easy to convert it to multicol.

[rhogan, 2016/04/03 14:56:41]

Now covered by:
LayoutTests/fast/multicol/single-line-cells-paginated-with-text.html 

+    // in which case we've no choice but to allow it. This special-casing is needed because cells don't propagate struts like normal
+    // layout blocks - the struts are managed directly by the section when it lays out rows and cells.
+    LayoutUnit pageLogicalHeight = pageLogicalHeightForOffset(offset);
+    if (toLayoutBox(parent())->getPaginationBreakability() != AllowAnyBreaks && table()->vBorderSpacing() + logicalHeight() + contentLogicalHeight < pageLogicalHeight)
+        return LayoutUnit();
+    return LayoutBox::calculatePaginationStrutToFitContent(offset, strutToNextPage, contentLogicalHeight);

[mstensho (USE GERRIT), 2016/03/29 09:57:46]

Please use LayoutBlockFlow:: (super class)

+}
 } // namespace blink

[mstensho (USE GERRIT), 2016/03/29 09:57:46]

Please re-add the blank line before this.