<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[164214] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/164214">164214</a></dd>
<dt>Author</dt> <dd>rego@igalia.com</dd>
<dt>Date</dt> <dd>2014-02-17 02:57:43 -0800 (Mon, 17 Feb 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>[CSS Grid Layout] Fix missing layout in flexible and content sized columns
https://bugs.webkit.org/show_bug.cgi?id=128672
Reviewed by Sergio Villar Senin.
Source/WebCore:
RenderGrid::logicalContentHeightForChild() is called for some items at the beginning of RenderGrid::layoutGridItems()
from RenderGrid::computeUsedBreadthOfGridTracks(). This causes that the comparison inside the for loop in
RenderGrid::layoutGridItems() does not detect width changes, so elements won't be marked as needsLayout.
So the comparison is done in RenderGrid::logicalContentHeightForChild() and the element is marked to perform a layout if
the width has changed.
The issue can be reproduced easily with a simple grid with one flexible or content sized column, all the available width
is not used. On top of that, when you resize the window the flexible or content sized columns are not updating their
size properly.
CSS Grid Layout perftest results are around 4% worse, which is expected as we're adding a missing layout.
Test: fast/css-grid-layout/flex-content-sized-column-use-available-width.html
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::logicalContentHeightForChild): Check width changes and mark element as needed layout if required.
LayoutTests:
Add test that reproduce the issue for both cases flexible and content sized columns.
* fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html: Added.
* fast/css-grid-layout/flex-content-sized-column-use-available-width.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderGridcpp">trunk/Source/WebCore/rendering/RenderGrid.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssgridlayoutflexcontentsizedcolumnuseavailablewidthexpectedhtml">trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutflexcontentsizedcolumnuseavailablewidthhtml">trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (164213 => 164214)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-02-17 10:54:24 UTC (rev 164213)
+++ trunk/LayoutTests/ChangeLog 2014-02-17 10:57:43 UTC (rev 164214)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2014-02-17 Manuel Rego Casasnovas <rego@igalia.com>
+
+ [CSS Grid Layout] Fix missing layout in flexible and content sized columns
+ https://bugs.webkit.org/show_bug.cgi?id=128672
+
+ Reviewed by Sergio Villar Senin.
+
+ Add test that reproduce the issue for both cases flexible and content sized columns.
+
+ * fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html: Added.
+ * fast/css-grid-layout/flex-content-sized-column-use-available-width.html: Added.
+
</ins><span class="cx"> 2014-02-15 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> FTL should inline polymorphic heap accesses
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutflexcontentsizedcolumnuseavailablewidthexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html (0 => 164214)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html 2014-02-17 10:57:43 UTC (rev 164214)
</span><span class="lines">@@ -0,0 +1,27 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+ <style type="text/css">
+ .item {
+ background-color: blue;
+ }
+ </style>
+</head>
+<body>
+ <h1>Description</h1>
+ <p>Flex and content sized column should use all the available width and you should not see the grid background (grey at the end).</p>
+ <h1>Grid 1 flex column</h1>
+ <div>
+ <div class="item">grid item</div>
+ </div>
+ <h1>Grid 1 auto column</h1>
+ <div>
+ <div class="item">
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ </div>
+ </div>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutflexcontentsizedcolumnuseavailablewidthhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html (0 => 164214)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html 2014-02-17 10:57:43 UTC (rev 164214)
</span><span class="lines">@@ -0,0 +1,36 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ if (window.testRunner)
+ testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
+ </script>
+ <link href="resources/grid.css" rel="stylesheet">
+ <style type="text/css">
+ #grid-1 {
+ -webkit-grid-template-columns: 1fr;
+ }
+
+ #grid-2 {
+ -webkit-grid-template-columns: auto;
+ }
+ </style>
+</head>
+<body>
+ <h1>Description</h1>
+ <p>Flex and content sized column should use all the available width and you should not see the grid background (grey at the end).</p>
+ <h1>Grid 1 flex column</h1>
+ <div id="grid-1" class="grid">
+ <div class="firstRowFirstColumn">grid item</div>
+ </div>
+ <h1>Grid 1 auto column</h1>
+ <div id="grid-2" class="grid">
+ <div class="firstRowFirstColumn">
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ </div>
+ </div>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (164213 => 164214)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-02-17 10:54:24 UTC (rev 164213)
+++ trunk/Source/WebCore/ChangeLog 2014-02-17 10:57:43 UTC (rev 164214)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2014-02-17 Manuel Rego Casasnovas <rego@igalia.com>
+
+ [CSS Grid Layout] Fix missing layout in flexible and content sized columns
+ https://bugs.webkit.org/show_bug.cgi?id=128672
+
+ Reviewed by Sergio Villar Senin.
+
+ RenderGrid::logicalContentHeightForChild() is called for some items at the beginning of RenderGrid::layoutGridItems()
+ from RenderGrid::computeUsedBreadthOfGridTracks(). This causes that the comparison inside the for loop in
+ RenderGrid::layoutGridItems() does not detect width changes, so elements won't be marked as needsLayout.
+
+ So the comparison is done in RenderGrid::logicalContentHeightForChild() and the element is marked to perform a layout if
+ the width has changed.
+
+ The issue can be reproduced easily with a simple grid with one flexible or content sized column, all the available width
+ is not used. On top of that, when you resize the window the flexible or content sized columns are not updating their
+ size properly.
+
+ CSS Grid Layout perftest results are around 4% worse, which is expected as we're adding a missing layout.
+
+ Test: fast/css-grid-layout/flex-content-sized-column-use-available-width.html
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::logicalContentHeightForChild): Check width changes and mark element as needed layout if required.
+
</ins><span class="cx"> 2014-02-16 Andreas Kling <akling@apple.com>
</span><span class="cx">
</span><span class="cx"> Ensure that removing an iframe from the DOM tree disconnects its Frame.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (164213 => 164214)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2014-02-17 10:54:24 UTC (rev 164213)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2014-02-17 10:57:43 UTC (rev 164214)
</span><span class="lines">@@ -424,10 +424,12 @@
</span><span class="cx">
</span><span class="cx"> LayoutUnit RenderGrid::logicalContentHeightForChild(RenderBox* child, Vector<GridTrack>& columnTracks)
</span><span class="cx"> {
</span><del>- if (child->style().logicalHeight().isPercent())
</del><ins>+ LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
+ LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(child, ForColumns, columnTracks);
+ if (child->style().logicalHeight().isPercent() || oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth)
</ins><span class="cx"> child->setNeedsLayout(MarkOnlyThis);
</span><span class="cx">
</span><del>- child->setOverrideContainingBlockContentLogicalWidth(gridAreaBreadthForChild(child, ForColumns, columnTracks));
</del><ins>+ child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
</ins><span class="cx"> // If |child| has a percentage logical height, we shouldn't let it override its intrinsic height, which is
</span><span class="cx"> // what we are interested in here. Thus we need to set the override logical height to -1 (no possible resolution).
</span><span class="cx"> child->setOverrideContainingBlockContentLogicalHeight(-1);
</span></span></pre>
</div>
</div>
</body>
</html>