Bug 152796 – Padding added to table-cell element after font-size change

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Raphael Schweikert]

Created attachment 268388 [details]
Reduced testcase

Verified in Safari 9.0.2 (11601.3.9) and WebKit Nightly 11601.3.9, r194284

In some circumstances, changing the font-size on a display: table-cell element makes said element change its position. The added space behaves like padding and lights up green in the inspector but is not removable by setting style.paddingTop = 0 on the node.

Note that this behaviour can only be observed when two display: table-cell are used, it’s not present with only one such element.

Firefox and Chrome do not exhibit this behaviour.

STEPS TO REPRODUCE

1. Open the attached testcase in Safari or a WebKit nightly
2. Hover over and out from the texts a few times

EXPECTED RESULTS

When not hovered the text should look the same as when the page was loaded.

ACTUAL RESULTS

The position of the text changes with each hover.

[zalan]

Created attachment 268419 [details]
Another test reduction

[zalan]

baseline position compute is broken.

diff --git a/Source/WebCore/rendering/RenderTableSection.cpp b/Source/WebCore/rendering/RenderTableSection.cpp
index ff9877b..a96cede 100644
--- a/Source/WebCore/rendering/RenderTableSection.cpp
+++ b/Source/WebCore/rendering/RenderTableSection.cpp
@@ -333,15 +333,15 @@ LayoutUnit RenderTableSection::calcRowLogicalHeight()
 
                 // Find out the baseline. The baseline is set on the first row in a rowspan.
                 if (cell->isBaselineAligned()) {
-                    LayoutUnit baselinePosition = cell->cellBaselinePosition();
-                    if (baselinePosition > cell->borderAndPaddingBefore()) {
+                    LayoutUnit baselinePosition = cell->cellBaselinePosition() - cell->intrinsicPaddingBefore();
+                    if (baselinePosition > cell->borderAndPaddingBefore() - cell->intrinsicPaddingBefore()) {
                         m_grid[cellStartRow].baseline = std::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. Also we don't account for the baseline descent of
                         // non-spanning cells when computing a spanning cell's extent.
                         LayoutUnit cellStartRowBaselineDescent = 0;
                         if (cell->rowSpan() == 1) {
-                            baselineDescent = std::max(baselineDescent, cellLogicalHeight - (baselinePosition - cell->intrinsicPaddingBefore()));
+                            baselineDescent = std::max(baselineDescent, cellLogicalHeight - baselinePosition);
                             cellStartRowBaselineDescent = baselineDescent;
                         }
                         m_rowPos[cellStartRow + 1] = std::max(m_rowPos[cellStartRow + 1], m_rowPos[cellStartRow] + m_grid[cellStartRow].baseline + cellStartRowBaselineDescent);

[zalan]

Created attachment 268691 [details]
Patch

[Dave Hyatt]

Comment on attachment 268691 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 268691 [details]
Patch

Clearing flags on attachment: 268691

Committed r194867: <http://trac.webkit.org/changeset/194867>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.