Update our treatment of repeating headers in tables

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

Index: third_party/WebKit/Source/core/layout/LayoutTable.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutTable.cpp b/third_party/WebKit/Source/core/layout/LayoutTable.cpp
index 3e64f7298ab2b4bdaaceb428a285793cb3c289cd..5bb9ab68d2ca68bd67fdfa2d1e692f8b8ed3f440 100644
--- a/third_party/WebKit/Source/core/layout/LayoutTable.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutTable.cpp
@@ -470,8 +470,10 @@ void LayoutTable::LayoutSection(
   bool needed_layout = section.NeedsLayout();
   if (needed_layout)
     section.UpdateLayout();
-  if (needed_layout || table_height_changing == kTableHeightChanging)
+  if (needed_layout || table_height_changing == kTableHeightChanging) {
     section.SetLogicalHeight(LayoutUnit(section.CalcRowLogicalHeight()));
+    section.SetHeaderGroupShouldRepeat();
+  }
 
   if (View()->GetLayoutState()->IsPaginated())
     UpdateFragmentationInfoForChild(section);
@@ -661,15 +663,15 @@ void LayoutTable::UpdateLayout() {
         // If the repeating header group allows at least one row of content,
         // then store the offset for other sections to offset their rows
         // against.
-        LayoutUnit section_logical_height = section->LogicalHeight();
-        if (section_logical_height <
-                section->PageLogicalHeightForOffset(section->LogicalTop()) &&
-            section->GetPaginationBreakability() != kAllowAnyBreaks) {
+        if (section->IsRepeatingHeaderGroup()) {
+          LayoutUnit offset_for_table_headers =
+              state.HeightOffsetForTableHeaders();
+          offset_for_table_headers += section->LogicalHeight();

[mstensho (USE GERRIT), 2017/05/22 10:40:44]

So here we are re-introducing support for nested repeated headers, while
previously we only repeated the header of the innermost table? Is that really
something we want? I mean, it's fine, but only if the alternative (NOT
re-introducing it) would lead to more complex code. Or is the spec super-clear
on how this is supposed to be handled now? I couldn't find anything.

[rhogan, 2017/05/22 18:06:04]

I think the spec implies it now (by allowing you to repeat a header regardless
of whether one content row fits entirely on the page or not) and I know Francois
the spec editor wants to maintain the majority browser behaviour in most cases,
which is to repeat nested headers. I don't think it makes things simpler to
support it - but I think it will be hard to support repeating headers even where
one content row doesn't fit but not support nested headers.

Relevant discussion re 'one content row':

https://lists.w3.org/Archives/Public/www-style/2016Dec/0070.html

I can raise it but given that FF and Edge currently nest and the spec supports
that implicitly I don't think I'll get a response that supports removing the
nested case.

[mstensho (USE GERRIT), 2017/05/22 19:17:34]

Thanks for the clarification. I agree that support for nested tables is now
implied. I definitely think you did the right thing by bringing it back.

           // Don't include any strut in the header group - we only want the
           // height from its content.
-          LayoutUnit offset_for_table_headers = section_logical_height;
           if (LayoutTableRow* row = section->FirstRow())
             offset_for_table_headers -= row->PaginationStrut();
+          state.SetHeightOffsetForTableHeaders(offset_for_table_headers);
           SetRowOffsetFromRepeatingHeader(offset_for_table_headers);
         }
       }