Source/WebCore: [CSS Grid Layout] Support paddings and margins on grid items
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Dec 2012 02:24:23 +0000 (02:24 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Dec 2012 02:24:23 +0000 (02:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=103677

Reviewed by Tony Chang.

After bug 102968, we properly resolve grid items' width and height against the
grid areas' sizes. However we didn't check for paddings and margins, which is
what this change fixes..

Test: fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::computeLogicalWidthInRegion):
Don't stretch the end margin to match the containing block's extent.
The fix is similar to what was done for flex-box in bug 65887.

* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::computeStickyPositionConstraints):
Added a comment about not using containingBlockLogicalWidthForContent.

(WebCore::RenderBoxModelObject::computedCSSPaddingTop):
(WebCore::RenderBoxModelObject::computedCSSPaddingBottom):
(WebCore::RenderBoxModelObject::computedCSSPaddingLeft):
(WebCore::RenderBoxModelObject::computedCSSPaddingRight):
(WebCore::RenderBoxModelObject::computedCSSPaddingBefore):
(WebCore::RenderBoxModelObject::computedCSSPaddingAfter):
(WebCore::RenderBoxModelObject::computedCSSPaddingStart):
(WebCore::RenderBoxModelObject::computedCSSPaddingEnd):
Updated these functions to use containingBlockLogicalWidthForContent.

* rendering/RenderGrid.h:
* rendering/RenderObject.h:
(WebCore::RenderObject::isRenderGrid):
Added isRenderGrid.

LayoutTests: [CSS Grid Layout] Support percentage paddings and margins on grid items
https://bugs.webkit.org/show_bug.cgi?id=103677

Reviewed by Tony Chang.

* resources/check-layout.js:
Extended check-layout to be able to query paddings and margins. Note that in order to compare,
the attribute with the returned value from getComputedStyle, we need to trim the unit ("px")
from the actual values. This trick also works in FireFox and Opera.

* fast/css-grid-layout/percent-padding-margin-resolution-grid-item-expected.txt: Added.
* fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/percent-padding-margin-resolution-grid-item-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html [new file with mode: 0644]
LayoutTests/resources/check-layout.js
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderGrid.h
Source/WebCore/rendering/RenderObject.h

index 44b431c..a365663 100644 (file)
@@ -1,3 +1,18 @@
+2012-12-03  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        [CSS Grid Layout] Support percentage paddings and margins on grid items
+        https://bugs.webkit.org/show_bug.cgi?id=103677
+
+        Reviewed by Tony Chang.
+
+        * resources/check-layout.js:
+        Extended check-layout to be able to query paddings and margins. Note that in order to compare,
+        the attribute with the returned value from getComputedStyle, we need to trim the unit ("px")
+        from the actual values. This trick also works in FireFox and Opera.
+
+        * fast/css-grid-layout/percent-padding-margin-resolution-grid-item-expected.txt: Added.
+        * fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html: Added.
+
 2012-12-03  Roger Fong  <roger_fong@apple.com>
 
         Unreviewed. Expected failing results on Windows.
