Changeset 140907 in webkit

Timestamp:

Jan 26, 2013 1:36:34 AM (11 years ago)

Author:

robert@webkit.org

Message:

REGRESSION(r120616): Cell's logical height wrongly computed with vertical-align: baseline and rowspan
​https://bugs.webkit.org/show_bug.cgi?id=106571

Reviewed by Julien Chaffraix.

Source/WebCore:

When a cell spans multiple rows, its baseline is set on the first row it spans. r120616 contained a
couple of errors in its attempt to ensure the row height calculated in such cases was correct. When it
calculated the baseline on the first row in the span, it didn't ensure that the row height was increased
if necessary. It also suffered from allowing the baseline descent calculated on a rowspan to affect the
height of the other cells in the first row of the span.

Fix both of these errors in calcRowLogicalHeight() and refactor the calculation of the baseline and baseline
descent so that it is freestanding (rather than depending on the height of the section so far).

Tests: fast/css/vertical-align-baseline-rowspan-010.html

fast/css/vertical-align-baseline-rowspan-011.html

• rendering/RenderTableSection.cpp:

(WebCore::RenderTableSection::calcRowLogicalHeight):

LayoutTests:

• fast/css/vertical-align-baseline-rowspan-010-expected.html: Added.
• fast/css/vertical-align-baseline-rowspan-010.html: Added.
• fast/css/vertical-align-baseline-rowspan-011-expected.html: Added.
• fast/css/vertical-align-baseline-rowspan-011.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/vertical-align-baseline-rowspan-010-expected.html (added)
• LayoutTests/fast/css/vertical-align-baseline-rowspan-010.html (added)
• LayoutTests/fast/css/vertical-align-baseline-rowspan-011-expected.html (added)
• LayoutTests/fast/css/vertical-align-baseline-rowspan-011.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderTableSection.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-26  Robert Hogan  <robert@webkit.org>
+
+        REGRESSION(r120616): Cell's logical height wrongly computed with vertical-align: baseline and rowspan
+        https://bugs.webkit.org/show_bug.cgi?id=106571
+
+        Reviewed by Julien Chaffraix.
+
+        * fast/css/vertical-align-baseline-rowspan-010-expected.html: Added.
+        * fast/css/vertical-align-baseline-rowspan-010.html: Added.
+        * fast/css/vertical-align-baseline-rowspan-011-expected.html: Added.
+        * fast/css/vertical-align-baseline-rowspan-011.html: Added.
+
 2013-01-25  Elliott Sprehn  <esprehn@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-26  Robert Hogan  <robert@webkit.org>
+
+        REGRESSION(r120616): Cell's logical height wrongly computed with vertical-align: baseline and rowspan
+        https://bugs.webkit.org/show_bug.cgi?id=106571
+
+        Reviewed by Julien Chaffraix.
+
+        When a cell spans multiple rows, its baseline is set on the first row it spans. r120616 contained a
+        couple of errors in its attempt to ensure the row height calculated in such cases was correct. When it
+        calculated the baseline on the first row in the span, it didn't ensure that the row height was increased
+        if necessary. It also suffered from allowing the baseline descent calculated on a rowspan to affect the
+        height of the other cells in the first row of the span.
+
+        Fix both of these errors in calcRowLogicalHeight() and refactor the calculation of the baseline and baseline
+        descent so that it is freestanding (rather than depending on the height of the section so far). 
+
+        Tests: fast/css/vertical-align-baseline-rowspan-010.html
+               fast/css/vertical-align-baseline-rowspan-011.html
+
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::calcRowLogicalHeight):
+
 2013-01-26  James Simonsen  <simonjam@chromium.org>
 

trunk/Source/WebCore/rendering/RenderTableSection.cpp

@@ -319 +319 @@
                 m_rowPos[r + 1] = max(m_rowPos[r + 1], m_rowPos[cellStartRow] + cellLogicalHeight);
 
-                // find out the baseline
+                // Find out the baseline. The baseline is set on the first row in a rowspan.
                 EVerticalAlign va = cell->style()->verticalAlign();
                 if (va == BASELINE || va == TEXT_BOTTOM || va == TEXT_TOP || va == SUPER || va == SUB || va == LENGTH) {
                     LayoutUnit baselinePosition = cell->cellBaselinePosition();
                     if (baselinePosition > cell->borderBefore() + cell->paddingBefore()) {
-                        m_grid[cellStartRow].baseline = max(m_grid[cellStartRow].baseline, baselinePosition - cell->intrinsicPaddingBefore());
-                        baselineDescent = max(baselineDescent, m_rowPos[cellStartRow] + cellLogicalHeight - (baselinePosition - cell->intrinsicPaddingBefore()));
+                        m_grid[cellStartRow].baseline = max(m_grid[cellStartRow].baseline, baselinePosition);
+                        // The descent of a cell that spans multiple rows does not affect the height of the first row it spans, so don't let it
+                        // become the baseline descent applied to the rest of the row. 
+                        if (cell->rowSpan() == 1)
+                            baselineDescent = max(baselineDescent, cellLogicalHeight - (baselinePosition - cell->intrinsicPaddingBefore()));
+                        m_rowPos[cellStartRow + 1] = max<int>(m_rowPos[cellStartRow + 1], m_rowPos[cellStartRow] + m_grid[cellStartRow].baseline + baselineDescent);
                     }
                 }
             }
         }
-
-        // do we have baseline aligned elements?
-        if (m_grid[r].baseline)
-            // increase rowheight if baseline requires
-            m_rowPos[r + 1] = max<int>(m_rowPos[r + 1], m_grid[r].baseline + baselineDescent);
 
         // Add the border-spacing to our final position.