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)
 {
 }
@@ skipping 17 matching lines @@
             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;
+                columnLayout.columnHasNoCells = false;
 
-                bool cellHasContent = cell->children()->firstChild() || cell->style()->hasBorder() || cell->style()->hasPadding() || cell->style()->hasBackground();
+                bool cellHasContent = cell->minPreferredLogicalWidth();
                 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 281 matching lines @@
                 // can't satify this condition, treat as variable
                 cellLogicalWidth = Length();
             } else {
                 maxLogicalWidth = std::max(maxLogicalWidth, static_cast<int>(std::max(spanMaxLogicalWidth, cellMaxLogicalWidth) * 100  / cellLogicalWidth.percent()));
 
                 // 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.hasPercent())
-                        totalWidth += m_layoutStruct[pos].effectiveMaxLogicalWidth;
+                        totalWidth += m_layoutStruct[pos].clampedEffectiveMaxLogicalWidth();
                 }
 
                 for (unsigned pos = effCol; pos < lastCol && totalWidth > 0; ++pos) {
                     if (!m_layoutStruct[pos].effectiveLogicalWidth.hasPercent()) {
                         float percent = percentMissing * static_cast<float>(m_layoutStruct[pos].effectiveMaxLogicalWidth) / totalWidth;
-                        totalWidth -= m_layoutStruct[pos].effectiveMaxLogicalWidth;
+                        totalWidth -= m_layoutStruct[pos].clampedEffectiveMaxLogicalWidth();
                         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 += m_layoutStruct[i].clampedEffectiveMaxLogicalWidth();
             // fall through
             break;
         case Auto:
             if (m_layoutStruct[i].emptyCellsOnly) {
                 numAutoEmptyCellsOnly++;
             } else {
                 numAuto++;
-                totalAuto += m_layoutStruct[i].effectiveMaxLogicalWidth;
+                totalAuto += m_layoutStruct[i].clampedEffectiveMaxLogicalWidth();
                 allocAuto += cellLogicalWidth;
             }
             break;
         default:
             break;
         }
     }
 
     // allocate width to percent cols
     if (available > 0 && havePercent) {
@@ skipping 32 matching lines @@
                 m_layoutStruct[i].computedLogicalWidth = logicalWidth.value();
             }
         }
     }
 
     // Give each auto width column its share of the available width.
     if (available > 0 && numAuto) {
         available += allocAuto;
         distributeWidthToColumns<float, Auto, NonEmptyCells, InitialWidth, StartToEnd>(available, totalAuto);
     }
+    if (available > 0 && numAutoEmptyCellsOnly) {

[mstensho (USE GERRIT), 2015/05/20 09:34:22]

A comment in front of this would be nice.

+        unsigned total = numAutoEmptyCellsOnly;
+        distributeWidthToColumns<float, Auto, EmptyCells, InitialWidth, StartToEnd>(available, total);
+    }
 
-    // Any remaining available width expands fixed width, percent width and non-empty auto width columns, in that order.
+    // Any remaining available width expands fixed width, percent width, non-empty auto width columns and empty auto width columns, in that order.

[mstensho (USE GERRIT), 2015/05/20 09:34:22]

Should you put the following inside a large else block? If there were empty
cells above, there should be no space left to distribute.

     if (available > 0 && numFixed)
         distributeWidthToColumns<float, Fixed, AllCells, ExtraWidth, StartToEnd>(available, totalFixed);
 
     if (available > 0 && m_hasPercent && totalPercent < 100)
         distributeWidthToColumns<float, Percent, AllCells, ExtraWidth, StartToEnd>(available, totalPercent);
 
     if (available > 0 && nEffCols > numAutoEmptyCellsOnly) {
         unsigned total = nEffCols - numAutoEmptyCellsOnly;
         // Starting from the last cell is for compatibility with FF/IE - it isn't specified anywhere.
         distributeWidthToColumns<unsigned, Auto, NonEmptyCells, LeftoverWidth, EndToStart>(available, total);
     }
 
+    if (available > 0) {
+        unsigned total = numAutoEmptyCellsOnly;
+        distributeWidthToColumns<unsigned, Auto, EmptyCells, LeftoverWidth, EndToStart>(available, total);
+    }
+
     // If we have overallocated, reduce every cell according to the difference between desired width and minwidth
     // this seems to produce to the pixel exact results with IE. Wonder is some of this also holds for width distributing.
     // This is basically the reverse of how we grew the cells.
     if (available < 0)
         shrinkColumnWidth(Auto, available);
     if (available < 0)
         shrinkColumnWidth(Fixed, available);
     if (available < 0)
         shrinkColumnWidth(Percent, available);
 
     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);
 }
 
 template<typename Total, LengthType lengthType, CellsToProcess cellsToProcess, DistributionMode distributionMode, DistributionDirection distributionDirection>
 void TableLayoutAlgorithmAuto::distributeWidthToColumns(int& available, Total total)
 {
     // TODO(alancutter): Make this work correctly for calc lengths.
     int nEffCols = static_cast<int>(m_table->numEffCols());
     bool startToEnd = distributionDirection == StartToEnd;
     for (int i = startToEnd ? 0 : nEffCols - 1; startToEnd ? i < nEffCols : i > -1; startToEnd ? ++i : --i) {
         const Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
         if (cellsToProcess == NonEmptyCells && logicalWidth.isAuto() && m_layoutStruct[i].emptyCellsOnly)
             continue;
+        // When allocating width to columns with nothing but empty cells we avoid columns that exist only to flesh out a colspan and have no actual cells.

[mstensho (USE GERRIT), 2015/05/20 09:34:22]

Could you break this comment into two lines?

+        if (cellsToProcess == EmptyCells && logicalWidth.isAuto() && (!m_layoutStruct[i].emptyCellsOnly || m_layoutStruct[i].columnHasNoCells))
+            continue;
         if (distributionMode != LeftoverWidth && logicalWidth.type() != lengthType)
             continue;
 
         float factor = 1;
         if (distributionMode != LeftoverWidth) {
             if (lengthType == Percent)
                 factor = logicalWidth.percent();
             else if (lengthType == Auto || lengthType == Fixed)
-                factor = m_layoutStruct[i].effectiveMaxLogicalWidth;
+                factor = m_layoutStruct[i].clampedEffectiveMaxLogicalWidth();
         }
 
         int newWidth = available * factor / total;
         int cellLogicalWidth = (distributionMode == InitialWidth) ? max<int>(m_layoutStruct[i].computedLogicalWidth, newWidth) : newWidth;
         available -= cellLogicalWidth;
         total -= factor;
         m_layoutStruct[i].computedLogicalWidth = (distributionMode == InitialWidth) ? cellLogicalWidth : m_layoutStruct[i].computedLogicalWidth + cellLogicalWidth;
 
         // If we have run out of width to allocate we're done.
         // Also, any overallocation will be unwound later so no point in building up cells to unwind, just bail now.
@@ skipping 21 matching lines @@
             int reduce = available * minMaxDiff / logicalWidthBeyondMin;
             m_layoutStruct[i].computedLogicalWidth += reduce;
             available -= reduce;
             logicalWidthBeyondMin -= minMaxDiff;
             if (available >= 0)
                 break;
         }
     }
 }
 }