diff --git a/LayoutTests/fast/css-grid-layout/percent-padding-margin-resolution-grid-item-expected.txt b/LayoutTests/fast/css-grid-layout/percent-padding-margin-resolution-grid-item-expected.txt
new file mode 100644 (file)
index 0000000..afbedc8
--- /dev/null
@@ -0,0 +1,6 @@
+Test that resolving percentage padding and margin on grid items works properly on a fixed grid with different writing modes.
+
+PASS
+PASS
+PASS
+PASS
diff --git a/LayoutTests/fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html b/LayoutTests/fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html
new file mode 100644 (file)
index 0000000..eb57648
--- /dev/null
@@ -0,0 +1,98 @@
+<!DOCTYPE html>
+<html>
+<script>
+if (window.testRunner)
+    testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
+</script>
+<style>
+.grid {
+    display: -webkit-grid;
+    background-color: grey;
+    -webkit-grid-columns: 100px 300px;
+    -webkit-grid-rows: 50px 150px;
+    height: 200px;
+    width: 400px;
+}
+
+#a {
+    background-color: blue;
+    -webkit-grid-column: 1;
+    -webkit-grid-row: 1;
+}
+
+#b {
+    background-color: lime;
+    -webkit-grid-column: 2;
+    -webkit-grid-row: 1;
+}
+
+#c {
+    background-color: purple;
+    -webkit-grid-column: 1;
+    -webkit-grid-row: 2;
+}
+
+#d {
+    background-color: orange;
+    -webkit-grid-column: 2;
+    -webkit-grid-row: 2;
+}
+
+.percentPadding {
+    width: 0px;
+    height: 0px;
+    padding: 50%;
+}
+
+.percentMargin {
+    width: 0px;
+    height: 0px;
+    margin: 50%;
+}
+
+.percentPaddingAndMargin {
+    width: 0px;
+    height: 0px;
+    padding: 10%;
+    margin: 20%;
+}
+
+.verticalRL {
+    -webkit-writing-mode: vertical-rl;
+}
+</style>
+<script src="../../resources/check-layout.js"></script>
+<body onload="checkLayout('.grid')">
+
+<p>Test that resolving percentage padding and margin on grid items works properly on a fixed grid with different writing modes.</p>
+
+<div class="grid">
+    <div id="a" class="percentPadding" data-expected-padding-top="50" data-expected-padding-right="50" data-expected-padding-bottom="50" data-expected-padding-left="50"></div>
+    <div id="b" class="percentMargin" data-expected-margin-top="150" data-expected-margin-right="150" data-expected-margin-bottom="150" data-expected-margin-left="150"></div>
+    <div id="c" class="percentPaddingAndMargin" data-expected-padding-top="10" data-expected-padding-right="10" data-expected-padding-bottom="10" data-expected-padding-left="10" data-expected-margin-top="20" data-expected-margin-right="20" data-expected-margin-bottom="20" data-expected-margin-left="20"></div>
+    <div id="d" class="percentPaddingAndMargin" data-expected-padding-top="30" data-expected-padding-right="30" data-expected-padding-bottom="30" data-expected-padding-left="30" data-expected-margin-top="60" data-expected-margin-right="60" data-expected-margin-bottom="60" data-expected-margin-left="60"></div>
+</div>
+
+<div class="grid verticalRL">
+    <div id="a" class="percentPadding" data-expected-padding-top="50" data-expected-padding-right="50" data-expected-padding-bottom="50" data-expected-padding-left="50"></div>
+    <div id="b" class="percentMargin" data-expected-margin-top="150" data-expected-margin-right="150" data-expected-margin-bottom="150" data-expected-margin-left="150"></div>
+    <div id="c" class="percentPaddingAndMargin" data-expected-padding-top="10" data-expected-padding-right="10" data-expected-padding-bottom="10" data-expected-padding-left="10" data-expected-margin-top="20" data-expected-margin-right="20" data-expected-margin-bottom="20" data-expected-margin-left="20"></div>
+    <div id="d" class="percentPaddingAndMargin" data-expected-padding-top="30" data-expected-padding-right="30" data-expected-padding-bottom="30" data-expected-padding-left="30" data-expected-margin-top="60" data-expected-margin-right="60" data-expected-margin-bottom="60" data-expected-margin-left="60"></div>
+</div>
+
+<div class="grid">
+    <div id="a" class="percentPadding verticalRL" data-expected-padding-top="50" data-expected-padding-right="50" data-expected-padding-bottom="50" data-expected-padding-left="50"></div>
+    <div id="b" class="percentMargin verticalRL" data-expected-margin-top="150" data-expected-margin-right="150" data-expected-margin-bottom="150" data-expected-margin-left="150"></div>
+    <div id="c" class="percentPaddingAndMargin verticalRL" data-expected-padding-top="10" data-expected-padding-right="10" data-expected-padding-bottom="10" data-expected-padding-left="10" data-expected-margin-top="20" data-expected-margin-right="20" data-expected-margin-bottom="20" data-expected-margin-left="20"></div>
+    <div id="d" class="percentPaddingAndMargin verticalRL" data-expected-padding-top="30" data-expected-padding-right="30" data-expected-padding-bottom="30" data-expected-padding-left="30" data-expected-margin-top="60" data-expected-margin-right="60" data-expected-margin-bottom="60" data-expected-margin-left="60"></div>
+</div>
+
+<div class="grid">
+    <div id="a" class="percentPadding verticalRL" data-expected-padding-top="50" data-expected-padding-right="50" data-expected-padding-bottom="50" data-expected-padding-left="50"></div>
+    <div id="b" class="percentMargin" data-expected-margin-top="150" data-expected-margin-right="150" data-expected-margin-bottom="150" data-expected-margin-left="150"></div>
+    <div id="c" class="percentPaddingAndMargin verticalRL" data-expected-padding-top="10" data-expected-padding-right="10" data-expected-padding-bottom="10" data-expected-padding-left="10" data-expected-margin-top="20" data-expected-margin-right="20" data-expected-margin-bottom="20" data-expected-margin-left="20"></div>
+    <div id="d" class="percentPaddingAndMargin" data-expected-padding-top="30" data-expected-padding-right="30" data-expected-padding-bottom="30" data-expected-padding-left="30" data-expected-margin-top="60" data-expected-margin-right="60" data-expected-margin-bottom="60" data-expected-margin-left="60"></div>
+</div>
+
+</body>
+</html>
index f587647..2b1f540 100644 (file)
@@ -77,6 +77,78 @@ function checkExpectedValues(node, failures)
         if (actualDisplay != expectedDisplay)
             failures.push("Expected " + expectedDisplay + " for display, but got " + actualDisplay + ". ");
     }
