[CSS Grid Layout] Modify grid item height doesn't work
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Oct 2015 13:14:11 +0000 (13:14 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Oct 2015 13:14:11 +0000 (13:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149840

Reviewed by Sergio Villar Senin.

Source/WebCore:

When computing the logical height of content-sized grid tracks we
need to clear grid item's override height if it needs to be laid
out again.

Currently we are doing so only in the case of percentage heights
or when the grid track's width has changed; these situations would
obviously mark grid items as needing layout.

However, there are other situations, like the one defined in this
bug, which would imply a new layout of the grid items; hence we
need to clear its override value if we want the layout logic to be
computed correctly.

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::logicalContentHeightForChild):

LayoutTests:

Added new tests cases to verify content-sized grid tracks are resized
appropriately whenever grid item's height is changed.

* fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt: Added new test cases.
* fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html: Added new test cases.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190665 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt
LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGrid.cpp

index 64dcf66..f36d2a8 100644 (file)
@@ -1,3 +1,16 @@
+2015-10-07  Javier Fernandez  <jfernandez@igalia.com>
+
+        [CSS Grid Layout] Modify grid item height doesn't work
+        https://bugs.webkit.org/show_bug.cgi?id=149840
+
+        Reviewed by Sergio Villar Senin.
+
+        Added new tests cases to verify content-sized grid tracks are resized
+        appropriately whenever grid item's height is changed.
+
+        * fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt: Added new test cases.
+        * fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html: Added new test cases.
+
 2015-10-05  Sergio Villar Senin  <svillar@igalia.com>
 
         [css-grid] Implement grid gutters
index 52b82eb..c9124bb 100644 (file)
@@ -3,28 +3,31 @@
 <script src="../../resources/check-layout.js"></script>
 <style>
 .grid {
-    -webkit-grid-template: 200px 200px / 200px 200px;
     width: -webkit-fit-content;
     position: relative;
 }
+.content {
+    width: 200px;
+    height: 200px;
+}
 #fromFixedWidth { width: 150px; }
 #fromFixedHeight { height: 100px; }
 #fromMarginAuto { margin: auto; }
 </style>
 <p>The grids below had initially 'stretched' items, but we have changed 'height', 'width' 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 and width accordingly.</p>
-<div class="grid">
+<div class="grid" style="-webkit-grid-template: 200px 200px / 200px 200px;">
     <div id="toFixedHeight" class="firstRowFirstColumn" data-expected-width="200" data-expected-height="100"></div>
     <div class="firstRowSecondColumn" data-expected-width="200" data-expected-height="200"></div>
     <div class="secondRowFirstColumn" data-expected-width="200" data-expected-height="200"></div>
     <div id="fromFixedHeight" class="secondRowSecondColumn" data-expected-width="200" data-expected-height="200"></div>
 </div>
-<div class="grid">
+<div class="grid" style="-webkit-grid-template: 200px 200px / 200px 200px;">
     <div id="toFixedWidth" class="firstRowFirstColumn" data-expected-width="150" data-expected-height="200"></div>
     <div class="firstRowSecondColumn" data-expected-width="200" data-expected-height="200"></div>
     <div class="secondRowFirstColumn" data-expected-width="200" data-expected-height="200"></div>
     <div id="fromFixedWidth" class="secondRowSecondColumn" data-expected-width="200" data-expected-height="200"></div>
 </div>
-<div class="grid">
+<div class="grid" style="-webkit-grid-template: 200px 200px / 200px 200px;">
     <div id="toMarginAuto" class="firstRowFirstColumn" data-expected-width="150" data-expected-height="100">
         <div style="width: 150px; height: 100px"></div>
     </div>
         <div style="width: 150px; height: 100px"></div>
     </div>
 </div>
+<div class="grid">
+    <div id="contentSized-toFixedHeight" class="firstRowFirstColumn" data-expected-width="200" data-expected-height="100"></div>
+    <div class="firstRowSecondColumn" data-expected-width="200" data-expected-height="200"><div class="content"></div></div>
+    <div class="secondRowFirstColumn" data-expected-width="200" data-expected-height="200"><div class="content"></div></div>
+    <div id="contentSized-fromFixedHeight" class="secondRowSecondColumn" data-expected-width="200" data-expected-height="200"></div>
+</div>
+<div class="grid">
+    <div id="contentSized-toFixedWidth" class="firstRowFirstColumn" data-expected-width="150" data-expected-height="200"></div>
+    <div class="firstRowSecondColumn" data-expected-width="200" data-expected-height="200"><div class="content"></div></div>
+    <div class="secondRowFirstColumn" data-expected-width="200" data-expected-height="200"><div class="content"></div></div>
+    <div id="contentSized-fromFixedWidth" class="secondRowSecondColumn" data-expected-width="200" data-expected-height="200"></div>
+</div>
+<div class="grid">
+    <div id="contentSized-toMarginAuto" class="firstRowFirstColumn" data-expected-width="150" data-expected-height="100">
+        <div style="width: 150px; height: 100px"></div>
+    </div>
+    <div class="firstRowSecondColumn" data-expected-width="200" data-expected-height="200"><div class="content"></div></div>
+    <div class="secondRowFirstColumn" data-expected-width="200" data-expected-height="200"><div class="content"></div></div>
+    <div id="contentSized-fromMarginAuto" class="secondRowSecondColumn" data-expected-width="200" data-expected-height="200">
+        <div style="width: 150px; height: 100px"></div>
+    </div>
+</div>
 <script>
 document.body.offsetLeft;
 document.getElementById("fromFixedHeight").style.height = "auto";
@@ -42,5 +67,11 @@ document.getElementById("fromFixedWidth").style.width = "auto";
 document.getElementById("toFixedWidth").style.width = "150px";
 document.getElementById("fromMarginAuto").style.margin = "0";
 document.getElementById("toMarginAuto").style.margin = "auto";
+document.getElementById("contentSized-fromFixedHeight").style.height = "auto";
+document.getElementById("contentSized-toFixedHeight").style.height = "100px";
+document.getElementById("contentSized-fromFixedWidth").style.width = "auto";
+document.getElementById("contentSized-toFixedWidth").style.width = "150px";
+document.getElementById("contentSized-fromMarginAuto").style.margin = "0";
+document.getElementById("contentSized-toMarginAuto").style.margin = "auto";
 checkLayout(".grid");
 </script>
index 853dd1b..621eb70 100644 (file)
@@ -1,3 +1,26 @@
+2015-10-07  Javier Fernandez  <jfernandez@igalia.com>
+
+        [CSS Grid Layout] Modify grid item height doesn't work
+        https://bugs.webkit.org/show_bug.cgi?id=149840
+
+        Reviewed by Sergio Villar Senin.
+
+        When computing the logical height of content-sized grid tracks we
+        need to clear grid item's override height if it needs to be laid
+        out again.
+
+        Currently we are doing so only in the case of percentage heights
+        or when the grid track's width has changed; these situations would
+        obviously mark grid items as needing layout.
+
+        However, there are other situations, like the one defined in this
+        bug, which would imply a new layout of the grid items; hence we
+        need to clear its override value if we want the layout logic to be
+        computed correctly.
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::logicalContentHeightForChild):
+
 2015-10-07  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
         Automate WebCore JS builtins generation and build system
