Skip to content

Commit

Permalink
Fix issue where padding for box sizing is resolved against wrong refe…
Browse files Browse the repository at this point in the history 
…rence length (#1715)

Summary:
Pull Request resolved: #1715

X-link: facebook/react-native#46799

Content box impl had a bug where we resolved padding % against the same reference length as the dimensions. Padding should always be against containing block's width. This is also true for width, but not for height, which should be against containing block's height.

This just pipes the width into our box sizing functions.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D63787577

fbshipit-source-id: e512338770f25b66506cabab5a7cde8f04397ea0
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Oct 5, 2024
1 parent 990ec92 commit e69fcb2
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 103 deletions.
14 changes: 8 additions & 6 deletions gentest/fixtures/YGBoxSizingTest.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@
style="width: 100px; height: 100px; padding: 5px; box-sizing: content-box">
</div>

<div id="box_sizing_content_box_padding_only_percent" style="width: 100px; height: 100px;">
<div style="width: 50%; height: 75; padding: 10%; box-sizing: content-box">
<div id="box_sizing_content_box_padding_only_percent" style="width: 100px; height: 150px;">
<div style="width: 50px; height: 75px; padding: 10%; box-sizing: content-box">
</div>
</div>

<div id="box_sizing_border_box_padding_only" style="width: 100px; height: 100px; padding: 5px; box-sizing: border-box">
</div>

<div id="box_sizing_border_box_padding_only_percent" style="width: 100px; height: 100px;">
<div style="width: 50%; height: 75; padding: 10%; box-sizing: border-box">
<div id="box_sizing_border_box_padding_only_percent" style="width: 100px; height: 150px;">
<div style="width: 50px; height: 75px; padding: 10%; box-sizing: border-box">
</div>
</div>

Expand Down Expand Up @@ -197,7 +197,8 @@
</div>
</div>

<div data-disabled="true" id="box_sizing_content_box_flex_basis_row" style="width: 100px; height: 100px; flex-direction: row;">
<div data-disabled="true" id="box_sizing_content_box_flex_basis_row"
style="width: 100px; height: 100px; flex-direction: row;">
<div style="flex-basis: 50px; height: 25px; padding: 5px; border-width: 10px; box-sizing: content-box">
</div>
</div>
Expand All @@ -207,7 +208,8 @@
</div>
</div>

<div data-disabled="true" id="box_sizing_content_box_flex_basis_column" style="width: 100px; height: 100px; flex-direction: column;">
<div data-disabled="true" id="box_sizing_content_box_flex_basis_column"
style="width: 100px; height: 100px; flex-direction: column;">
<div style="flex-basis: 50px; height: 25px; padding: 5px; border-width: 10px; box-sizing: content-box">
</div>
</div>
Expand Down
28 changes: 15 additions & 13 deletions java/tests/generated/com/facebook/yoga/YGBoxSizingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<80fa9cf05afc330a721a756dfaf0d1a3>>
* @generated SignedSource<<7f42121bbd9772cdbc079aac71040dcc>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGBoxSizingTest.html
*/

Expand Down Expand Up @@ -452,14 +452,15 @@ public void test_box_sizing_content_box_padding_only_percent() {
final YogaNode root = createNode(config);
root.setPositionType(YogaPositionType.ABSOLUTE);
root.setWidth(100f);
root.setHeight(100f);
root.setHeight(150f);

final YogaNode root_child0 = createNode(config);
root_child0.setPaddingPercent(YogaEdge.LEFT, 10);
root_child0.setPaddingPercent(YogaEdge.TOP, 10);
root_child0.setPaddingPercent(YogaEdge.RIGHT, 10);
root_child0.setPaddingPercent(YogaEdge.BOTTOM, 10);
root_child0.setWidthPercent(50f);
root_child0.setWidth(50f);
root_child0.setHeight(75f);
root_child0.setBoxSizing(YogaBoxSizing.CONTENT_BOX);
root.addChildAt(root_child0, 0);
root.setDirection(YogaDirection.LTR);
Expand All @@ -468,25 +469,25 @@ public void test_box_sizing_content_box_padding_only_percent() {
assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(100f, root.getLayoutWidth(), 0.0f);
assertEquals(100f, root.getLayoutHeight(), 0.0f);
assertEquals(150f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(70f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(20f, root_child0.getLayoutHeight(), 0.0f);
assertEquals(95f, root_child0.getLayoutHeight(), 0.0f);

root.setDirection(YogaDirection.RTL);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(100f, root.getLayoutWidth(), 0.0f);
assertEquals(100f, root.getLayoutHeight(), 0.0f);
assertEquals(150f, root.getLayoutHeight(), 0.0f);

assertEquals(30f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(70f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(20f, root_child0.getLayoutHeight(), 0.0f);
assertEquals(95f, root_child0.getLayoutHeight(), 0.0f);
}

@Test
Expand Down Expand Up @@ -525,40 +526,41 @@ public void test_box_sizing_border_box_padding_only_percent() {
final YogaNode root = createNode(config);
root.setPositionType(YogaPositionType.ABSOLUTE);
root.setWidth(100f);
root.setHeight(100f);
root.setHeight(150f);

final YogaNode root_child0 = createNode(config);
root_child0.setPaddingPercent(YogaEdge.LEFT, 10);
root_child0.setPaddingPercent(YogaEdge.TOP, 10);
root_child0.setPaddingPercent(YogaEdge.RIGHT, 10);
root_child0.setPaddingPercent(YogaEdge.BOTTOM, 10);
root_child0.setWidthPercent(50f);
root_child0.setWidth(50f);
root_child0.setHeight(75f);
root.addChildAt(root_child0, 0);
root.setDirection(YogaDirection.LTR);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(100f, root.getLayoutWidth(), 0.0f);
assertEquals(100f, root.getLayoutHeight(), 0.0f);
assertEquals(150f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(50f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(20f, root_child0.getLayoutHeight(), 0.0f);
assertEquals(75f, root_child0.getLayoutHeight(), 0.0f);

root.setDirection(YogaDirection.RTL);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(100f, root.getLayoutWidth(), 0.0f);
assertEquals(100f, root.getLayoutHeight(), 0.0f);
assertEquals(150f, root.getLayoutHeight(), 0.0f);

assertEquals(50f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(50f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(20f, root_child0.getLayoutHeight(), 0.0f);
assertEquals(75f, root_child0.getLayoutHeight(), 0.0f);
}

@Test
Expand Down
28 changes: 15 additions & 13 deletions javascript/tests/generated/YGBoxSizingTest.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<48d3d46dec54df38f853a6fa17e6f0c6>>
* @generated SignedSource<<a841bb29f095097bae9635f62b9fb16d>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGBoxSizingTest.html
*/

Expand Down Expand Up @@ -499,39 +499,40 @@ test('box_sizing_content_box_padding_only_percent', () => {
root = Yoga.Node.create(config);
root.setPositionType(PositionType.Absolute);
root.setWidth(100);
root.setHeight(100);
root.setHeight(150);

const root_child0 = Yoga.Node.create(config);
root_child0.setPadding(Edge.Left, "10%");
root_child0.setPadding(Edge.Top, "10%");
root_child0.setPadding(Edge.Right, "10%");
root_child0.setPadding(Edge.Bottom, "10%");
root_child0.setWidth("50%");
root_child0.setWidth(50);
root_child0.setHeight(75);
root_child0.setBoxSizing(BoxSizing.ContentBox);
root.insertChild(root_child0, 0);
root.calculateLayout(undefined, undefined, Direction.LTR);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(100);
expect(root.getComputedHeight()).toBe(100);
expect(root.getComputedHeight()).toBe(150);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(70);
expect(root_child0.getComputedHeight()).toBe(20);
expect(root_child0.getComputedHeight()).toBe(95);

root.calculateLayout(undefined, undefined, Direction.RTL);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(100);
expect(root.getComputedHeight()).toBe(100);
expect(root.getComputedHeight()).toBe(150);

expect(root_child0.getComputedLeft()).toBe(30);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(70);
expect(root_child0.getComputedHeight()).toBe(20);
expect(root_child0.getComputedHeight()).toBe(95);
} finally {
if (typeof root !== 'undefined') {
root.freeRecursive();
Expand Down Expand Up @@ -582,38 +583,39 @@ test('box_sizing_border_box_padding_only_percent', () => {
root = Yoga.Node.create(config);
root.setPositionType(PositionType.Absolute);
root.setWidth(100);
root.setHeight(100);
root.setHeight(150);

const root_child0 = Yoga.Node.create(config);
root_child0.setPadding(Edge.Left, "10%");
root_child0.setPadding(Edge.Top, "10%");
root_child0.setPadding(Edge.Right, "10%");
root_child0.setPadding(Edge.Bottom, "10%");
root_child0.setWidth("50%");
root_child0.setWidth(50);
root_child0.setHeight(75);
root.insertChild(root_child0, 0);
root.calculateLayout(undefined, undefined, Direction.LTR);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(100);
expect(root.getComputedHeight()).toBe(100);
expect(root.getComputedHeight()).toBe(150);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(50);
expect(root_child0.getComputedHeight()).toBe(20);
expect(root_child0.getComputedHeight()).toBe(75);

root.calculateLayout(undefined, undefined, Direction.RTL);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(100);
expect(root.getComputedHeight()).toBe(100);
expect(root.getComputedHeight()).toBe(150);

expect(root_child0.getComputedLeft()).toBe(50);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(50);
expect(root_child0.getComputedHeight()).toBe(20);
expect(root_child0.getComputedHeight()).toBe(75);
} finally {
if (typeof root !== 'undefined') {
root.freeRecursive();
Expand Down
28 changes: 15 additions & 13 deletions tests/generated/YGBoxSizingTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*
* clang-format off
* @generated SignedSource<<36f0f519320608e2c5c6eb6666be7efc>>
* @generated SignedSource<<24bf988fec7e7f72a8f46ba74df63399>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGBoxSizingTest.html
*/

Expand Down Expand Up @@ -446,39 +446,40 @@ TEST(YogaTest, box_sizing_content_box_padding_only_percent) {
YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
YGNodeStyleSetWidth(root, 100);
YGNodeStyleSetHeight(root, 100);
YGNodeStyleSetHeight(root, 150);

YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeLeft, 10);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeTop, 10);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeRight, 10);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeBottom, 10);
YGNodeStyleSetWidthPercent(root_child0, 50);
YGNodeStyleSetWidth(root_child0, 50);
YGNodeStyleSetHeight(root_child0, 75);
YGNodeStyleSetBoxSizing(root_child0, YGBoxSizingContentBox);
YGNodeInsertChild(root, root_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(150, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(70, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(95, YGNodeLayoutGetHeight(root_child0));

YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(150, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(30, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(70, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(95, YGNodeLayoutGetHeight(root_child0));

YGNodeFreeRecursive(root);

Expand Down Expand Up @@ -521,38 +522,39 @@ TEST(YogaTest, box_sizing_border_box_padding_only_percent) {
YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
YGNodeStyleSetWidth(root, 100);
YGNodeStyleSetHeight(root, 100);
YGNodeStyleSetHeight(root, 150);

YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeLeft, 10);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeTop, 10);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeRight, 10);
YGNodeStyleSetPaddingPercent(root_child0, YGEdgeBottom, 10);
YGNodeStyleSetWidthPercent(root_child0, 50);
YGNodeStyleSetWidth(root_child0, 50);
YGNodeStyleSetHeight(root_child0, 75);
YGNodeInsertChild(root, root_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(150, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(75, YGNodeLayoutGetHeight(root_child0));

YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(150, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(50, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(75, YGNodeLayoutGetHeight(root_child0));

YGNodeFreeRecursive(root);

Expand Down
10 changes: 8 additions & 2 deletions yoga/algorithm/AbsoluteLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,10 @@ void layoutAbsoluteChild(
if (child->hasDefiniteLength(Dimension::Width, containingBlockWidth)) {
childWidth = child
->getResolvedDimension(
direction, Dimension::Width, containingBlockWidth)
direction,
Dimension::Width,
containingBlockWidth,
containingBlockWidth)
.unwrap() +
marginRow;
} else {
Expand Down Expand Up @@ -362,7 +365,10 @@ void layoutAbsoluteChild(
if (child->hasDefiniteLength(Dimension::Height, containingBlockHeight)) {
childHeight = child
->getResolvedDimension(
direction, Dimension::Height, containingBlockHeight)
direction,
Dimension::Height,
containingBlockHeight,
containingBlockWidth)
.unwrap() +
marginColumn;
} else {
Expand Down
Loading

0 comments on commit e69fcb2

Please sign in to comment.