+
+    var expectedPaddingTop = node.getAttribute && node.getAttribute("data-expected-padding-top");
+    if (expectedPaddingTop) {
+        var actualPaddingTop = getComputedStyle(node).paddingTop;
+        // Trim the unit "px" from the output.
+        actualPaddingTop = actualPaddingTop.substring(0, actualPaddingTop.length - 2);
+        if (actualPaddingTop != expectedPaddingTop)
+            failures.push("Expected " + expectedPaddingTop + " for padding-top, but got " + actualPaddingTop + ". ");
+    }
+
+    var expectedPaddingBottom = node.getAttribute && node.getAttribute("data-expected-padding-bottom");
+    if (expectedPaddingBottom) {
+        var actualPaddingBottom = getComputedStyle(node).paddingBottom;
+        // Trim the unit "px" from the output.
+        actualPaddingBottom = actualPaddingBottom.substring(0, actualPaddingBottom.length - 2);
+        if (actualPaddingBottom != expectedPaddingBottom)
+            failures.push("Expected " + expectedPaddingBottom + " for padding-bottom, but got " + actualPaddingBottom + ". ");
+    }
+
+    var expectedPaddingLeft = node.getAttribute && node.getAttribute("data-expected-padding-left");
+    if (expectedPaddingLeft) {
+        var actualPaddingLeft = getComputedStyle(node).paddingLeft;
+        // Trim the unit "px" from the output.
+        actualPaddingLeft = actualPaddingLeft.substring(0, actualPaddingLeft.length - 2);
+        if (actualPaddingLeft != expectedPaddingLeft)
+            failures.push("Expected " + expectedPaddingLeft + " for padding-left, but got " + actualPaddingLeft + ". ");
+    }
+
+    var expectedPaddingRight = node.getAttribute && node.getAttribute("data-expected-padding-right");
+    if (expectedPaddingRight) {
+        var actualPaddingRight = getComputedStyle(node).paddingRight;
+        // Trim the unit "px" from the output.
+        actualPaddingRight = actualPaddingRight.substring(0, actualPaddingRight.length - 2);
+        if (actualPaddingRight != expectedPaddingRight)
+            failures.push("Expected " + expectedPaddingRight + " for padding-right, but got " + actualPaddingRight + ". ");
+    }
+
+    var expectedMarginTop = node.getAttribute && node.getAttribute("data-expected-margin-top");
+    if (expectedMarginTop) {
+        var actualMarginTop = getComputedStyle(node).marginTop;
+        // Trim the unit "px" from the output.
+        actualMarginTop = actualMarginTop.substring(0, actualMarginTop.length - 2);
+        if (actualMarginTop != expectedMarginTop)
+            failures.push("Expected " + expectedMarginTop + " for margin-top, but got " + actualMarginTop + ". ");
+    }
+
+    var expectedMarginBottom = node.getAttribute && node.getAttribute("data-expected-margin-bottom");
+    if (expectedMarginBottom) {
+        var actualMarginBottom = getComputedStyle(node).marginBottom;
+        // Trim the unit "px" from the output.
+        actualMarginBottom = actualMarginBottom.substring(0, actualMarginBottom.length - 2);
+        if (actualMarginBottom != expectedMarginBottom)
+            failures.push("Expected " + expectedMarginBottom + " for margin-bottom, but got " + actualMarginBottom + ". ");
+    }
+
+    var expectedMarginLeft = node.getAttribute && node.getAttribute("data-expected-margin-left");
+    if (expectedMarginLeft) {
+        var actualMarginLeft = getComputedStyle(node).marginLeft;
+        // Trim the unit "px" from the output.
+        actualMarginLeft = actualMarginLeft.substring(0, actualMarginLeft.length - 2);
+        if (actualMarginLeft != expectedMarginLeft)
+            failures.push("Expected " + expectedMarginLeft + " for margin-left, but got " + actualMarginLeft + ". ");
+    }
+
+    var expectedMarginRight = node.getAttribute && node.getAttribute("data-expected-margin-right");
+    if (expectedMarginRight) {
+        var actualMarginRight = getComputedStyle(node).marginRight;
+        // Trim the unit "px" from the output.
+        actualMarginRight = actualMarginRight.substring(0, actualMarginRight.length - 2);
+        if (actualMarginRight != expectedMarginRight)
+            failures.push("Expected " + expectedMarginRight + " for margin-right, but got " + actualMarginRight + ". ");
+    }
 }
 
 window.checkLayout = function(selectorList)
