Title: [145576] branches/chromium/1410
Revision
145576
Author
jchaffr...@webkit.org
Date
2013-03-12 13:20:02 -0700 (Tue, 12 Mar 2013)

Log Message

Merge 145305 "REGRESSION(r140907): Incorrect baseline for cells ..."

> REGRESSION(r140907): Incorrect baseline for cells with media content during load
> https://bugs.webkit.org/show_bug.cgi?id=108357
> 
> Reviewed by Julien Chaffraix.
> 
> Source/WebCore:
> 
> If a cell has replaced content, the intrinsic height of its content can change between layouts. If that's the case then the intrinsic padding we used
> for layout (the padding required to push the contents of the cell down to the row's baseline) is included in the new height and baseline and makes both
> of them wrong. So if a cell's content's intrinsic height has changed push the new content up into the intrinsic padding and relayout so that the rest of
> table and row layout can use the correct baseline and height for this cell.
> 
> Tests: fast/css/vertical-align-baseline-rowspan-012.html
>        http/tests/css/vertical-align-baseline-after-image-load-2.html
>        http/tests/css/vertical-align-baseline-after-image-load-3.html
>        http/tests/css/vertical-align-baseline-after-image-load.html
> 
> * rendering/RenderTableCell.cpp:
> (WebCore::RenderTableCell::layout):
> * rendering/RenderTableCell.h:
> (WebCore::RenderTableCell::isBaselineAligned):
> * rendering/RenderTableSection.cpp:
> (WebCore::RenderTableSection::calcRowLogicalHeight):
> (WebCore::RenderTableSection::layoutRows):
> 
> LayoutTests:
> 
> * fast/css/vertical-align-baseline-rowspan-012-expected.html: Added.
> * fast/css/vertical-align-baseline-rowspan-012.html: Added.
> * http/tests/css/vertical-align-baseline-after-image-load-2-expected.html: Added.
> * http/tests/css/vertical-align-baseline-after-image-load-2.html: Added.
> * http/tests/css/vertical-align-baseline-after-image-load-3-expected.html: Added.
> * http/tests/css/vertical-align-baseline-after-image-load-3.html: Added.
> * http/tests/css/vertical-align-baseline-after-image-load-expected.html: Added.
> * http/tests/css/vertical-align-baseline-after-image-load.html: Added.

TBR=rob...@webkit.org

Modified Paths

Added Paths

Diff

Copied: branches/chromium/1410/LayoutTests/fast/css/vertical-align-baseline-rowspan-012-expected.html (from rev 145305, trunk/LayoutTests/fast/css/vertical-align-baseline-rowspan-012-expected.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/fast/css/vertical-align-baseline-rowspan-012-expected.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/fast/css/vertical-align-baseline-rowspan-012-expected.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90;
+    }
+    .padded {
+        padding-top: 51px;
+    }
+    #img1 {
+        width: 100px;
+        height: 150px;
+        background-color: green;
+    }
+    #img2 {
+        background-color: green;
+    }
+</style>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  The bottom of the two green boxes should be aligned and there should be no space above the first box.</p>
+<table>
+    <tbody>
+        <tr>
+             <td><img id="img1" src="" class="padded"><img id="img2" src=""
+        </tr>
+    </tbody>
+</table>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/fast/css/vertical-align-baseline-rowspan-012.html (from rev 145305, trunk/LayoutTests/fast/css/vertical-align-baseline-rowspan-012.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/fast/css/vertical-align-baseline-rowspan-012.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/fast/css/vertical-align-baseline-rowspan-012.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    table {
+        vertical-align:baseline;
+    }
+    td {
+        vertical-align:baseline;
+        border: 1px solid #f90;
+    }
+    #img1 {
+        width: 100px;
+        height: 150px;
+        background-color: green;
+    }
+    #img2 {
+        background-color: green;
+    }
+</style>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  The bottom of the two green boxes should be aligned and there should be no space above the first box.</p>
+<table>
+    <tbody>
+        <tr>
+             <td><img id="img1" src="" id="img2" src=""
+        </tr>
+    </tbody>
+</table>
+<script>
+    var img = document.getElementById("img2");
+    img.src = "" 
+</script>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2-expected.html (from rev 145305, trunk/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2-expected.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2-expected.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2-expected.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90; padding: 0px;
+    }
+</style>
+<script>
+    function loadAndStall()
+    {
+        return "http://127.0.0.1:8000/resources/load-and-stall.php";
+    }
+    
+    function pngImage()
+    {
+        return "?name=../../../fast/css/resources/bikes.bmp&mimeType=image%bmp";
+    }
+    
+    function runTest()
+    {
+        var images = document.querySelectorAll("img");
+        for (var i = 0, len = images.length; i < len; i++) {
+            var image = images[i];
+            image.src = "" + pngImage() + "&stallAt=0&stallFor=0";
+        }
+    }
+</script>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  There should be no space between the top of the image and the orange border.</p>
+<table>
+    <tbody>
+        <tr>
+             <td><img></td><td><video autoplay></video></td>
+        </tr>
+        <tr>
+             <td><img></td><td><video autoplay></video></td>
+        </tr>
+    </tbody>
+    <script>runTest();</script>
+</table>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2.html (from rev 145305, trunk/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-2.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90; padding: 0px;
+    }
+    #super {
+        vertical-align:super;
+    }
+    #sub {
+        vertical-align:sub;
+    }
+</style>
+<script>
+    function loadAndStall()
+    {
+        return "http://127.0.0.1:8000/resources/load-and-stall.php";
+    }
+    
+    function pngImage()
+    {
+        return "?name=../../../fast/css/resources/bikes.bmp&mimeType=image%bmp";
+    }
+    
+    function runTest()
+    {
+        var images = document.querySelectorAll("img");
+        for (var i = 0, len = images.length; i < len; i++) {
+            var image = images[i];
+            image.src = "" + pngImage() + "&stallAt=0&stallFor=0";
+        }
+    }
+</script>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  There should be no space between the top of the image and the orange border.</p>
+<table>
+    <tbody>
+        <tr>
+             <td id="super"><img></td><td id="super"><video autoplay></video></td>
+        </tr>
+        <tr>
+             <td id="sub"><img></td><td id="sub"><video autoplay></video></td>
+        </tr>
+    </tbody>
+             <script>runTest();</script>
+</table>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3-expected.html (from rev 145305, trunk/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3-expected.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3-expected.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3-expected.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90; padding: 0px;
+    }
+</style>
+<script>
+    function loadAndStall()
+    {
+        return "http://127.0.0.1:8000/resources/load-and-stall.php";
+    }
+    
+    function pngImage()
+    {
+        return "?name=../../../fast/css/resources/bikes.bmp&mimeType=image%bmp";
+    }
+    
+    function runTest()
+    {
+        var images = document.querySelectorAll("img");
+        for (var i = 0, len = images.length; i < len; i++) {
+            var image = images[i];
+            image.src = "" + pngImage() + "&stallAt=0&stallFor=0";
+        }
+    }
+</script>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  There should be no space between the top of the image and the orange border.</p>
+<table>
+    <tbody>
+        <tr>
+             <td><img></td><td><video autoplay></video></td>
+        </tr>
+        <tr>
+             <td><img></td><td><video autoplay></video></td>
+        </tr>
+    </tbody>
+    <script>runTest();</script>
+</table>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3.html (from rev 145305, trunk/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-3.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90; padding: 0px;
+    }
+    #text-bottom {
+        vertical-align:text-bottom;
+    }
+    #text-top {
+        vertical-align:text-top;
+    }
+</style>
+<script>
+    function loadAndStall()
+    {
+        return "http://127.0.0.1:8000/resources/load-and-stall.php";
+    }
+    
+    function pngImage()
+    {
+        return "?name=../../../fast/css/resources/bikes.bmp&mimeType=image%bmp";
+    }
+    
+    function runTest()
+    {
+        var images = document.querySelectorAll("img");
+        for (var i = 0, len = images.length; i < len; i++) {
+            var image = images[i];
+            image.src = "" + pngImage() + "&stallAt=0&stallFor=0";
+        }
+    }
+</script>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  There should be no space between the top of the image and the orange border.</p>
+<table>
+    <tbody>
+        <tr>
+             <td id="text-bottom"><img></td><td id="text-bottom"><video autoplay></video></td>
+        </tr>
+        <tr>
+             <td id="text-top"><img></td><td id="text-top"><video autoplay></video></td>
+        </tr>
+    </tbody>
+             <script>runTest();</script>
+</table>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-expected.html (from rev 145305, trunk/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-expected.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-expected.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load-expected.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90; padding: 0px;
+    }
+</style>
+<script>
+    function loadAndStall()
+    {
+        return "http://127.0.0.1:8000/resources/load-and-stall.php";
+    }
+    
+    function pngImage()
+    {
+        return "?name=../../../fast/css/resources/bikes.bmp&mimeType=image%bmp";
+    }
+    
+    function runTest()
+    {
+        var images = document.querySelectorAll("img");
+        for (var i = 0, len = images.length; i < len; i++) {
+            var image = images[i];
+            image.src = "" + pngImage() + "&stallAt=0&stallFor=0";
+        }
+    }
+</script>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  There should be no space between the top of the image and the orange border.</p>
+<table>
+    <tbody>
+        <tr>
+             <td><img></td><td><video autoplay></video></td>
+        </tr>
+        <tr>
+             <td><img></td><td><video autoplay></video></td>
+        </tr>
+    </tbody>
+    <script>runTest();</script>
+</table>
+</body>
+</html>
+

