[CSS Grid Layout] Do not stretch always grid items with auto width
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Aug 2015 17:03:05 +0000 (17:03 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Aug 2015 17:03:05 +0000 (17:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148069

Reviewed by Darin Adler.

Source/WebCore:

We assumed that any grid item with 'auto' width will be stretched
to fill all the available space in its grid area. We assumed this
because grid area acts as item's container.

However, Grid Layout specification states on its Alignment section
that items will be stretched by default, unless either
justify-self or align-self compute to a value other than stretch
or margins are auto. In those cases, grid items will auto-size to
fit their contents.

Tests:
fast/css-grid-layout/grid-align-justify-stretch.html
fast/css-grid-layout/grid-item-auto-margins-and-stretch.html
fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::computeLogicalWidthInRegion):
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutGridItems):
(WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
(WebCore::RenderGrid::needToStretchChildLogicalHeight): Deleted.
* rendering/RenderGrid.h:

LayoutTests:

New Layout tests to verify that grid stretching logic works as expected
when aligning both horizontally and vertically.

* fast/css-grid-layout/grid-align-justify-stretch-expected.txt: Added.
* fast/css-grid-layout/grid-align-justify-stretch.html: Added.
* fast/css-grid-layout/grid-item-auto-margins-and-stretch-expected.txt: Added.
* fast/css-grid-layout/grid-item-auto-margins-and-stretch.html: Added.
* fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt: Added.
* fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-align-justify-stretch.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-auto-margins-and-stretch-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-item-auto-margins-and-stretch.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderGrid.cpp
Source/WebCore/rendering/RenderGrid.h

index 47750f6..99c60c0 100644 (file)
@@ -1,3 +1,20 @@
+2015-08-18  Javier Fernandez  <jfernandez@igalia.com>
+
+        [CSS Grid Layout] Do not stretch always grid items with auto width
+        https://bugs.webkit.org/show_bug.cgi?id=148069
+
+        Reviewed by Darin Adler.
+
+        New Layout tests to verify that grid stretching logic works as expected
+        when aligning both horizontally and vertically.
+
+        * fast/css-grid-layout/grid-align-justify-stretch-expected.txt: Added.
+        * fast/css-grid-layout/grid-align-justify-stretch.html: Added.
+        * fast/css-grid-layout/grid-item-auto-margins-and-stretch-expected.txt: Added.
+        * fast/css-grid-layout/grid-item-auto-margins-and-stretch.html: Added.
+        * fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt: Added.
+        * fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html: Added.
+
 2015-08-18  Xabier Rodriguez Calvar  <calvaris@igalia.com>
 
         [Streams API] Update implementation with the latest spec
diff --git a/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-expected.txt b/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch-expected.txt
new file mode 100644 (file)
index 0000000..9911dff
--- /dev/null
@@ -0,0 +1,18 @@
+This test checks that the 'stretch' value is applied correctly for 'align' and 'justify' properties.
+
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch.html b/LayoutTests/fast/css-grid-layout/grid-align-justify-stretch.html
new file mode 100644 (file)
index 0000000..2aa2c47
--- /dev/null
@@ -0,0 +1,249 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<script src="../../resources/check-layout.js"></script>
+<style>
+body {
+    margin: 0;
+}
+
+.grid {
+    -webkit-grid-template-columns: 100px 100px;
+    -webkit-grid-template-rows: 200px 200px;
+    width: -webkit-fit-content;
+    margin-bottom: 20px;
+}
+
+.widthAndHeightSet {
+    width: 20px;
+    height: 40px;
+}
+
+.onlyWidthSet {
+    width: 20px;
+}
+
+.onlyHeightSet {
+    height: 40px;
+}
+
+.maxHeight {
+    max-height: 160px;
+}
+
+.maxWidth {
+    max-width: 90px;
+}
+
+.minWidth {
+    min-width: 120px;
+}
+
+.minHeight {
+    min-height: 220px;
+}
+
+.alignItemsAuto {
+    align-items: auto;
+}
+
+.alignItemsStretch {
+    align-items: stretch;
+}
+
+.alignItemsStart {
+    align-items: start;
+}
+
+.alignSelfAuto {
+    align-self: auto;
+}
+
+.alignSelfStretch {
+    align-self: stretch;
+}
+
+.alignSelfStart {
+    align-self: start;
+}
+
+.justifyItemsAuto {
+    justify-items: auto;
+}
+
+.justifyItemsStretch {
+    justify-items: stretch;
+}
+
+.justifyItemsStart {
+    justify-items: start;
+}
+
+.justifySelfAuto {
+    justify-self: auto;
+}
+
+.justifySelfStretch {
+    justify-self: stretch;
+}
+
+.justifySelfStart {
+    justify-self: start;
+}
+</style>
+</head>
+<body onload="checkLayout('.grid')">
+
+<p>This test checks that the 'stretch' value is applied correctly for 'align' and 'justify' properties.</p>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="alignSelfStretch justifySelfStart firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="0" data-expected-height="200"></div>
+        <div class="widthAndHeightSet alignSelfStretch justifySelfStart firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet alignSelfStretch justifySelfStart secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet alignSelfStretch justifySelfStart secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="0" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="alignSelfStart justifySelfStretch firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="0"></div>
+        <div class="widthAndHeightSet alignSelfStart justifySelfStretch firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet alignSelfStart justifySelfStretch secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="0"></div>
+        <div class="onlyHeightSet alignSelfStart justifySelfStretch secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="maxHeight alignSelfStretch justifySelfStart firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="0" data-expected-height="160"></div>
+        <div class="minWidth widthAndHeightSet alignSelfStretch justifySelfStart firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="120" data-expected-height="40"></div>
+        <div class="minHeight onlyWidthSet alignSelfStretch justifySelfStart secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="220"></div>
+        <div class="maxWidth onlyHeightSet alignSelfStretch justifySelfStretch secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="90" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="alignSelfStretch justifySelfStretch firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="widthAndHeightSet alignSelfStretch justifySelfStretch firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet alignSelfStretch justifySelfStretch secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet alignSelfStretch justifySelfStretch secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid alignItemsStretch justifyItemsStart" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="0" data-expected-height="200"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="0" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid alignItemsStart justifyItemsStretch" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="0"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="0"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid alignItemsStretch justifyItemsStretch" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid alignItemsAuto justifyItemsAuto" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="firstRowSecondColumn" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="100" data-expected-height="200"></div>
+        <div class="secondRowSecondColumn" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="200"></div>
+    </div>
+</div>
+
+<!-- RTL direction (it should not affect the block-flow direction). -->
+<div style="position: relative">
+    <div class="grid directionRTL alignItemsStretch justifyItemsStretch" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="80" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="180" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="0" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<!-- RTL direction (it should not affect the block-flow) with opposite directions grid container vs grid item. -->
+<div style="position: relative">
+    <div class="grid alignItemsStretch justifyItemsStretch" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn  directionRTL" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn  directionRTL" data-offset-x="100" data-offset-y="0" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn  directionRTL" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200"></div>
+        <div class="onlyHeightSet secondRowSecondColumn  directionRTL" data-offset-x="100" data-offset-y="200" data-expected-width="100" data-expected-height="40"></div>
+    </div>
+</div>
+
+<!-- Vertical RL writing mode. -->
+<div style="position: relative">
+    <div class="grid verticalRL alignItemsStretch justifyItemsStretch" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="200" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="380" data-offset-y="100" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="180" data-offset-y="0" data-expected-width="20" data-expected-height="100"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="0" data-offset-y="100" data-expected-width="200" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid verticalRL" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="200" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+        <div class="firstRowSecondColumn" data-offset-x="200" data-offset-y="100" data-expected-width="200" data-expected-height="100"></div>
+        <div class="secondRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+        <div class="secondRowSecondColumn" data-offset-x="0" data-offset-y="100" data-expected-width="200" data-expected-height="100"></div>
+    </div>
+</div>
+
+<!-- Vertical LR writing mode. -->
+<div style="position: relative">
+    <div class="grid verticalLR alignItemsStretch justifyItemsStretch" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+        <div class="widthAndHeightSet firstRowSecondColumn" data-offset-x="0" data-offset-y="100" data-expected-width="20" data-expected-height="40"></div>
+        <div class="onlyWidthSet secondRowFirstColumn" data-offset-x="200" data-offset-y="0" data-expected-width="20" data-expected-height="100"></div>
+        <div class="onlyHeightSet secondRowSecondColumn" data-offset-x="200" data-offset-y="100" data-expected-width="200" data-expected-height="40"></div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid verticalLR" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+        <div class="firstRowSecondColumn" data-offset-x="0" data-offset-y="100" data-expected-width="200" data-expected-height="100"></div>
+        <div class="secondRowFirstColumn" data-offset-x="200" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+        <div class="secondRowSecondColumn" data-offset-x="200" data-offset-y="100" data-expected-width="200" data-expected-height="100"></div>
+    </div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-auto-margins-and-stretch-expected.txt b/LayoutTests/fast/css-grid-layout/grid-item-auto-margins-and-stretch-expected.txt
new file mode 100644 (file)
index 0000000..2697303
--- /dev/null
@@ -0,0 +1,8 @@
+This test checks that the 'stretch' value is only applied if neither of its margins (in the appropriate axis) are 'auto'.
+
+PASS
+PASS
+PASS
+PASS
+PASS
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/grid-item-auto-margins-and-stretch.html b/LayoutTests/fast/css-grid-layout/grid-item-auto-margins-and-stretch.html
new file mode 100644 (file)
index 0000000..1cd3a7b
--- /dev/null
@@ -0,0 +1,153 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="resources/grid.css" rel="stylesheet">
+<script src="../../resources/check-layout.js"></script>
+<style>
+body {
+    margin: 0;
+}
+
+.grid {
+    -webkit-grid-template-columns: 100px 100px;
+    -webkit-grid-template-rows: 200px 200px;
+    width: -webkit-fit-content;
+    margin-bottom: 20px;
+    font: 15px/1 Ahem;
+}
+
+.autoMarginsTopBottom {
+    margin: auto 0px;
+}
+
+.autoMarginsRightLeft {
+    margin: 0px auto;
+}
+
+.autoMargins {
+    margin: auto auto;
+}
+
+.onlyWidthSet {
+    width: 20px;
+}
+
+.onlyHeightSet {
+    height: 40px;
+}
+
+.widthAndHeightSet {
+    width: 20px;
+    height: 40px;
+}
+</style>
+</head>
+<body onload="checkLayout('.grid')">
+
+<p>This test checks that the 'stretch' value is only applied if neither of its margins (in the appropriate axis) are 'auto'.</p>
+
+<div style="position: relative">
+    <div class="grid" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="200">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMargins firstRowSecondColumn" data-offset-x="100" data-offset-y="80" data-expected-width="20" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsRightLeft secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="20" data-expected-height="200">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsTopBottom secondRowSecondColumn" data-offset-x="100" data-offset-y="280" data-expected-width="100" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid verticalRL" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="200" data-offset-y="0" data-expected-width="200" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMargins firstRowSecondColumn" data-offset-x="290" data-offset-y="100" data-expected-width="20" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsRightLeft secondRowFirstColumn" data-offset-x="90" data-offset-y="0" data-expected-width="20" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsTopBottom secondRowSecondColumn" data-offset-x="0" data-offset-y="100" data-expected-width="200" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid verticalLR" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMargins firstRowSecondColumn" data-offset-x="90" data-offset-y="100" data-expected-width="20" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsRightLeft secondRowFirstColumn" data-offset-x="290" data-offset-y="0" data-expected-width="20" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsTopBottom secondRowSecondColumn" data-offset-x="200" data-offset-y="100" data-expected-width="200" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+    </div>
+</div>
+
+<!-- RTL direction -->
+<div style="position: relative">
+    <div class="grid directionRTL" data-expected-width="200" data-expected-height="400">
+        <div class="firstRowFirstColumn" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="200">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMargins firstRowSecondColumn" data-offset-x="80" data-offset-y="80" data-expected-width="20" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsRightLeft secondRowFirstColumn" data-offset-x="180" data-offset-y="200" data-expected-width="20" data-expected-height="200">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsTopBottom secondRowSecondColumn" data-offset-x="0" data-offset-y="280" data-expected-width="100" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid directionRTL verticalRL" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="200" data-offset-y="100" data-expected-width="200" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMargins firstRowSecondColumn" data-offset-x="290" data-offset-y="60" data-expected-width="20" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsRightLeft secondRowFirstColumn" data-offset-x="90" data-offset-y="100" data-expected-width="20" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsTopBottom secondRowSecondColumn" data-offset-x="0" data-offset-y="60" data-expected-width="200" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+    </div>
+</div>
+
+<div style="position: relative">
+    <div class="grid  directionRTL verticalLR" data-expected-width="400" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="100" data-expected-width="200" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMargins firstRowSecondColumn" data-offset-x="90" data-offset-y="60" data-expected-width="20" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsRightLeft secondRowFirstColumn" data-offset-x="290" data-offset-y="100" data-expected-width="20" data-expected-height="100">
+            <div class="widthAndHeightSet"></div>
+        </div>
+        <div class="autoMarginsTopBottom secondRowSecondColumn" data-offset-x="200" data-offset-y="60" data-expected-width="200" data-expected-height="40">
+            <div class="widthAndHeightSet"></div>
+        </div>
+    </div>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt b/LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change-expected.txt
new file mode 100644 (file)
index 0000000..65f4411
--- /dev/null
@@ -0,0 +1,5 @@
+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.
+
+PASS
+PASS
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html b/LayoutTests/fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html
new file mode 100644 (file)
index 0000000..52b82eb
--- /dev/null
@@ -0,0 +1,46 @@
+<!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;
+}
+#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 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 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 id="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>
+    <div class="secondRowFirstColumn" data-expected-width="200" data-expected-height="200"></div>
+    <div id="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";
+document.getElementById("toFixedHeight").style.height = "100px";
+document.getElementById("fromFixedWidth").style.width = "auto";
+document.getElementById("toFixedWidth").style.width = "150px";
+document.getElementById("fromMarginAuto").style.margin = "0";
+document.getElementById("toMarginAuto").style.margin = "auto";
+checkLayout(".grid");
+</script>
index 59a8dce..f413718 100644 (file)
@@ -1,3 +1,33 @@
+2015-08-18  Javier Fernandez  <jfernandez@igalia.com>
+
+        [CSS Grid Layout] Do not stretch always grid items with auto width
+        https://bugs.webkit.org/show_bug.cgi?id=148069
+
+        Reviewed by Darin Adler.
+
+        We assumed that any grid item with 'auto' width will be stretched
+        to fill all the available space in its grid area. We assumed this
+        because grid area acts as item's container.
+
+        However, Grid Layout specification states on its Alignment section
+        that items will be stretched by default, unless either
+        justify-self or align-self compute to a value other than stretch
+        or margins are auto. In those cases, grid items will auto-size to
+        fit their contents.
+
+        Tests:
+        fast/css-grid-layout/grid-align-justify-stretch.html
+        fast/css-grid-layout/grid-item-auto-margins-and-stretch.html
+        fast/css-grid-layout/grid-items-should-not-be-stretched-when-height-or-width-or-margin-change.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computeLogicalWidthInRegion):
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::layoutGridItems):
+        (WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded):
+        (WebCore::RenderGrid::needToStretchChildLogicalHeight): Deleted.
+        * rendering/RenderGrid.h:
+
 2015-08-18  Xabier Rodriguez Calvar  <calvaris@igalia.com>
 
         [Streams API] Update implementation with the latest spec
