Attachment 254483 Details for Bug 145604 – Patch

[patch] Patch

bug-145604-20150608121858.patch (text/plain), 8.08 KB, created by Javier Fernandez on 2015-06-08 03:19:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Javier Fernandez

Created: 2015-06-08 03:19:48 PDT

Size: 8.08 KB

patch

obsolete

>Subversion Revision: 185072
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ddf49b73d70a1246df2bc1315314d17ae5470cd9..4bdc215a017f654d7833a3112d366bd5fbd8e0d8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+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 NOBODY (OOPS!).
>+
>+        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-01  Benjamin Poulain  <bpoulain@apple.com>
> 
>         [CSS JIT] Fail to compile when we are out of executable memory
>diff --git a/Source/WebCore/rendering/RenderGrid.cpp b/Source/WebCore/rendering/RenderGrid.cpp
>index 3bd26a5e77bae0093768373675e16622ad53742d..bc378a533b8a32ad4c57463725c21507199e2377 100644
>--- a/Source/WebCore/rendering/RenderGrid.cpp
>+++ b/Source/WebCore/rendering/RenderGrid.cpp
>@@ -1296,25 +1296,25 @@ 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)
> {
>-    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: 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: 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();
>         }
>     }
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 792e2152457286b723d1db5b447e601bfd5756a2..539b460a966e841fe3fa8567ca92b4dce763827f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+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 NOBODY (OOPS!).
>+
>+        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-01  Brady Eidson  <beidson@apple.com>
> 
>         Add WKTR support for "should open external URLs".
>diff --git a/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d950e0e0fb3cd0af743f357e7bca3c780b7fbb7a
>--- /dev/null
>+++ b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt
>@@ -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
>diff --git a/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..7b4f30f8acd418b76c07f3a77787394dffa7a882
>--- /dev/null
>+++ b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html
>@@ -0,0 +1,37 @@
>+<!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>

Actions: View | Formatted Diff | Diff

Attachments on bug 145604: 254173 | 254181 | 254483