Copied: branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load.html (from rev 145305, trunk/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load.html) (0 => 145576)


--- branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load.html	                        (rev 0)
+++ branches/chromium/1410/LayoutTests/http/tests/css/vertical-align-baseline-after-image-load.html	2013-03-12 20:20:02 UTC (rev 145576)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    td {
+        border: 1px solid #f90; padding: 0px;
+    }
+    #baseline {
+        vertical-align:baseline;
+    }
+    #top {
+        vertical-align:top;
+    }
+</style>
+<script>
+    function loadAndStall()
+    {
+        return "http://127.0.0.1:8000/resources/load-and-stall.php";
+    }
+    
+    function pngImage()
+    {
+        return "?name=../../../fast/css/resources/bikes.bmp&mimeType=image%bmp";
+    }
+    
+    function runTest()
+    {
+        var images = document.querySelectorAll("img");
+        for (var i = 0, len = images.length; i < len; i++) {
+            var image = images[i];
+            image.src = "" + pngImage() + "&stallAt=0&stallFor=0";
+        }
+    }
+</script>
+</head>
+<body>
+<p> https://bugs.webkit.org/show_bug.cgi?id=108357:  There should be no space between the top of the image and the orange border.</p>
+<table>
+    <tbody>
+        <tr>
+             <td id="baseline"><img></td><td id="baseline"><video autoplay></video></td>
+        </tr>
+        <tr>
+             <td id="top"><img></td><td id="top"><video autoplay></video></td>
+        </tr>
+    </tbody>
+             <script>runTest();</script>
+</table>
+</body>
+</html>
+