index e4706ad..81e59d0 100644 (file)
@@ -2338,7 +2338,7 @@ void RenderBox::computeLogicalWidthInRegion(LogicalExtentComputedValues& compute
     // width.  Use the width from the style context.
     // FIXME: Account for block-flow in flexible boxes.
     // https://bugs.webkit.org/show_bug.cgi?id=46418
-    if (hasOverrideLogicalContentWidth() && (isRubyRun() || style().borderFit() == BorderFitLines || parent()->isFlexibleBoxIncludingDeprecated())) {
+    if (hasOverrideLogicalContentWidth() && (isRubyRun() || style().borderFit() == BorderFitLines || (parent()->isFlexibleBoxIncludingDeprecated() || parent()->isRenderGrid()))) {
         computedValues.m_extent = overrideLogicalContentWidth() + borderAndPaddingLogicalWidth();
         return;
     }
index 6cab13e..14e7416 100644 (file)
@@ -1190,7 +1190,7 @@ void RenderGrid::layoutGridItems()
         // Stretching logic might force a child layout, so we need to run it before the layoutIfNeeded
         // call to avoid unnecessary relayouts. This might imply that child margins, needed to correctly
         // determine the available space before stretching, are not set yet.
-        applyStretchAlignmentToChildIfNeeded(*child, overrideContainingBlockContentLogicalHeight);
+        applyStretchAlignmentToChildIfNeeded(*child);
 
         child->layoutIfNeeded();
 
@@ -1271,11 +1271,6 @@ static inline LayoutUnit computeOverflowAlignmentOffset(OverflowAlignment overfl
     return 0;
 }
 
-bool RenderGrid::allowedToStretchLogicalHeightForChild(const RenderBox& child) const
-{
-    return child.style().logicalHeight().isAuto() && !child.style().marginBeforeUsing(&style()).isAuto() && !child.style().marginAfterUsing(&style()).isAuto();
-}
-
 // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
 bool RenderGrid::needToStretchChildLogicalHeight(const RenderBox& child) const
 {
@@ -1297,27 +1292,46 @@ LayoutUnit RenderGrid::availableAlignmentSpaceForChildBeforeStretching(LayoutUni
 }
 
 // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
-void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child, LayoutUnit gridAreaBreadthForChild)
+void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child)
 {
-    if (!allowedToStretchLogicalHeightForChild(child) || RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch) {
-        child.clearOverrideLogicalContentHeight();
-        return;
+    // We clear both width and height override values because we will decide now whether they
+    // are allowed or not, evaluating the conditions which might have changed since the old
+    // values were set.
+    child.clearOverrideSize();
+
+    auto& gridStyle = style();
+    auto& childStyle = child.style();
+    bool isHorizontalMode = isHorizontalWritingMode();
+    bool hasAutoSizeInRowAxis = isHorizontalMode ? childStyle.width().isAuto() : childStyle.height().isAuto();
+    bool allowedToStretchChildAlongRowAxis = hasAutoSizeInRowAxis && !childStyle.marginStartUsing(&gridStyle).isAuto() && !childStyle.marginEndUsing(&gridStyle).isAuto();
+    if (!allowedToStretchChildAlongRowAxis || RenderStyle::resolveJustification(gridStyle, childStyle, ItemPositionStretch) != ItemPositionStretch) {
+        bool hasAutoMinSizeInRowAxis = isHorizontalMode ? childStyle.minWidth().isAuto() : childStyle.minHeight().isAuto();
+        bool canShrinkToFitInRowAxisForChild = !hasAutoMinSizeInRowAxis || child.minPreferredLogicalWidth() <= child.overrideContainingBlockContentLogicalWidth();
+        // TODO(lajava): how to handle orthogonality in this case ?.
+        // TODO(lajava): grid track sizing and positioning do not support orthogonal modes yet.
+        if (hasAutoSizeInRowAxis && canShrinkToFitInRowAxisForChild) {
+            LayoutUnit childWidthToFitContent = std::max(std::min(child.maxPreferredLogicalWidth(), child.overrideContainingBlockContentLogicalWidth()  - child.marginLogicalWidth()), child.minPreferredLogicalWidth());
+            LayoutUnit desiredLogicalWidth = child.constrainLogicalHeightByMinMax(childWidthToFitContent, -1);
+            child.setOverrideLogicalContentWidth(desiredLogicalWidth - child.borderAndPaddingLogicalWidth());
+            if (desiredLogicalWidth != child.logicalWidth())
+                child.setNeedsLayout();
+        }
     }
 
-    bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
-    // 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, -1);
-
-        // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
-        bool childNeedsRelayout = desiredLogicalHeight != child.logicalHeight();
-        if (childNeedsRelayout || !child.hasOverrideLogicalContentHeight())
+    bool hasAutoSizeInColumnAxis = isHorizontalMode ? childStyle.height().isAuto() : childStyle.width().isAuto();
+    bool allowedToStretchChildAlongColumnAxis = hasAutoSizeInColumnAxis && !childStyle.marginBeforeUsing(&gridStyle).isAuto() && !childStyle.marginAfterUsing(&gridStyle).isAuto();
+    if (allowedToStretchChildAlongColumnAxis && RenderStyle::resolveAlignment(gridStyle, childStyle, ItemPositionStretch) == ItemPositionStretch) {
+        // TODO (lajava): If the child has orthogonal flow, then it already has an override height set, so use it.
+        // TODO (lajava): grid track sizing and positioning do not support orthogonal modes yet.
+        if (child.isHorizontalWritingMode() == isHorizontalMode) {
+            LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(child.overrideContainingBlockContentLogicalHeight(), child);
+            LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight, -1);
             child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight());
-        if (childNeedsRelayout) {
-            child.setLogicalHeight(0);
-            child.setNeedsLayout();
+            if (desiredLogicalHeight != child.logicalHeight()) {
+                // TODO (lajava): Can avoid laying out here in some cases. See https://webkit.org/b/87905.
+                child.setLogicalHeight(0);
+                child.setNeedsLayout();
+            }
         }
     }
 }
index c3bda56..2863fb8 100644 (file)
@@ -130,12 +130,11 @@ private:
     LayoutUnit gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection, const Vector<GridTrack>&) const;
 
     virtual void paintChildren(PaintInfo& forSelf, const LayoutPoint& paintOffset, PaintInfo& forChild, bool usePrintRect) override;
-    bool allowedToStretchLogicalHeightForChild(const RenderBox&) const;
     bool needToStretchChildLogicalHeight(const RenderBox&) const;
     LayoutUnit marginLogicalHeightForChild(const RenderBox&) const;
     LayoutUnit computeMarginLogicalHeightForChild(const RenderBox&) const;
     LayoutUnit availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const RenderBox&) const;
-    void applyStretchAlignmentToChildIfNeeded(RenderBox&, LayoutUnit gridAreaBreadthForChild);
+    void applyStretchAlignmentToChildIfNeeded(RenderBox&);
     bool hasAutoMarginsInColumnAxis(const RenderBox&) const;
     bool hasAutoMarginsInRowAxis(const RenderBox&) const;
     void updateAutoMarginsInColumnAxisIfNeeded(RenderBox&, LayoutUnit gridAreaBreadthForChild);