index 6b7c606..31c0d00 100644 (file)
@@ -590,11 +590,12 @@ LayoutUnit RenderGrid::logicalContentHeightForChild(RenderBox& child, Vector<Gri
 {
     Optional<LayoutUnit> oldOverrideContainingBlockContentLogicalWidth = child.hasOverrideContainingBlockLogicalWidth() ? child.overrideContainingBlockContentLogicalWidth() : LayoutUnit();
     LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(child, ForColumns, columnTracks);
-    if (child.hasRelativeLogicalHeight() || !oldOverrideContainingBlockContentLogicalWidth || oldOverrideContainingBlockContentLogicalWidth.value() != overrideContainingBlockContentLogicalWidth) {
+    if (child.hasRelativeLogicalHeight() || !oldOverrideContainingBlockContentLogicalWidth || oldOverrideContainingBlockContentLogicalWidth.value() != overrideContainingBlockContentLogicalWidth)
         child.setNeedsLayout(MarkOnlyThis);
-        // We need to clear the stretched height to properly compute logical height during layout.
+
+    // We need to clear the stretched height to properly compute logical height during layout.
+    if (child.needsLayout())
         child.clearOverrideLogicalContentHeight();
-    }
 
     child.setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
     // If |child| has a relative logical height, we shouldn't let it override its intrinsic height, which is