Title: [207663] trunk
Revision
207663
Author
jfernan...@igalia.com
Date
2016-10-21 01:11:14 -0700 (Fri, 21 Oct 2016)

Log Message

[css-grid] Content Alignment broken with indefinite sized grid container
https://bugs.webkit.org/show_bug.cgi?id=163724

Reviewed by Manuel Rego Casasnovas.

Source/WebCore:

The Grid Tracks sizing algorithm receives as parameter the
available space to be used as space for tracks. We hold a variable
to store the remaining free space for each dimension.

When the grid container size is indefinite we can't compute the
available free space after computing track sizes until such
indefinite size is resolved.

No new tests, just added some additional test cases.

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock): Compute freeSpace for Rows
after doing layout and resolving the indefinite height.

LayoutTests:

Added additional test cases to verify we compute properly the
available free space for content-alignment, handling correctly the
overflow when needed.

* fast/css-grid-layout/grid-content-alignment-overflow.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (207662 => 207663)


--- trunk/LayoutTests/ChangeLog	2016-10-21 08:04:43 UTC (rev 207662)
+++ trunk/LayoutTests/ChangeLog	2016-10-21 08:11:14 UTC (rev 207663)
@@ -1,3 +1,16 @@
+2016-10-21  Javier Fernandez  <jfernan...@igalia.com>
+
+        [css-grid] Content Alignment broken with indefinite sized grid container
+        https://bugs.webkit.org/show_bug.cgi?id=163724
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        Added additional test cases to verify we compute properly the
+        available free space for content-alignment, handling correctly the
+        overflow when needed.
+
+        * fast/css-grid-layout/grid-content-alignment-overflow.html:
+
 2016-10-21  Jer Noble  <jer.no...@apple.com>
 
         [mac-wk2 release] LayoutTest media/media-source/media-source-seek-detach-crash.html is a flaky failure

Modified: trunk/LayoutTests/fast/css-grid-layout/grid-content-alignment-overflow-expected.txt (207662 => 207663)


--- trunk/LayoutTests/fast/css-grid-layout/grid-content-alignment-overflow-expected.txt	2016-10-21 08:04:43 UTC (rev 207662)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-content-alignment-overflow-expected.txt	2016-10-21 08:11:14 UTC (rev 207663)
@@ -1,10 +1,59 @@
 This test checks that the 'overflow' keyword is applied correctly for Content Alignment properties.
 
 PASS
+Grid container width of 60px not enough for 2 column tracks of 50px.
+Content-Alignment: center and Overflow-Alignment: default
+
 PASS
+Grid container height of 150px not enough for 2 row tracks of 100px.
+Content-Alignment: center and Overflow-Alignment: unsafe
+
 PASS
+Grid container width of 60px not enough for 2 column tracks of 50px.
+Content-Alignment: center and Overflow-Alignment: safe
+
 PASS
+Grid container height of 150px not enough for 2 row tracks of 100px.
+Content-Alignment: center and Overflow-Alignment: safe
+
 PASS
+Grid container width of 60px not enough for 2 column tracks of 50px.
+Content-Alignment: end and Overflow-Alignment: default
+
 PASS
+Grid container height of 150px not enough for 2 row tracks of 100px.
+Content-Alignment: end and Overflow-Alignment: unsafe
+
 PASS
+Grid container width of 60px not enough for 2 column tracks of 50px.
+Content-Alignment: end and Overflow-Alignment: safe
+
 PASS
+Grid container height of 150px not enough for 2 row tracks of 100px.
+Content-Alignment: end and Overflow-Alignment: safe
+
+PASS
+Grid container indefinite size using fit-content, hence no possible overflow.
+Content-Alignment: end and Overflow-Alignment: unsafe
+
+PASS
+Grid container indefinite size using fit-content, hence no possible overflow.
+Content-Alignment: center and Overflow-Alignment: unsafe
+
+PASS
+Grid container indefinite size using fit-content, but max-size constraints implies that the content-sized tracks overflow.
+Content-Alignment: end and Overflow-Alignment: unsafe
+
+PASS
+Grid container indefinite size using fit-content, but max-size constraints implies that the content-sized tracks overflow.
+Content-Alignment: center and Overflow-Alignment: unsafe
+
+PASS
+Grid container indefinite size using fit-content, but min-size constraints implies that the content-sized tracks don't overflow.
+Content-Alignment: end and Overflow-Alignment: unsafe
+
+PASS
+Grid container indefinite size using fit-content, but min-size constraints implies that the content-sized tracks don't overflow.
+Content-Alignment: center and Overflow-Alignment: unsafe
+
+