index ae540c8..002883a 100644 (file)
@@ -1,3 +1,40 @@
+2012-12-03  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        [CSS Grid Layout] Support paddings and margins on grid items
+        https://bugs.webkit.org/show_bug.cgi?id=103677
+
+        Reviewed by Tony Chang.
+
+        After bug 102968, we properly resolve grid items' width and height against the
+        grid areas' sizes. However we didn't check for paddings and margins, which is
+        what this change fixes..
+
+        Test: fast/css-grid-layout/percent-padding-margin-resolution-grid-item.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computeLogicalWidthInRegion):
+        Don't stretch the end margin to match the containing block's extent.
+        The fix is similar to what was done for flex-box in bug 65887.
+
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::computeStickyPositionConstraints):
+        Added a comment about not using containingBlockLogicalWidthForContent.
+
+        (WebCore::RenderBoxModelObject::computedCSSPaddingTop):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingBottom):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingLeft):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingRight):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingBefore):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingAfter):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingStart):
+        (WebCore::RenderBoxModelObject::computedCSSPaddingEnd):
+        Updated these functions to use containingBlockLogicalWidthForContent.
+
+        * rendering/RenderGrid.h:
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::isRenderGrid):
+        Added isRenderGrid.
+
 2012-12-03  Scott Violet  <sky@chromium.org>
 
         [chromium] Remove linux theme related files and switch to default