Modified: branches/chromium/1410/Source/WebCore/rendering/RenderTableCell.cpp (145575 => 145576)


--- branches/chromium/1410/Source/WebCore/rendering/RenderTableCell.cpp	2013-03-12 20:11:58 UTC (rev 145575)
+++ branches/chromium/1410/Source/WebCore/rendering/RenderTableCell.cpp	2013-03-12 20:20:02 UTC (rev 145576)
@@ -243,7 +243,21 @@
 {
     StackStats::LayoutCheckPoint layoutCheckPoint;
     updateFirstLetter();
+
+    int oldCellBaseline = cellBaselinePosition();
     layoutBlock(cellWidthChanged());
+
+    // If we have replaced content, the intrinsic height of our content may have changed since the last time we laid out. If that's the case the intrinsic padding we used
+    // for layout (the padding required to push the contents of the cell down to the row's baseline) is included in our new height and baseline and makes both
+    // of them wrong. So if our content's intrinsic height has changed push the new content up into the intrinsic padding and relayout so that the rest of
+    // table and row layout can use the correct baseline and height for this cell.
+    if (isBaselineAligned() && cellBaselinePosition() > section()->rowBaseline(rowIndex())) {
+        int newIntrinsicPaddingBefore = max<LayoutUnit>(0, intrinsicPaddingBefore() - max<LayoutUnit>(0, cellBaselinePosition() - oldCellBaseline));
+        setIntrinsicPaddingBefore(newIntrinsicPaddingBefore);
+        setNeedsLayout(true, MarkOnlyThis);
+        layoutBlock(cellWidthChanged());
+    }
+
     setCellWidthChanged(false);
 }
 