Modified: trunk/LayoutTests/fast/css-grid-layout/grid-content-alignment-overflow.html (207662 => 207663)


--- trunk/LayoutTests/fast/css-grid-layout/grid-content-alignment-overflow.html	2016-10-21 08:04:43 UTC (rev 207662)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-content-alignment-overflow.html	2016-10-21 08:11:14 UTC (rev 207663)
@@ -3,11 +3,16 @@
 <head>
 <link href="" rel="stylesheet">
 <link href="" rel="stylesheet">
+<link href="" rel="stylesheet">
 <script src=""
 <style>
 body {
     margin: 0;
 }
+.container {
+    position: relative;
+    float: left;
+}
 
 .grid {
     grid-template-columns: 50px 50px;
@@ -14,6 +19,10 @@
     grid-template-rows: 100px 100px;
 }
 
+.contentSizedTracks {
+    grid-template:  -webkit-max-content 100px / -webkit-max-content 50px;
+}
+
 .overflowWidth {
     width: 60px;
     height: 300px;
@@ -23,6 +32,24 @@
     width: 200px;
     height: 150px;
 }
+
+.item1 {
+   width: 50px;
+   height: 150px;
+}
+.item2 {
+   width: 150px;
+   height: 100px;
+}
+
+.minSize {
+   min-width: 300px;
+   min-height: 400px;
+}
+.maxSize {
+   max-width: 100px;
+   max-height: 100px;
+}
 </style>
 </head>
 <body _onload_="checkLayout('.grid')">
@@ -29,7 +56,7 @@
 
 <p>This test checks that the 'overflow' keyword is applied correctly for Content Alignment properties.</p>
 
-<div style="position: relative">
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
     <div class="grid overflowWidth contentCenter" data-expected-width="60" data-expected-height="300">
         <div class="firstRowFirstColumn" data-offset-x="-20" data-offset-y="50" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="-20" data-offset-y="150" data-expected-width="50" data-expected-height="100"></div>
@@ -37,8 +64,11 @@
         <div class="secondRowSecondColumn" data-offset-x="30" data-offset-y="150" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container width of 60px not enough for 2 column tracks of 50px.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>default</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
     <div class="grid overflowHeight contentCenterUnsafe" data-expected-width="200" data-expected-height="150">
         <div class="firstRowFirstColumn" data-offset-x="50" data-offset-y="-25" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="50" data-offset-y="75" data-expected-width="50" data-expected-height="100"></div>
@@ -46,8 +76,11 @@
         <div class="secondRowSecondColumn" data-offset-x="100" data-offset-y="75" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container height of 150px not enough for 2 row tracks of 100px.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>unsafe</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
     <div class="grid overflowWidth contentCenterSafe" data-expected-width="60" data-expected-height="300">
         <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="50" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="0" data-offset-y="150" data-expected-width="50" data-expected-height="100"></div>
@@ -55,8 +88,11 @@
         <div class="secondRowSecondColumn" data-offset-x="50" data-offset-y="150" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container width of 60px not enough for 2 column tracks of 50px.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>safe</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 75px; margin-right: 25px;">
     <div class="grid overflowHeight contentCenterSafe" data-expected-width="200" data-expected-height="150">
         <div class="firstRowFirstColumn" data-offset-x="50" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="50" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
@@ -64,8 +100,11 @@
         <div class="secondRowSecondColumn" data-offset-x="100" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container height of 150px not enough for 2 row tracks of 100px.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>safe</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
     <div class="grid overflowWidth contentEnd" data-expected-width="60" data-expected-height="300">
         <div class="firstRowFirstColumn" data-offset-x="-40" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="-40" data-offset-y="200" data-expected-width="50" data-expected-height="100"></div>
