From ff602d4606d1af2c571e4d241dd750adeff13560 Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Wed, 9 Nov 2016 11:45:18 -0800 Subject: [PATCH] Absolute positioned items should apear inside padding but outside border Summary: Absolute positioned elements were not correctly respecting border / padding. https://github.com/facebook/css-layout/issues/245 fixes #245 Reviewed By: gkassabli Differential Revision: D4153332 fbshipit-source-id: 251e29e02018a433f60349b78c03feb121512797 --- CSSLayout/CSSLayout.c | 16 ++-- .../CSSLayoutAbsolutePositionTest.cs | 76 +++++++++++++++++++ .../CSSLayoutAbsolutePositionTest.html | 5 ++ .../CSSLayoutAbsolutePositionTest.java | 75 ++++++++++++++++++ tests/CSSLayoutAbsolutePositionTest.cpp | 74 ++++++++++++++++++ 5 files changed, 239 insertions(+), 7 deletions(-) diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c index 7b5c3b4a..e8e4d21c 100644 --- a/CSSLayout/CSSLayout.c +++ b/CSSLayout/CSSLayout.c @@ -1115,12 +1115,14 @@ static void absoluteLayoutChild(const CSSNodeRef node, if (isTrailingPosDefined(child, mainAxis) && !isLeadingPosDefined(child, mainAxis)) { child->layout.position[leading[mainAxis]] = node->layout.measuredDimensions[dim[mainAxis]] - child->layout.measuredDimensions[dim[mainAxis]] - + getTrailingBorder(node, mainAxis) - getTrailingPosition(child, mainAxis); } if (isTrailingPosDefined(child, crossAxis) && !isLeadingPosDefined(child, crossAxis)) { child->layout.position[leading[crossAxis]] = node->layout.measuredDimensions[dim[crossAxis]] - child->layout.measuredDimensions[dim[crossAxis]] - + getTrailingBorder(node, crossAxis) - getTrailingPosition(child, crossAxis); } } @@ -1830,16 +1832,14 @@ static void layoutNodeImpl(const CSSNodeRef node, getLeadingMargin(child, mainAxis); } } else { - if (performLayout) { - // If the child is position absolute (without top/left) or relative, - // we put it at the current accumulated offset. - child->layout.position[pos[mainAxis]] += mainDim; - } - // Now that we placed the element, we need to update the variables. // We need to do that only for relative elements. Absolute elements // do not take part in that phase. if (child->style.positionType == CSSPositionTypeRelative) { + if (performLayout) { + child->layout.position[pos[mainAxis]] += mainDim; + } + if (canSkipFlex) { // If we skipped the flex step, then we can't rely on the // measuredDims because @@ -1857,6 +1857,8 @@ static void layoutNodeImpl(const CSSNodeRef node, // can only be one element in that cross dimension. crossDim = fmaxf(crossDim, getDimWithMargin(child, crossAxis)); } + } else if (performLayout) { + child->layout.position[pos[mainAxis]] += getLeadingBorder(node, mainAxis) + leadingMainDim; } } } @@ -1901,7 +1903,7 @@ static void layoutNodeImpl(const CSSNodeRef node, getLeadingMargin(child, crossAxis); } else { child->layout.position[pos[crossAxis]] = - leadingPaddingAndBorderCross + getLeadingMargin(child, crossAxis); + getLeadingBorder(node, crossAxis) + getLeadingMargin(child, crossAxis); } } else { float leadingCrossDim = leadingPaddingAndBorderCross; diff --git a/csharp/tests/Facebook.CSSLayout/CSSLayoutAbsolutePositionTest.cs b/csharp/tests/Facebook.CSSLayout/CSSLayoutAbsolutePositionTest.cs index b5ee81a3..c1884387 100644 --- a/csharp/tests/Facebook.CSSLayout/CSSLayoutAbsolutePositionTest.cs +++ b/csharp/tests/Facebook.CSSLayout/CSSLayoutAbsolutePositionTest.cs @@ -31,6 +31,11 @@
+ +
+
+
+
* */ @@ -264,5 +269,76 @@ namespace Facebook.CSSLayout Assert.AreEqual(100, root_child0_child0.LayoutHeight); } + [Test] + public void Test_absolute_layout_within_border() + { + CSSNode root = new CSSNode(); + root.SetMargin(CSSEdge.Left, 10); + root.SetMargin(CSSEdge.Top, 10); + root.SetMargin(CSSEdge.Right, 10); + root.SetMargin(CSSEdge.Bottom, 10); + root.SetPadding(CSSEdge.Left, 10); + root.SetPadding(CSSEdge.Top, 10); + root.SetPadding(CSSEdge.Right, 10); + root.SetPadding(CSSEdge.Bottom, 10); + root.SetBorder(CSSEdge.Left, 10); + root.SetBorder(CSSEdge.Top, 10); + root.SetBorder(CSSEdge.Right, 10); + root.SetBorder(CSSEdge.Bottom, 10); + root.StyleWidth = 100; + root.StyleHeight = 100; + + CSSNode root_child0 = new CSSNode(); + root_child0.PositionType = CSSPositionType.Absolute; + root_child0.SetPosition(CSSEdge.Left, 0); + root_child0.SetPosition(CSSEdge.Top, 0); + root_child0.StyleWidth = 50; + root_child0.StyleHeight = 50; + root.Insert(0, root_child0); + + CSSNode root_child1 = new CSSNode(); + root_child1.PositionType = CSSPositionType.Absolute; + root_child1.SetPosition(CSSEdge.Right, 0); + root_child1.SetPosition(CSSEdge.Bottom, 0); + root_child1.StyleWidth = 50; + root_child1.StyleHeight = 50; + root.Insert(1, root_child1); + root.StyleDirection = CSSDirection.LeftToRight; + root.CalculateLayout(); + + Assert.AreEqual(10, root.LayoutX); + Assert.AreEqual(10, root.LayoutY); + Assert.AreEqual(100, root.LayoutWidth); + Assert.AreEqual(100, root.LayoutHeight); + + Assert.AreEqual(10, root_child0.LayoutX); + Assert.AreEqual(10, root_child0.LayoutY); + Assert.AreEqual(50, root_child0.LayoutWidth); + Assert.AreEqual(50, root_child0.LayoutHeight); + + Assert.AreEqual(40, root_child1.LayoutX); + Assert.AreEqual(40, root_child1.LayoutY); + Assert.AreEqual(50, root_child1.LayoutWidth); + Assert.AreEqual(50, root_child1.LayoutHeight); + + root.StyleDirection = CSSDirection.RightToLeft; + root.CalculateLayout(); + + Assert.AreEqual(10, root.LayoutX); + Assert.AreEqual(10, root.LayoutY); + Assert.AreEqual(100, root.LayoutWidth); + Assert.AreEqual(100, root.LayoutHeight); + + Assert.AreEqual(10, root_child0.LayoutX); + Assert.AreEqual(10, root_child0.LayoutY); + Assert.AreEqual(50, root_child0.LayoutWidth); + Assert.AreEqual(50, root_child0.LayoutHeight); + + Assert.AreEqual(40, root_child1.LayoutX); + Assert.AreEqual(40, root_child1.LayoutY); + Assert.AreEqual(50, root_child1.LayoutWidth); + Assert.AreEqual(50, root_child1.LayoutHeight); + } + } } diff --git a/gentest/fixtures/CSSLayoutAbsolutePositionTest.html b/gentest/fixtures/CSSLayoutAbsolutePositionTest.html index 37ebffe8..8530e045 100644 --- a/gentest/fixtures/CSSLayoutAbsolutePositionTest.html +++ b/gentest/fixtures/CSSLayoutAbsolutePositionTest.html @@ -19,3 +19,8 @@
+ +
+
+
+
diff --git a/java/tests/com/facebook/csslayout/CSSLayoutAbsolutePositionTest.java b/java/tests/com/facebook/csslayout/CSSLayoutAbsolutePositionTest.java index f3703e70..e4ced9fc 100644 --- a/java/tests/com/facebook/csslayout/CSSLayoutAbsolutePositionTest.java +++ b/java/tests/com/facebook/csslayout/CSSLayoutAbsolutePositionTest.java @@ -31,6 +31,11 @@
+ +
+
+
+
* */ @@ -258,4 +263,74 @@ public class CSSLayoutAbsolutePositionTest { assertEquals(100, root_child0_child0.getLayoutHeight(), 0.0f); } + @Test + public void test_absolute_layout_within_border() { + final CSSNode root = new CSSNode(); + root.setMargin(Spacing.LEFT, 10); + root.setMargin(Spacing.TOP, 10); + root.setMargin(Spacing.RIGHT, 10); + root.setMargin(Spacing.BOTTOM, 10); + root.setPadding(Spacing.LEFT, 10); + root.setPadding(Spacing.TOP, 10); + root.setPadding(Spacing.RIGHT, 10); + root.setPadding(Spacing.BOTTOM, 10); + root.setBorder(Spacing.LEFT, 10); + root.setBorder(Spacing.TOP, 10); + root.setBorder(Spacing.RIGHT, 10); + root.setBorder(Spacing.BOTTOM, 10); + root.setStyleWidth(100); + root.setStyleHeight(100); + + final CSSNode root_child0 = new CSSNode(); + root_child0.setPositionType(CSSPositionType.ABSOLUTE); + root_child0.setPosition(Spacing.LEFT, 0); + root_child0.setPosition(Spacing.TOP, 0); + root_child0.setStyleWidth(50); + root_child0.setStyleHeight(50); + root.addChildAt(root_child0, 0); + + final CSSNode root_child1 = new CSSNode(); + root_child1.setPositionType(CSSPositionType.ABSOLUTE); + root_child1.setPosition(Spacing.RIGHT, 0); + root_child1.setPosition(Spacing.BOTTOM, 0); + root_child1.setStyleWidth(50); + root_child1.setStyleHeight(50); + root.addChildAt(root_child1, 1); + root.setDirection(CSSDirection.LTR); + root.calculateLayout(null); + + assertEquals(10, root.getLayoutX(), 0.0f); + assertEquals(10, root.getLayoutY(), 0.0f); + assertEquals(100, root.getLayoutWidth(), 0.0f); + assertEquals(100, root.getLayoutHeight(), 0.0f); + + assertEquals(10, root_child0.getLayoutX(), 0.0f); + assertEquals(10, root_child0.getLayoutY(), 0.0f); + assertEquals(50, root_child0.getLayoutWidth(), 0.0f); + assertEquals(50, root_child0.getLayoutHeight(), 0.0f); + + assertEquals(40, root_child1.getLayoutX(), 0.0f); + assertEquals(40, root_child1.getLayoutY(), 0.0f); + assertEquals(50, root_child1.getLayoutWidth(), 0.0f); + assertEquals(50, root_child1.getLayoutHeight(), 0.0f); + + root.setDirection(CSSDirection.RTL); + root.calculateLayout(null); + + assertEquals(10, root.getLayoutX(), 0.0f); + assertEquals(10, root.getLayoutY(), 0.0f); + assertEquals(100, root.getLayoutWidth(), 0.0f); + assertEquals(100, root.getLayoutHeight(), 0.0f); + + assertEquals(10, root_child0.getLayoutX(), 0.0f); + assertEquals(10, root_child0.getLayoutY(), 0.0f); + assertEquals(50, root_child0.getLayoutWidth(), 0.0f); + assertEquals(50, root_child0.getLayoutHeight(), 0.0f); + + assertEquals(40, root_child1.getLayoutX(), 0.0f); + assertEquals(40, root_child1.getLayoutY(), 0.0f); + assertEquals(50, root_child1.getLayoutWidth(), 0.0f); + assertEquals(50, root_child1.getLayoutHeight(), 0.0f); + } + } diff --git a/tests/CSSLayoutAbsolutePositionTest.cpp b/tests/CSSLayoutAbsolutePositionTest.cpp index 92fd8f0f..905cd73b 100644 --- a/tests/CSSLayoutAbsolutePositionTest.cpp +++ b/tests/CSSLayoutAbsolutePositionTest.cpp @@ -31,6 +31,11 @@
+ +
+
+
+
* */ @@ -248,3 +253,72 @@ TEST(CSSLayoutTest, do_not_clamp_height_of_absolute_node_to_height_of_its_overfl CSSNodeFreeRecursive(root); } + +TEST(CSSLayoutTest, absolute_layout_within_border) { + const CSSNodeRef root = CSSNodeNew(); + CSSNodeStyleSetMargin(root, CSSEdgeLeft, 10); + CSSNodeStyleSetMargin(root, CSSEdgeTop, 10); + CSSNodeStyleSetMargin(root, CSSEdgeRight, 10); + CSSNodeStyleSetMargin(root, CSSEdgeBottom, 10); + CSSNodeStyleSetPadding(root, CSSEdgeLeft, 10); + CSSNodeStyleSetPadding(root, CSSEdgeTop, 10); + CSSNodeStyleSetPadding(root, CSSEdgeRight, 10); + CSSNodeStyleSetPadding(root, CSSEdgeBottom, 10); + CSSNodeStyleSetBorder(root, CSSEdgeLeft, 10); + CSSNodeStyleSetBorder(root, CSSEdgeTop, 10); + CSSNodeStyleSetBorder(root, CSSEdgeRight, 10); + CSSNodeStyleSetBorder(root, CSSEdgeBottom, 10); + CSSNodeStyleSetWidth(root, 100); + CSSNodeStyleSetHeight(root, 100); + + const CSSNodeRef root_child0 = CSSNodeNew(); + CSSNodeStyleSetPositionType(root_child0, CSSPositionTypeAbsolute); + CSSNodeStyleSetPosition(root_child0, CSSEdgeLeft, 0); + CSSNodeStyleSetPosition(root_child0, CSSEdgeTop, 0); + CSSNodeStyleSetWidth(root_child0, 50); + CSSNodeStyleSetHeight(root_child0, 50); + CSSNodeInsertChild(root, root_child0, 0); + + const CSSNodeRef root_child1 = CSSNodeNew(); + CSSNodeStyleSetPositionType(root_child1, CSSPositionTypeAbsolute); + CSSNodeStyleSetPosition(root_child1, CSSEdgeRight, 0); + CSSNodeStyleSetPosition(root_child1, CSSEdgeBottom, 0); + CSSNodeStyleSetWidth(root_child1, 50); + CSSNodeStyleSetHeight(root_child1, 50); + CSSNodeInsertChild(root, root_child1, 1); + CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); + + ASSERT_EQ(10, CSSNodeLayoutGetLeft(root)); + ASSERT_EQ(10, CSSNodeLayoutGetTop(root)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root)); + ASSERT_EQ(100, CSSNodeLayoutGetHeight(root)); + + ASSERT_EQ(10, CSSNodeLayoutGetLeft(root_child0)); + ASSERT_EQ(10, CSSNodeLayoutGetTop(root_child0)); + ASSERT_EQ(50, CSSNodeLayoutGetWidth(root_child0)); + ASSERT_EQ(50, CSSNodeLayoutGetHeight(root_child0)); + + ASSERT_EQ(40, CSSNodeLayoutGetLeft(root_child1)); + ASSERT_EQ(40, CSSNodeLayoutGetTop(root_child1)); + ASSERT_EQ(50, CSSNodeLayoutGetWidth(root_child1)); + ASSERT_EQ(50, CSSNodeLayoutGetHeight(root_child1)); + + CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionRTL); + + ASSERT_EQ(10, CSSNodeLayoutGetLeft(root)); + ASSERT_EQ(10, CSSNodeLayoutGetTop(root)); + ASSERT_EQ(100, CSSNodeLayoutGetWidth(root)); + ASSERT_EQ(100, CSSNodeLayoutGetHeight(root)); + + ASSERT_EQ(10, CSSNodeLayoutGetLeft(root_child0)); + ASSERT_EQ(10, CSSNodeLayoutGetTop(root_child0)); + ASSERT_EQ(50, CSSNodeLayoutGetWidth(root_child0)); + ASSERT_EQ(50, CSSNodeLayoutGetHeight(root_child0)); + + ASSERT_EQ(40, CSSNodeLayoutGetLeft(root_child1)); + ASSERT_EQ(40, CSSNodeLayoutGetTop(root_child1)); + ASSERT_EQ(50, CSSNodeLayoutGetWidth(root_child1)); + ASSERT_EQ(50, CSSNodeLayoutGetHeight(root_child1)); + + CSSNodeFreeRecursive(root); +}