Changeset 64225 in webkit

Timestamp:

Jul 28, 2010 1:07:19 PM (14 years ago)

Author:

jamesr@google.com

Message:

2010-07-28 ​fsamuel@chromium.org <​fsamuel@chromium.org>

Reviewed by David Hyatt.

REGRESSION (r63994): Bank of America's home page is horribly mis-rendered
​https://bugs.webkit.org/show_bug.cgi?id=42993

Fixed bug introduced by patch for bug 40775.

The new table rendering code did not take into account table layouts starting
during a partial construction of the table render tree. As such,
multiple layouts on the same table resulted in more columns being generated.
This patch solves this issue.

• rendering/AutoTableLayout.cpp: (WebCore::AutoTableLayout::recalcColumn):
• rendering/RenderTable.cpp: (WebCore::RenderTable::cellAbove): (WebCore::RenderTable::cellBelow): (WebCore::RenderTable::cellBefore):
• rendering/RenderTableSection.cpp: (WebCore::RenderTableSection::ensureRows): (WebCore::RenderTableSection::addCell): (WebCore::RenderTableSection::setCellWidths): (WebCore::RenderTableSection::calcRowHeight): (WebCore::RenderTableSection::layoutRows): (WebCore::RenderTableSection::paintObject): (WebCore::RenderTableSection::appendColumn): (WebCore::RenderTableSection::splitColumn):
• rendering/RenderTableSection.h: (WebCore::RenderTableSection::CellStruct::CellStruct):

2010-07-28 ​fsamuel@chromium.org <​fsamuel@chromium.org>

Reviewed by David Hyatt.

REGRESSION (r63994): Bank of America's home page is horribly mis-rendered
​https://bugs.webkit.org/show_bug.cgi?id=42993

Restored the expectation for the slashdot logo table test.

Added a new layout test to check for the correctness of early table layout.

• fast/table/early-table-layout.html: Added.
• platform/chromium/test_expectations.txt:
• platform/gtk/fast/table/early-table-layout-expected.checksum: Added.
• platform/gtk/fast/table/early-table-layout-expected.png: Added.
• platform/gtk/fast/table/early-table-layout-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/table/early-table-layout.html (added)
• LayoutTests/platform/chromium/test_expectations.txt (modified) (1 diff)
• LayoutTests/platform/gtk/fast/table/early-table-layout-expected.checksum (added)
• LayoutTests/platform/gtk/fast/table/early-table-layout-expected.png (added)
• LayoutTests/platform/gtk/fast/table/early-table-layout-expected.txt (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/rendering/AutoTableLayout.cpp (modified) (1 diff)
• WebCore/rendering/RenderTable.cpp (modified) (3 diffs)
• WebCore/rendering/RenderTableSection.cpp (modified) (11 diffs)
• WebCore/rendering/RenderTableSection.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-07-28  fsamuel@chromium.org  <fsamuel@chromium.org>
+
+        Reviewed by David Hyatt.
+
+        REGRESSION (r63994): Bank of America's home page is horribly mis-rendered
+        https://bugs.webkit.org/show_bug.cgi?id=42993
+
+        Restored the expectation for the slashdot logo table test.
+
+        Added a new layout test to check for the correctness of early table layout.
+
+        * fast/table/early-table-layout.html: Added.
+        * platform/chromium/test_expectations.txt:
+        * platform/gtk/fast/table/early-table-layout-expected.checksum: Added.
+        * platform/gtk/fast/table/early-table-layout-expected.png: Added.
+        * platform/gtk/fast/table/early-table-layout-expected.txt: Added.
+
 2010-07-28  Victor Wang  <victorw@chromium.org>
 

trunk/LayoutTests/platform/chromium/test_expectations.txt

@@ -67 +67 @@
 BUG24182 SLOW : http/tests/xmlhttprequest/simple-cross-origin-progress-events.html = PASS
 BUG24182 SLOW WIN MAC : svg/custom/invisible-text-after-scrolling.xhtml = PASS
+BUG24182 SLOW : tables/mozilla/other/slashlogo.html = PASS
 BUG24182 SLOW WIN RELEASE : tables/mozilla/bugs/bug113235-1.html = PASS
 BUG24182 SLOW DEBUG : fast/js/toString-and-valueOf-override.html = PASS

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-28  fsamuel@chromium.org  <fsamuel@chromium.org>
+
+        Reviewed by David Hyatt.
+
+        REGRESSION (r63994): Bank of America's home page is horribly mis-rendered
+        https://bugs.webkit.org/show_bug.cgi?id=42993
+
+        Fixed bug introduced by patch for bug 40775.
+
+        The new table rendering code did not take into account table layouts starting
+        during a partial construction of the table render tree. As such,
+        multiple layouts on the same table resulted in more columns being generated.
+        This patch solves this issue.
+
+        * rendering/AutoTableLayout.cpp:
+        (WebCore::AutoTableLayout::recalcColumn):
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::cellAbove):
+        (WebCore::RenderTable::cellBelow):
+        (WebCore::RenderTable::cellBefore):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::ensureRows):
+        (WebCore::RenderTableSection::addCell):
+        (WebCore::RenderTableSection::setCellWidths):
+        (WebCore::RenderTableSection::calcRowHeight):
+        (WebCore::RenderTableSection::layoutRows):
+        (WebCore::RenderTableSection::paintObject):
+        (WebCore::RenderTableSection::appendColumn):
+        (WebCore::RenderTableSection::splitColumn):
+        * rendering/RenderTableSection.h:
+        (WebCore::RenderTableSection::CellStruct::CellStruct):
+
 2010-07-28  Tony Gentilcore  <tonyg@chromium.org>
 

