Don't add artifical 1 pixel width to empty tables

Source/core/layout/TableLayoutAlgorithmAuto.cpp

Index: Source/core/layout/TableLayoutAlgorithmAuto.cpp
diff --git a/Source/core/layout/TableLayoutAlgorithmAuto.cpp b/Source/core/layout/TableLayoutAlgorithmAuto.cpp
index f518af3372fc6a9f3404b5badcaa2a70af603852..1cd78a0e700a78012d3661aa7bd4d002f04594f9 100644
--- a/Source/core/layout/TableLayoutAlgorithmAuto.cpp
+++ b/Source/core/layout/TableLayoutAlgorithmAuto.cpp
@@ -26,6 +26,7 @@
 #include "core/layout/LayoutTableCell.h"
 #include "core/layout/LayoutTableCol.h"
 #include "core/layout/LayoutTableSection.h"
+#include "core/layout/LayoutText.h"
 #include "core/layout/TextAutosizer.h"
 
 namespace blink {
@@ -41,6 +42,16 @@ TableLayoutAlgorithmAuto::~TableLayoutAlgorithmAuto()
 {
 }
 
+static bool isEmptyCell(LayoutObject* object)
+{
+    for (LayoutObject* curr = object; curr; curr = curr->nextSibling()) {
+        if (curr->isText() && toLayoutText(curr)->isAllCollapsibleWhitespace())
+            continue;
+        return false;
+    }
+    return true;
+}
+
 void TableLayoutAlgorithmAuto::recalcColumn(unsigned effCol)
 {
     Layout& columnLayout = m_layoutStruct[effCol];
@@ -64,14 +75,13 @@ void TableLayoutAlgorithmAuto::recalcColumn(unsigned effCol)
                 if (current.inColSpan || !cell)
                     continue;
 
-                bool cellHasContent = cell->children()->firstChild() || cell->style()->hasBorder() || cell->style()->hasPadding() || cell->style()->hasBackground();
+                bool cellHasContent = !isEmptyCell(cell->firstChild()) || cell->style()->hasBorder() || cell->style()->hasPadding() || cell->style()->hasBackground();
                 if (cellHasContent)
                     columnLayout.emptyCellsOnly = false;
 
                 // A cell originates in this column. Ensure we have
                 // a min/max width of at least 1px for this column now.
                 columnLayout.minLogicalWidth = std::max<int>(columnLayout.minLogicalWidth, cellHasContent ? 1 : 0);
-                columnLayout.maxLogicalWidth = std::max<int>(columnLayout.maxLogicalWidth, 1);
 
                 if (cell->colSpan() == 1) {
                     columnLayout.minLogicalWidth = std::max<int>(cell->minPreferredLogicalWidth(), columnLayout.minLogicalWidth);
@@ -374,7 +384,7 @@ int TableLayoutAlgorithmAuto::calcEffectiveLogicalWidth()
 
                 for (unsigned pos = effCol; pos < lastCol && totalWidth > 0; ++pos) {
                     if (!m_layoutStruct[pos].effectiveLogicalWidth.isPercent()) {
-                        float percent = percentMissing * static_cast<float>(m_layoutStruct[pos].effectiveMaxLogicalWidth) / totalWidth;
+                        float percent = percentMissing * static_cast<float>(std::max<int>(1, m_layoutStruct[pos].effectiveMaxLogicalWidth)) / totalWidth;
                         totalWidth -= m_layoutStruct[pos].effectiveMaxLogicalWidth;
                         percentMissing -= percent;
                         if (percent > 0)
@@ -533,7 +543,7 @@ void TableLayoutAlgorithmAuto::layout()
             break;
         case Fixed:
             numFixed++;
-            totalFixed += m_layoutStruct[i].effectiveMaxLogicalWidth;
+            totalFixed += std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth);

[dsinclair, 2015/03/24 00:03:48]

We do this a bunch of times, would it be better to change how we store
effectiveMaxLogicalWidth to so it does the std::max call implicitly? Or, do we
need to use it in cases where we don't want the 1 minimum?

[mstensho (USE GERRIT), 2015/03/24 08:36:37]

I don't think I understand why it's still necessary to clamp to 1 like this.
Sounds like a bug / shortcoming in the column width distribution algorithm.

[rhogan, 2015/03/24 20:06:11]

It uses the clamped value when deciding what proportion of the table's width the
column represents and also when figuring out how much width is left to
distribute.

I think I can represent it better per dsinclair's suggestion.

[mstensho (USE GERRIT), 2015/03/25 10:10:52]

I think that it's the width distribution algorithm that needs to be fixed, so
that it can cope with 0 max width. Zero-width columns should only receive a
portion of extra space if there are no better candidates (columns with
percentages, columns with fixed width, columns with non-zero max width). Blink
seems to lack this step in the width distribution algorithm.

<p>There should be no red.</p>
<table cellspacing="0" cellpadding="0" style="width:100%;">
    <tr>
        <td style="background:red;"></td>
        <td>cell2</td>
    </tr>
</table>

This is essentially crbug.com/406273 and I think that's what should be fixed.
Then crbug.com/424338 (what you're aiming to fix) should automatically go away
too.

             // fall through
             break;
         case Auto:
@@ -541,7 +551,7 @@ void TableLayoutAlgorithmAuto::layout()
                 numAutoEmptyCellsOnly++;
             } else {
                 numAuto++;
-                totalAuto += m_layoutStruct[i].effectiveMaxLogicalWidth;
+                totalAuto += std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth);
                 allocAuto += cellLogicalWidth;
             }
             break;
@@ -595,9 +605,9 @@ void TableLayoutAlgorithmAuto::layout()
         for (size_t i = 0; i < nEffCols; ++i) {
             Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (logicalWidth.isAuto() && totalAuto && !m_layoutStruct[i].emptyCellsOnly) {
-                int cellLogicalWidth = std::max<int>(m_layoutStruct[i].computedLogicalWidth, static_cast<int>(available * static_cast<float>(m_layoutStruct[i].effectiveMaxLogicalWidth) / totalAuto));
+                int cellLogicalWidth = std::max<int>(m_layoutStruct[i].computedLogicalWidth, static_cast<int>(available * static_cast<float>(std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth)) / totalAuto));
                 available -= cellLogicalWidth;
-                totalAuto -= m_layoutStruct[i].effectiveMaxLogicalWidth;
+                totalAuto -= std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth);
                 m_layoutStruct[i].computedLogicalWidth = cellLogicalWidth;
             }
         }
@@ -608,9 +618,9 @@ void TableLayoutAlgorithmAuto::layout()
         for (size_t i = 0; i < nEffCols; ++i) {
             Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (logicalWidth.isFixed()) {
-                int cellLogicalWidth = static_cast<int>(available * static_cast<float>(m_layoutStruct[i].effectiveMaxLogicalWidth) / totalFixed);
+                int cellLogicalWidth = static_cast<int>(available * static_cast<float>(std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth)) / totalFixed);
                 available -= cellLogicalWidth;
-                totalFixed -= m_layoutStruct[i].effectiveMaxLogicalWidth;
+                totalFixed -= std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth);
                 m_layoutStruct[i].computedLogicalWidth += cellLogicalWidth;
             }
         }