Respect break-inside:avoid on table rows

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

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 985 matching lines @@
 
 bool LayoutTableCell::backgroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect) const
 {
     // If this object has layer, the area of collapsed borders should be transparent
     // to expose the collapsed borders painted on the underlying layer.
     if (hasLayer() && table()->collapseBorders())
         return false;
     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.