index 79c4b10..ad28428 100644 (file)
@@ -1829,7 +1829,7 @@ void RenderBox::computeLogicalWidthInRegion(LogicalExtentComputedValues& compute
     }
     
     if (!hasPerpendicularContainingBlock && containerLogicalWidth && containerLogicalWidth != (computedValues.m_extent + computedValues.m_margins.m_start + computedValues.m_margins.m_end)
-            && !isFloating() && !isInline() && !cb->isFlexibleBoxIncludingDeprecated()) {
+        && !isFloating() && !isInline() && !cb->isFlexibleBoxIncludingDeprecated() && !cb->isRenderGrid()) {
         LayoutUnit newMargin = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this);
         bool hasInvertedDirection = cb->style()->isLeftToRightDirection() != style()->isLeftToRightDirection();
         if (hasInvertedDirection)
index 47e1b9e..1f8b4eb 100644 (file)
@@ -461,6 +461,8 @@ void RenderBoxModelObject::computeStickyPositionConstraints(StickyPositionViewpo
 
     LayoutRect containerContentRect = containingBlock->contentBoxRect();
 
+    // Sticky positioned element ignore any override logical width on the containing block (as they don't call
+    // containingBlockLogicalWidthForContent). It's unclear whether this is totally fine.
     LayoutUnit minLeftMargin = minimumValueForLength(style()->marginLeft(), containingBlock->availableLogicalWidth(), view());
     LayoutUnit minTopMargin = minimumValueForLength(style()->marginTop(), containingBlock->availableLogicalWidth(), view());
     LayoutUnit minRightMargin = minimumValueForLength(style()->marginRight(), containingBlock->availableLogicalWidth(), view());
@@ -555,7 +557,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingTop() const
     RenderView* renderView = 0;
     Length padding = style()->paddingTop();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -567,7 +569,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingBottom() const
     RenderView* renderView = 0;
     Length padding = style()->paddingBottom();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -579,7 +581,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingLeft() const
     RenderView* renderView = 0;
     Length padding = style()->paddingLeft();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -591,7 +593,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingRight() const
     RenderView* renderView = 0;
     Length padding = style()->paddingRight();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -603,7 +605,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingBefore() const
     RenderView* renderView = 0;
     Length padding = style()->paddingBefore();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -615,7 +617,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingAfter() const
     RenderView* renderView = 0;
     Length padding = style()->paddingAfter();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -627,7 +629,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingStart() const
     RenderView* renderView = 0;
     Length padding = style()->paddingStart();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
@@ -639,7 +641,7 @@ LayoutUnit RenderBoxModelObject::computedCSSPaddingEnd() const
     RenderView* renderView = 0;
     Length padding = style()->paddingEnd();
     if (padding.isPercent())
-        w = containingBlock()->availableLogicalWidth();
+        w = containingBlockLogicalWidthForContent();
     else if (padding.isViewportPercentage())
         renderView = view();
     return minimumValueForLength(padding, w, renderView);
index 340c648..be9bbad 100644 (file)
@@ -44,6 +44,8 @@ public:
     virtual bool canCollapseAnonymousBlockChild() const OVERRIDE { return false; }
 
 private:
+    virtual bool isRenderGrid() const OVERRIDE { return true; }
+
     class GridTrack;
     enum TrackSizingDirection { ForColumns, ForRows };
     void computedUsedBreadthOfGridTracks(TrackSizingDirection, Vector<GridTrack>&);
index 258276b..4792f73 100644 (file)
@@ -396,6 +396,8 @@ public:
     virtual bool isRenderFullScreenPlaceholder() const { return false; }
 #endif
 
+    virtual bool isRenderGrid() const { return false; }
+
     virtual bool isRenderFlowThread() const { return false; }
     virtual bool isRenderNamedFlowThread() const { return false; }