<!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>[185327] 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/185327">185327</a></dd>
<dt>Author</dt> <dd>jfernandez@igalia.com</dd>
<dt>Date</dt> <dd>2015-06-08 13:26:02 -0700 (Mon, 08 Jun 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[CSS Grid Layout] Setting height on a grid item doesn't have any effect
https://bugs.webkit.org/show_bug.cgi?id=145604
Reviewed by Sergio Villar Senin.
Source/WebCore:
Box Alignment spec states that stretch is only possible when height is
'auto' and no 'auto' margins are used.
It might be the case that style changes so that stretching is not allowed,
hence we need to detect it and clear the override height the stretching
algorithm previously set. The new layout triggered by the style change
will then set grid item's height according to the new style rules.
Test: fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
LayoutTests:
Tests to verify that we clear the override height set by the stretching logic
whenever height or margin change in a way they don't allow stretching anymore.
* fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt: Added.
* fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.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="#trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangeexpectedtxt">trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangehtml">trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (185326 => 185327)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-06-08 19:55:23 UTC (rev 185326)
+++ trunk/LayoutTests/ChangeLog 2015-06-08 20:26:02 UTC (rev 185327)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2015-06-08 Javier Fernandez <jfernandez@igalia.com>
+
+ [CSS Grid Layout] Setting height on a grid item doesn't have any effect
+ https://bugs.webkit.org/show_bug.cgi?id=145604
+
+ Reviewed by Sergio Villar Senin.
+
+ Tests to verify that we clear the override height set by the stretching logic
+ whenever height or margin change in a way they don't allow stretching anymore.
+
+ * fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt: Added.
+ * fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html: Added.
+
</ins><span class="cx"> 2015-06-08 Brady Eidson <beidson@apple.com>
</span><span class="cx">
</span><span class="cx"> Completely remove all IDB properties/constructors when it is disabled at runtime.
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangeexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt (0 => 185327)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt 2015-06-08 20:26:02 UTC (rev 185327)
</span><span class="lines">@@ -0,0 +1,4 @@
</span><ins>+The grids below had initially 'stretched' items, but we have changed 'height' and 'margin' to values which don't allow stretch. This test verifies that the layout algorithm properly detects such changes and clear the override height accordingly.
+
+PASS
+PASS
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssgridlayoutgriditemshouldnotbestretchedwhenheightormarginchangehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html (0 => 185327)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html 2015-06-08 20:26:02 UTC (rev 185327)
</span><span class="lines">@@ -0,0 +1,37 @@
</span><ins>+<!DOCTYPE HTML>
+<link href="resources/grid.css" rel="stylesheet">
+<script src="../../resources/check-layout.js"></script>
+<style>
+.grid {
+ -webkit-grid-template: 200px 200px / 200px 200px;
+ width: -webkit-fit-content;
+ position: relative;
+}
+#fromFixedHeight { height: 100px; }
+#fromMarginAuto { margin: auto; }
+</style>
+<p>The grids below had initially 'stretched' items, but we have changed 'height' and 'margin' to values which don't allow stretch. This test verifies that the layout algorithm properly detects such changes and clear the override height accordingly.</p>
+<div class="grid">
+ <div id="toFixedHeight" class="firstRowFirstColumn" data-expected-height="100"></div>
+ <div class="firstRowSecondColumn" data-expected-height="200"></div>
+ <div class="secondRowFirstColumn" data-expected-height="200"></div>
+ <div id="fromFixedHeight" class="secondRowSecondColumn" data-expected-height="200"></div>
+</div>
+<div class="grid">
+ <div id="toMarginAuto" class="firstRowFirstColumn" data-expected-height="100">
+ <div style="height: 100px"></div>
+ </div>
+ <div class="firstRowSecondColumn" data-expected-height="200"></div>
+ <div class="secondRowFirstColumn" data-expected-height="200"></div>
+ <div id="fromMarginAuto" class="secondRowSecondColumn" data-expected-height="200">
+ <div style="height: 100px"></div>
+ </div>
+</div>
+<script>
+document.body.offsetLeft;
+document.getElementById("fromFixedHeight").style.height = "auto";
+document.getElementById("toFixedHeight").style.height = "100px";
+document.getElementById("fromMarginAuto").style.margin = "0";
+document.getElementById("toMarginAuto").style.margin = "auto";
+checkLayout(".grid");
+</script>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (185326 => 185327)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-06-08 19:55:23 UTC (rev 185326)
+++ trunk/Source/WebCore/ChangeLog 2015-06-08 20:26:02 UTC (rev 185327)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2015-06-08 Javier Fernandez <jfernandez@igalia.com>
+
+ [CSS Grid Layout] Setting height on a grid item doesn't have any effect
+ https://bugs.webkit.org/show_bug.cgi?id=145604
+
+ Reviewed by Sergio Villar Senin.
+
+ Box Alignment spec states that stretch is only possible when height is
+ 'auto' and no 'auto' margins are used.
+
+ It might be the case that style changes so that stretching is not allowed,
+ hence we need to detect it and clear the override height the stretching
+ algorithm previously set. The new layout triggered by the style change
+ will then set grid item's height according to the new style rules.
+
+ Test: fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
+
</ins><span class="cx"> 2015-06-08 Brady Eidson <beidson@apple.com>
</span><span class="cx">
</span><span class="cx"> Completely remove all IDB properties/constructors when it is disabled at runtime.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (185326 => 185327)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2015-06-08 19:55:23 UTC (rev 185326)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2015-06-08 20:26:02 UTC (rev 185327)
</span><span class="lines">@@ -1296,25 +1296,25 @@
</span><span class="cx"> // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
</span><span class="cx"> void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child, LayoutUnit gridAreaBreadthForChild)
</span><span class="cx"> {
</span><del>- if (RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch)
</del><ins>+ if (!allowedToStretchLogicalHeightForChild(child) || RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch) {
+ child.clearOverrideLogicalContentHeight();
</ins><span class="cx"> return;
</span><ins>+ }
</ins><span class="cx">
</span><span class="cx"> bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
</span><del>- if (allowedToStretchLogicalHeightForChild(child)) {
- // FIXME: If the child has orthogonal flow, then it already has an override height set, so use it.
- // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
- if (!hasOrthogonalWritingMode) {
- LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(gridAreaBreadthForChild, child);
- LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight);
</del><ins>+ // FIXME: If the child has orthogonal flow, then it already has an override height set, so use it.
+ // FIXME: grid track sizing and positioning do not support orthogonal modes yet.
+ if (!hasOrthogonalWritingMode) {
+ LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(gridAreaBreadthForChild, child);
+ LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight);
</ins><span class="cx">
</span><del>- // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
- bool childNeedsRelayout = desiredLogicalHeight != child.logicalHeight();
- if (childNeedsRelayout || !child.hasOverrideLogicalContentHeight())
- child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight());
- if (childNeedsRelayout) {
- child.setLogicalHeight(0);
- child.setNeedsLayout();
- }
</del><ins>+ // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
+ bool childNeedsRelayout = desiredLogicalHeight != child.logicalHeight();
+ if (childNeedsRelayout || !child.hasOverrideLogicalContentHeight())
+ child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight());
+ if (childNeedsRelayout) {
+ child.setLogicalHeight(0);
+ child.setNeedsLayout();
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>