Not able to set width using ColGroup when table-layout is fixed.

Source/core/rendering/FixedTableLayout.cpp

 /*
  * Copyright (C) 2002 Lars Knoll (knoll@kde.org)
  *           (C) 2002 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc.
  *
  * 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.
  *
@@ skipping 141 matching lines @@
         if (logicalWidth.isFixed() && logicalWidth.isPositive()) {
             fixedBorderBoxLogicalWidth = cell->adjustBorderBoxLogicalWidthForBoxSizing(logicalWidth.value());
             logicalWidth.setValue(fixedBorderBoxLogicalWidth);
         }
 
         unsigned usedSpan = 0;
         while (usedSpan < span && currentColumn < nEffCols) {
             float eSpan = m_table->spanOfEffCol(currentColumn);
             // Only set if no col element has already set it.
             if (m_width[currentColumn].isAuto() && logicalWidth.type() != Auto) {
-                m_width[currentColumn] = logicalWidth;
+                // Some column do not have col width specified by colgroup so
+                // by default those column should set as auto.
+                if (span > 1)
+                    m_width[currentColumn].setValue(m_width[currentColumn].type(), logicalWidth.intValue());

[Julien - ping for review, 2013/10/22 21:11:41]

I don't understand how this line is correct: m_width[currentColumn].type() is
Auto if you are in this branch, setting a value in this case is just wrong.

If I am reading correctly, this line just ignore the logical width value.

[suchit.agrawal, 2013/10/23 13:41:06]

If column is auto then we need to keep it auto.
By assigning 'logicalWidth' to 'm_width[currentColumn]',
'm_width[currentColumn]' become fixed. So now we are assigning 'logicalWidth' to
'm_width[currentColumn]' but setting 'm_width[currentColumn]' as Auto.

[Julien - ping for review, 2013/10/23 17:04:52]

Again, you're explaining 'what' which can be inferred from the code. What I am
interested in is *why*.

Also what you are telling me is basically that we should ignore logicalWidth for
span > 1, we don't need this confusing line.

+                else
+                    m_width[currentColumn] = logicalWidth;
                 m_width[currentColumn] *= eSpan / span;
                 usedWidth += fixedBorderBoxLogicalWidth * eSpan / span;
             }
             usedSpan += eSpan;
             ++currentColumn;
         }
 
         // FixedTableLayout doesn't use min/maxPreferredLogicalWidths, but we need to clear the
         // dirty bit on the cell so that we'll correctly mark its ancestors dirty
         // in case we later call setPreferredLogicalWidthsDirty() on it later.
@@ skipping 139 matching lines @@
     for (unsigned i = 0; i < nEffCols; i++) {
         m_table->setColumnPosition(i, pos);
         pos += calcWidth[i] + hspacing;
     }
     int colPositionsSize = m_table->columnPositions().size();
     if (colPositionsSize > 0)
         m_table->setColumnPosition(colPositionsSize - 1, pos);
 }
 
 } // namespace WebCore