@@ -73,8 +112,11 @@
         <div class="secondRowSecondColumn" data-offset-x="10" data-offset-y="200" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container width of 60px not enough for 2 column tracks of 50px.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>default</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
     <div class="grid overflowHeight contentEndUnsafe" data-expected-width="200" data-expected-height="150">
         <div class="firstRowFirstColumn" data-offset-x="100" data-offset-y="-50" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="100" data-offset-y="50" data-expected-width="50" data-expected-height="100"></div>
@@ -82,8 +124,11 @@
         <div class="secondRowSecondColumn" data-offset-x="150" data-offset-y="50" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container height of 150px not enough for 2 row tracks of 100px.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>unsafe</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
     <div class="grid overflowWidth contentEndSafe" data-expected-width="60" data-expected-height="300">
         <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="0" data-offset-y="200" data-expected-width="50" data-expected-height="100"></div>
@@ -91,8 +136,11 @@
         <div class="secondRowSecondColumn" data-offset-x="50" data-offset-y="200" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container width of 60px not enough for 2 column tracks of 50px.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>safe</b></div>
 
-<div style="position: relative">
+<br clear="all">
+
+<div class="container" style="margin-bottom: 75px; margin-right: 25px;">
     <div class="grid overflowHeight contentEndSafe" data-expected-width="200" data-expected-height="150">
         <div class="firstRowFirstColumn" data-offset-x="100" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
         <div class="secondRowFirstColumn" data-offset-x="100" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
@@ -100,6 +148,73 @@
         <div class="secondRowSecondColumn" data-offset-x="150" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
     </div>
 </div>
+<div>Grid container height of 150px not enough for 2 row tracks of 100px.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>safe</b></div>
 
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
+    <div class="grid fit-content contentEndUnsafe" data-expected-width="100" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
+        <div class="secondRowFirstColumn" data-offset-x="0" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
+        <div class="firstRowSecondColumn" data-offset-x="50" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
+        <div class="secondRowSecondColumn" data-offset-x="50" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
+    </div>
+</div>
+<div clas="title">Grid container indefinite size using fit-content, hence no possible overflow.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>unsafe</b></div>
+
+<br clear="all">
+
+<div class="container" style="margin-bottom: 200px; margin-right: 25px;">
+    <div class="grid fit-content contentCenterUnsafe" data-expected-width="100" data-expected-height="200">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
+        <div class="secondRowFirstColumn" data-offset-x="0" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
+        <div class="firstRowSecondColumn" data-offset-x="50" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
+        <div class="secondRowSecondColumn" data-offset-x="50" data-offset-y="100" data-expected-width="50" data-expected-height="100"></div>
+    </div>
+</div>
+<div>Grid container indefinite size using fit-content, hence no possible overflow.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>unsafe</b></div>
+
+<br clear="all">
+
+<div class="container" style="margin-bottom: 100px; margin-right: 25px;">
+    <div class="grid contentSizedTracks fit-content maxSize contentEndUnsafe" data-expected-width="100" data-expected-height="100">
+        <div class="item1 firstRowSecondColumn" data-offset-x="50" data-offset-y="-150" data-expected-width="50" data-expected-height="150"></div>
+        <div class="item2 secondRowFirstColumn" data-offset-x="-100" data-offset-y="0" data-expected-width="150" data-expected-height="100"></div>
+    </div>
+</div>
+<div>Grid container indefinite size using fit-content, but max-size constraints implies that the content-sized tracks overflow.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>unsafe</b></div>
+
+<br clear="all">
+
+<div class="container" style="margin-bottom: 100px; margin-right: 75px;">
+    <div class="grid contentSizedTracks fit-content maxSize contentCenterUnsafe" data-expected-width="100" data-expected-height="100">
+        <div class="item1 firstRowSecondColumn" data-offset-x="100" data-offset-y="-75" data-expected-width="50" data-expected-height="150"></div>
+        <div class="item2 secondRowFirstColumn" data-offset-x="-50" data-offset-y="75" data-expected-width="150" data-expected-height="100"></div>
+    </div>
+</div>
+<div>Grid container indefinite size using fit-content, but max-size constraints implies that the content-sized tracks overflow.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>unsafe</b></div>
+
+<br clear="all">
+
+<div class="container" style="margin-bottom: 50px; margin-right: 25px;">
+    <div class="grid contentSizedTracks fit-content minSize contentEndUnsafe" data-expected-width="300" data-expected-height="400">
+        <div class="item1 firstRowSecondColumn" data-offset-x="250" data-offset-y="150" data-expected-width="50" data-expected-height="150"></div>
+        <div class="item2 secondRowFirstColumn" data-offset-x="100" data-offset-y="300" data-expected-width="150" data-expected-height="100"></div>
+    </div>
+</div>
+<div>Grid container indefinite size using fit-content, but min-size constraints implies that the content-sized tracks don't overflow.<br> Content-Alignment: <b>end</b> and Overflow-Alignment: <b>unsafe</b></div>
+
+<br clear="all">
+
+<div class="container" style="margin-right: 25px;">
+    <div class="grid contentSizedTracks fit-content minSize contentCenterUnsafe" data-expected-width="300" data-expected-height="400">
+        <div class="item1 firstRowSecondColumn" data-offset-x="200" data-offset-y="75" data-expected-width="50" data-expected-height="150"></div>
+        <div class="item2 secondRowFirstColumn" data-offset-x="50" data-offset-y="225" data-expected-width="150" data-expected-height="100"></div>
+    </div>
+</div>
+<div>Grid container indefinite size using fit-content, but min-size constraints implies that the content-sized tracks don't overflow.<br> Content-Alignment: <b>center</b> and Overflow-Alignment: <b>unsafe</b></div>
+
+<br clear="all">
+
 </body>
 </html>