trunk/WebCore/rendering/AutoTableLayout.cpp

@@ -126 +126 @@
                     }
                 } else {
-                    if (cell && (!effCol || cell->section()->primaryCellAt(i, effCol-1) != cell)) {
+                    if (cell && (!effCol || section->primaryCellAt(i, effCol-1) != cell)) {
                         // This spanning cell originates in this column.  Ensure we have
                         // a min/max width of at least 1px for this column now.

trunk/WebCore/rendering/RenderTable.cpp

@@ -1020 +1020 @@
     if (section) {
         int effCol = colToEffCol(cell->col());
-        RenderTableSection::CellStruct aboveCell;
-        // If we hit a span back up to a real cell.
-        do {
-            aboveCell = section->cellAt(rAbove, effCol);
-            effCol--;
-        } while (!aboveCell.hasCells() && aboveCell.inColSpan && effCol >= 0);
+        RenderTableSection::CellStruct& aboveCell = section->cellAt(rAbove, effCol);
         return aboveCell.primaryCell();
     } else
@@ -1052 +1047 @@
     if (section) {
         int effCol = colToEffCol(cell->col());
-        RenderTableSection::CellStruct belowCell;
-        // If we hit a colspan back up to a real cell.
-        do {
-            belowCell = section->cellAt(rBelow, effCol);
-            effCol--;
-        } while (!belowCell.hasCells() && belowCell.inColSpan && effCol >= 0);
+        RenderTableSection::CellStruct& belowCell = section->cellAt(rBelow, effCol);
         return belowCell.primaryCell();
     } else
@@ -1073 +1063 @@
     
     // If we hit a colspan back up to a real cell.
-    RenderTableSection::CellStruct prevCell;
-    do {
-        prevCell = section->cellAt(cell->row(), effCol - 1);
-        effCol--;
-    } while (!prevCell.hasCells() && prevCell.inColSpan && effCol >= 0);
+    RenderTableSection::CellStruct& prevCell = section->cellAt(cell->row(), effCol - 1);
     return prevCell.primaryCell();
 }

trunk/WebCore/rendering/RenderTableSection.cpp

@@ -162 +162 @@
         m_gridRows = numRows;
         int nCols = max(1, table()->numEffCols());
-        CellStruct emptyCellStruct;
-        emptyCellStruct.inColSpan = false;
-        emptyCellStruct.empty = false;
         for (int r = nRows; r < numRows; r++) {
             m_grid[r].row = new Row(nCols);
-            m_grid[r].row->fill(emptyCellStruct);
             m_grid[r].rowRenderer = 0;
             m_grid[r].baseline = 0;
@@ -192 +188 @@
     while (m_cCol < nCols && (cellAt(m_cRow, m_cCol).hasCells() || cellAt(m_cRow, m_cCol).inColSpan))
         m_cCol++;
+
     if (rSpan == 1) {
         // we ignore height settings on rowspan cells
@@ -225 +222 @@
     bool inColSpan = false;
     while (cSpan) {
-      int currentSpan;
-      if (m_cCol >= nCols) {
-          table()->appendColumn(cSpan);
-          currentSpan = cSpan;
-      } else {
-          if (cSpan < (int)columns[m_cCol].span)
-              table()->splitColumn(m_cCol, cSpan);
-          currentSpan = columns[m_cCol].span;
-      }
-      for (int r = 0; r < rSpan; r++) {
-          CellStruct& c = cellAt(m_cRow + r, m_cCol);
-          ASSERT(cell);
-          c.cells.append(cell);
-          // If cells overlap then we take the slow path for painting.
-          if (c.cells.size() > 1)
-              m_hasMultipleCellLevels = true;
-          if (inColSpan)
-              c.inColSpan = true;
-      }
-      m_cCol++;
-      cSpan -= currentSpan;
-      inColSpan = true;
+        int currentSpan;
+        if (m_cCol >= nCols) {
+            table()->appendColumn(cSpan);
+            currentSpan = cSpan;
+        } else {
+            if (cSpan < (int)columns[m_cCol].span)
+                table()->splitColumn(m_cCol, cSpan);
+            currentSpan = columns[m_cCol].span;
+        }
+        for (int r = 0; r < rSpan; r++) {
+            CellStruct& c = cellAt(m_cRow + r, m_cCol);
+            ASSERT(cell);
+            c.cells.append(cell);
+            // If cells overlap then we take the slow path for painting.
+            if (c.cells.size() > 1)
+                m_hasMultipleCellLevels = true;
+            if (inColSpan)
+                c.inColSpan = true;
+        }
+        m_cCol++;
+        cSpan -= currentSpan;
+        inColSpan = true;
     }
     cell->setRow(m_cRow);
@@ -261 +258 @@
         Row& row = *m_grid[i].row;
         int cols = row.size();
-        RenderTableCell* lastCell = 0;
         for (int j = 0; j < cols; j++) {
             CellStruct& current = row[j];
             RenderTableCell* cell = current.primaryCell();
-            if (!cell) {
-              ASSERT(lastCell || !current.inColSpan);
-              if (lastCell)
-                current.cells.append(lastCell);
-              if (!current.inColSpan)
-                current.empty = true;
+            if (!cell || current.inColSpan)
               continue;
-            }
-            if (current.inColSpan || current.empty)
-              continue;
-            lastCell = cell;
             int endCol = j;
             int cspan = cell->colSpan();
@@ -336 +323 @@
             CellStruct& current = cellAt(r, c);
             cell = current.primaryCell();
-            ASSERT(cell || current.empty);
-            if (!cell || current.empty || current.inColSpan)
+
+            if (!cell || current.inColSpan)
                 continue;
 
@@ -512 +499 @@
             RenderTableCell* cell = cs.primaryCell();
 
-            if (!cell || cs.empty || cs.inColSpan)
+            if (!cell || cs.inColSpan)
                 continue;
 
@@ -652 +639 @@
             CellStruct& cs = cellAt(r, c);
             RenderTableCell* cell = cs.primaryCell();
-            if (!cell || cs.empty || cs.inColSpan)
+            if (!cell || cs.inColSpan)
                 continue;
             if (r < totalRows - 1 && cell == primaryCellAt(r + 1, c))
@@ -1065 +1052 @@
                     CellStruct& current = cellAt(r, c);
                     RenderTableCell* cell = current.primaryCell();
-                    if (current.empty || (r > startrow && primaryCellAt(r - 1, c) == cell) || (c > startcol && primaryCellAt(r, c - 1) == cell))
+                    if (!cell || (r > startrow && primaryCellAt(r - 1, c) == cell) || (c > startcol && primaryCellAt(r, c - 1) == cell))
                         continue;
                     paintCell(cell, paintInfo, tx, ty);
@@ -1077 +1064 @@
                 for (unsigned c = startcol; c < endcol; c++) {
                     CellStruct& current = cellAt(r, c);
-                    if (current.empty)
+                    if (!current.hasCells())
                         continue;
                     for (unsigned i = 0; i < current.cells.size(); ++i) {
@@ -1157 +1144 @@
 void RenderTableSection::appendColumn(int pos)
 {
-    for (int row = 0; row < m_gridRows; ++row) {
+    for (int row = 0; row < m_gridRows; ++row)
         m_grid[row].row->resize(pos + 1);
-        CellStruct& c = cellAt(row, pos);
-        c.inColSpan = false;
-        c.empty = false;
-    }
 }
 
@@ -1184 +1167 @@
             r[pos + 1].inColSpan = 0;
         }
-        r[pos + 1].empty = false;
     }
 }

trunk/WebCore/rendering/RenderTableSection.h

@@ -57 +57 @@
         Vector<RenderTableCell*, 1> cells; 
         bool inColSpan; // true for columns after the first in a colspan
-        bool empty;
 
         CellStruct():
-          inColSpan(false),
-          empty(false) {}
-
-        CellStruct(const CellStruct& other):
-            cells(other.cells),
-            inColSpan(other.inColSpan),
-            empty(other.empty) {}
+          inColSpan(false) {}
         
         RenderTableCell* primaryCell()