Don't add artifical 1 pixel width to empty tables

Source/core/layout/TableLayoutAlgorithmAuto.cpp

 /*
  * Copyright (C) 2002 Lars Knoll (knoll@kde.org)
  *           (C) 2002 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2006, 2008, 2010 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
  * License as published by the Free Software Foundation; either
  * version 2 of the License.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Library General Public License for more details.
  *
  * You should have received a copy of the GNU Library General Public License
  * along with this library; see the file COPYING.LIB.  If not, write to
  * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
  * Boston, MA 02110-1301, USA.
  */
 
 #include "config.h"
 #include "core/layout/TableLayoutAlgorithmAuto.h"
 
 #include "core/layout/LayoutTable.h"
 #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 {
 
 TableLayoutAlgorithmAuto::TableLayoutAlgorithmAuto(LayoutTable* table)
     : TableLayoutAlgorithm(table)
     , m_hasPercent(false)
     , m_effectiveLogicalWidthDirty(true)
 {
 }
 
 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];
 
     LayoutTableCell* fixedContributor = 0;
     LayoutTableCell* maxContributor = 0;
 
     for (LayoutObject* child = m_table->children()->firstChild(); child; child = child->nextSibling()) {
         if (child->isLayoutTableCol()) {
             // LayoutTableCols don't have the concept of preferred logical width, but we need to clear their dirty bits
             // so that if we call setPreferredWidthsDirty(true) on a col or one of its descendants, we'll mark it's
             // ancestors as dirty.
             toLayoutTableCol(child)->clearPreferredLogicalWidthsDirtyBits();
         } else if (child->isTableSection()) {
             LayoutTableSection* section = toLayoutTableSection(child);
             unsigned numRows = section->numRows();
             for (unsigned i = 0; i < numRows; i++) {
                 LayoutTableSection::CellStruct current = section->cellAt(i, effCol);
                 LayoutTableCell* cell = current.primaryCell();
 
                 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);
                     if (cell->maxPreferredLogicalWidth() > columnLayout.maxLogicalWidth) {
                         columnLayout.maxLogicalWidth = cell->maxPreferredLogicalWidth();
                         maxContributor = cell;
                     }
 
                     // All browsers implement a size limit on the cell's max width.
                     // Our limit is based on KHTML's representation that used 16 bits widths.
@@ skipping 282 matching lines @@
                 // all non percent columns in the span get percent values to sum up correctly.
                 float percentMissing = cellLogicalWidth.percent() - totalPercent;
                 int totalWidth = 0;
                 for (unsigned pos = effCol; pos < lastCol; ++pos) {
                     if (!m_layoutStruct[pos].effectiveLogicalWidth.isPercent())
                         totalWidth += m_layoutStruct[pos].effectiveMaxLogicalWidth;
                 }
 
                 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)
                             m_layoutStruct[pos].effectiveLogicalWidth.setValue(Percent, percent);
                         else
                             m_layoutStruct[pos].effectiveLogicalWidth = Length();
                     }
                 }
             }
         }
@@ skipping 138 matching lines @@
         m_layoutStruct[i].computedLogicalWidth = cellLogicalWidth;
         available -= cellLogicalWidth;
         Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
         switch (logicalWidth.type()) {
         case Percent:
             havePercent = true;
             totalPercent += logicalWidth.percent();
             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:
             if (m_layoutStruct[i].emptyCellsOnly) {
                 numAutoEmptyCellsOnly++;
             } else {
                 numAuto++;
-                totalAuto += m_layoutStruct[i].effectiveMaxLogicalWidth;
+                totalAuto += std::max<int>(1, m_layoutStruct[i].effectiveMaxLogicalWidth);
                 allocAuto += cellLogicalWidth;
             }
             break;
         default:
             break;
         }
     }
 
     // allocate width to percent cols
     if (available > 0 && havePercent) {
@@ skipping 33 matching lines @@
             }
         }
     }
 
     // now satisfy variable
     if (available > 0 && numAuto) {
         available += allocAuto; // this gets redistributed
         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;
             }
         }
     }
 
     // spread over fixed columns
     if (available > 0 && numFixed) {
         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;
             }
         }
     }
 
     // spread over percent colums
     if (available > 0 && m_hasPercent && totalPercent < 100) {
         for (size_t i = 0; i < nEffCols; ++i) {
             Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (logicalWidth.isPercent()) {
@@ skipping 106 matching lines @@
 
     int pos = 0;
     for (size_t i = 0; i < nEffCols; ++i) {
         m_table->setColumnPosition(i, pos);
         pos += m_layoutStruct[i].computedLogicalWidth + m_table->hBorderSpacing();
     }
     m_table->setColumnPosition(m_table->columnPositions().size() - 1, pos);
 }
 
 }