Bug 152796 - Padding added to table-cell element after font-size change
Summary: Padding added to table-cell element after font-size change
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Macintosh OS X 10.11
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-06 12:21 PST by Raphael Schweikert
Modified: 2016-01-11 14:03 PST (History)
7 users (show)

See Also:


Attachments
Reduced testcase (290 bytes, text/html)
2016-01-06 12:21 PST, Raphael Schweikert
no flags Details
Another test reduction (467 bytes, text/html)
2016-01-06 15:29 PST, zalan
no flags Details
Patch (6.25 KB, patch)
2016-01-11 09:44 PST, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Raphael Schweikert 2016-01-06 12:21:07 PST
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.
Comment 1 zalan 2016-01-06 15:29:27 PST
Created attachment 268419 [details]
Another test reduction
Comment 2 zalan 2016-01-10 14:46:51 PST
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);
Comment 3 zalan 2016-01-11 09:44:15 PST
Created attachment 268691 [details]
Patch
Comment 4 Dave Hyatt 2016-01-11 13:13:05 PST
Comment on attachment 268691 [details]
Patch

r=me
Comment 5 WebKit Commit Bot 2016-01-11 14:03:17 PST
Comment on attachment 268691 [details]
Patch

Clearing flags on attachment: 268691

Committed r194867: <http://trac.webkit.org/changeset/194867>
Comment 6 WebKit Commit Bot 2016-01-11 14:03:21 PST
All reviewed patches have been landed.  Closing bug.