Modified: branches/chromium/1410/Source/WebCore/rendering/RenderTableCell.h (145575 => 145576)


--- branches/chromium/1410/Source/WebCore/rendering/RenderTableCell.h	2013-03-12 20:11:58 UTC (rev 145575)
+++ branches/chromium/1410/Source/WebCore/rendering/RenderTableCell.h	2013-03-12 20:20:02 UTC (rev 145576)
@@ -125,6 +125,11 @@
     void paintBackgroundsBehindCell(PaintInfo&, const LayoutPoint&, RenderObject* backgroundObject);
 
     LayoutUnit cellBaselinePosition() const;
+    bool isBaselineAligned() const 
+    { 
+        EVerticalAlign va = style()->verticalAlign();
+        return va == BASELINE || va == TEXT_BOTTOM || va == TEXT_TOP || va == SUPER || va == SUB || va == LENGTH; 
+    }
 
     void computeIntrinsicPadding(int rowHeight);
     void clearIntrinsicPadding() { setIntrinsicPadding(0, 0); }

Modified: branches/chromium/1410/Source/WebCore/rendering/RenderTableSection.cpp (145575 => 145576)


--- branches/chromium/1410/Source/WebCore/rendering/RenderTableSection.cpp	2013-03-12 20:11:58 UTC (rev 145575)
+++ branches/chromium/1410/Source/WebCore/rendering/RenderTableSection.cpp	2013-03-12 20:20:02 UTC (rev 145576)
@@ -319,8 +319,7 @@
                 m_rowPos[r + 1] = max(m_rowPos[r + 1], m_rowPos[cellStartRow] + cellLogicalHeight);
 
                 // 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) {
+                if (cell->isBaselineAligned()) {
                     LayoutUnit baselinePosition = cell->cellBaselinePosition();
                     if (baselinePosition > cell->borderBefore() + cell->paddingBefore()) {
                         m_grid[cellStartRow].baseline = max(m_grid[cellStartRow].baseline, baselinePosition);
@@ -583,8 +582,7 @@
                 cell->layoutIfNeeded();
 
                 // If the baseline moved, we may have to update the data for our row. Find out the new baseline.
-                EVerticalAlign va = cell->style()->verticalAlign();
-                if (va == BASELINE || va == TEXT_BOTTOM || va == TEXT_TOP || va == SUPER || va == SUB || va == LENGTH) {
+                if (cell->isBaselineAligned()) {
                     LayoutUnit baseline = cell->cellBaselinePosition();
                     if (baseline > cell->borderBefore() + cell->paddingBefore())
                         m_grid[r].baseline = max(m_grid[r].baseline, baseline);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to