Title: [185327] trunk
- Revision
- 185327
- Author
- [email protected]
- Date
- 2015-06-08 13:26:02 -0700 (Mon, 08 Jun 2015)
Log Message
[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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (185326 => 185327)
--- trunk/LayoutTests/ChangeLog 2015-06-08 19:55:23 UTC (rev 185326)
+++ trunk/LayoutTests/ChangeLog 2015-06-08 20:26:02 UTC (rev 185327)
@@ -1,3 +1,16 @@
+2015-06-08 Javier Fernandez <[email protected]>
+
+ [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.
+
2015-06-08 Brady Eidson <[email protected]>
Completely remove all IDB properties/constructors when it is disabled at runtime.
Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt (0 => 185327)
--- 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)
@@ -0,0 +1,4 @@
+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
Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html (0 => 185327)
--- 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)
@@ -0,0 +1,37 @@
+<!DOCTYPE HTML>
+<link href="" rel="stylesheet">
+<script src=""
+<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>
Modified: trunk/Source/WebCore/ChangeLog (185326 => 185327)
--- 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)
@@ -1,3 +1,23 @@
+2015-06-08 Javier Fernandez <[email protected]>
+
+ [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):
+
2015-06-08 Brady Eidson <[email protected]>
Completely remove all IDB properties/constructors when it is disabled at runtime.
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (185326 => 185327)
--- 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)
@@ -1296,25 +1296,25 @@
// FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child, LayoutUnit gridAreaBreadthForChild)
{
- if (RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch)
+ if (!allowedToStretchLogicalHeightForChild(child) || RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch) {
+ child.clearOverrideLogicalContentHeight();
return;
+ }
bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode();
- 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);
+ // 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);
- // 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();
- }
+ // 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();
}
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