Modified: trunk/Source/WebCore/ChangeLog (207662 => 207663)


--- trunk/Source/WebCore/ChangeLog	2016-10-21 08:04:43 UTC (rev 207662)
+++ trunk/Source/WebCore/ChangeLog	2016-10-21 08:11:14 UTC (rev 207663)
@@ -1,3 +1,24 @@
+2016-10-21  Javier Fernandez  <jfernan...@igalia.com>
+
+        [css-grid] Content Alignment broken with indefinite sized grid container
+        https://bugs.webkit.org/show_bug.cgi?id=163724
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        The Grid Tracks sizing algorithm receives as parameter the
+        available space to be used as space for tracks. We hold a variable
+        to store the remaining free space for each dimension.
+
+        When the grid container size is indefinite we can't compute the
+        available free space after computing track sizes until such
+        indefinite size is resolved.
+
+        No new tests, just added some additional test cases.
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::layoutBlock): Compute freeSpace for Rows
+        after doing layout and resolving the indefinite height.
+
 2016-10-21  Jer Noble  <jer.no...@apple.com>
 
         CRASH in SourceBuffer::sourceBufferPrivateDidReceiveSample + 2169

Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (207662 => 207663)


--- trunk/Source/WebCore/rendering/RenderGrid.cpp	2016-10-21 08:04:43 UTC (rev 207662)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp	2016-10-21 08:11:14 UTC (rev 207663)
@@ -494,11 +494,17 @@
         computeIntrinsicLogicalHeight(sizingData);
     else
         computeTrackSizesForDirection(ForRows, sizingData, availableLogicalHeight(ExcludeMarginBorderPadding));
-    setLogicalHeight(computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight());
+    LayoutUnit trackBasedLogicalHeight = computeTrackBasedLogicalHeight(sizingData) + borderAndPaddingLogicalHeight() + scrollbarLogicalHeight();
+    setLogicalHeight(trackBasedLogicalHeight);
 
     LayoutUnit oldClientAfterEdge = clientLogicalBottom();
     updateLogicalHeight();
 
+    // Once grid's indefinite height is resolved, we can compute the
+    // available free space for Content Alignment.
+    if (!hasDefiniteLogicalHeight)
+        sizingData.setFreeSpace(ForRows, logicalHeight() - trackBasedLogicalHeight);
+
     // 3- If the min-content contribution of any grid items have changed based on the row
     // sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content
     // contribution (once only).
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to