Fixes layout of nodes with YGDisplayNone and YGPositionTypeAbsolute (f…

…acebook#1068) Summary: Pull Request resolved: facebook#1068 There is an issue in react-native when the Yoga node position type is set to absolute and display: none is set where the node layout calculation gives the absolute dimensions, rather than the expected 0 x 0. Here are some OSS issues tracking this: facebook/react-native#18415 microsoft/react-native-windows#7289 ## Changelog [General] [Fix] - Fixes layout of nodes with YGDisplayNone and YGPositionTypeAbsolute Reviewed By: Andrey-Mishanin Differential Revision: D26849307 fbshipit-source-id: 197618aa3c4e1b3b7efeba7ea4efd30b2d1c982d
fabioarnold · Mar 10, 2021 · 342aebe · 342aebe
1 parent 1745c23
commit 342aebe
Show file tree

Hide file tree

Showing 8 changed files with 201 additions and 21 deletions.
diff --git a/csharp/tests/Facebook.Yoga/YGDisplayTest.cs b/csharp/tests/Facebook.Yoga/YGDisplayTest.cs
@@ -1,9 +1,10 @@
-/**
+/*
  * Copyright (c) Facebook, Inc. and its affiliates.
  *
- * This source code is licensed under the MIT license found in the LICENSE
- * file in the root directory of this source tree.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
  */
+
 // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html
 
 using System;
@@ -333,5 +334,47 @@ public void Test_display_none_with_position()
             Assert.AreEqual(0f, root_child1.LayoutHeight);
         }
 
+        [Test]
+        public void Test_display_none_with_position_absolute()
+        {
+            YogaConfig config = new YogaConfig();
+
+            YogaNode root = new YogaNode(config);
+            root.Width = 100;
+            root.Height = 100;
+
+            YogaNode root_child0 = new YogaNode(config);
+            root_child0.PositionType = YogaPositionType.Absolute;
+            root_child0.Width = 100;
+            root_child0.Height = 100;
+            root_child0.Display = YogaDisplay.None;
+            root.Insert(0, root_child0);
+            root.StyleDirection = YogaDirection.LTR;
+            root.CalculateLayout();
+
+            Assert.AreEqual(0f, root.LayoutX);
+            Assert.AreEqual(0f, root.LayoutY);
+            Assert.AreEqual(100f, root.LayoutWidth);
+            Assert.AreEqual(100f, root.LayoutHeight);
+
+            Assert.AreEqual(0f, root_child0.LayoutX);
+            Assert.AreEqual(0f, root_child0.LayoutY);
+            Assert.AreEqual(0f, root_child0.LayoutWidth);
+            Assert.AreEqual(0f, root_child0.LayoutHeight);
+
+            root.StyleDirection = YogaDirection.RTL;
+            root.CalculateLayout();
+
+            Assert.AreEqual(0f, root.LayoutX);
+            Assert.AreEqual(0f, root.LayoutY);
+            Assert.AreEqual(100f, root.LayoutWidth);
+            Assert.AreEqual(100f, root.LayoutHeight);
+
+            Assert.AreEqual(0f, root_child0.LayoutX);
+            Assert.AreEqual(0f, root_child0.LayoutY);
+            Assert.AreEqual(0f, root_child0.LayoutWidth);
+            Assert.AreEqual(0f, root_child0.LayoutHeight);
+        }
+
     }
 }
diff --git a/gentest/fixtures/YGDisplayTest.html b/gentest/fixtures/YGDisplayTest.html
@@ -25,3 +25,7 @@
  <div style="flex-grow: 1;"></div>
  <div style="flex-grow: 1; display:none; top: 10px;"></div>
 </div>
+
+<div id="display_none_with_position_absolute" style="width: 100px; height: 100px;">
+  <div style="display:none; position: absolute; width: 100px; height: 100px"></div>
+</div>
diff --git a/gentest/gentest.js b/gentest/gentest.js
@@ -41,11 +41,11 @@ function assert(condition, message) {
 
 function printTest(e, LTRContainer, RTLContainer, genericContainer) {
   e.push([
-    '/**',
+    '/*',
     ' * Copyright (c) Facebook, Inc. and its affiliates.',
     ' *',
-    ' * This source code is licensed under the MIT license found in the LICENSE',
-    ' * file in the root directory of this source tree.',
+    ' * This source code is licensed under the MIT license found in the',
+    ' * LICENSE file in the root directory of this source tree.',
     ' */',
     '// @Generated by gentest/gentest.rb from gentest/fixtures/' + document.title + '.html',
     '',

diff --git a/gentest/gentest.rb b/gentest/gentest.rb
@@ -7,13 +7,14 @@
 require 'watir'
 require 'fileutils'
 
-caps = Selenium::WebDriver::Remote::Capabilities.chrome(
-  "loggingPrefs"=>{
-    "browser"=>"ALL",
-    "performance"=>"ALL"
-  }
-)
-browser = Watir::Browser.new(:chrome, :desired_capabilities => caps, :switches => ['--force-device-scale-factor=1', '--window-position=0,0'])
+browser = Watir::Browser.new(:chrome, "goog:loggingPrefs" => {
+    "browser" => "ALL",
+    "performance" => "ALL"
+  },
+  "chromeOptions" => {
+    "w3c" => "false"
+  },
+  :switches => ['--force-device-scale-factor=1', '--window-position=0,0'])
 
 Dir.chdir(File.dirname($0))
 

diff --git a/java/tests/com/facebook/yoga/YGDisplayTest.java b/java/tests/com/facebook/yoga/YGDisplayTest.java
@@ -1,9 +1,10 @@
-/**
+/*
  * Copyright (c) Facebook, Inc. and its affiliates.
  *
- * This source code is licensed under the MIT license found in the LICENSE
- * file in the root directory of this source tree.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
  */
+
 // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html
 
 package com.facebook.yoga;
@@ -337,6 +338,47 @@ public void test_display_none_with_position() {
     assertEquals(0f, root_child1.getLayoutHeight(), 0.0f);
   }
 
+  @Test
+  public void test_display_none_with_position_absolute() {
+    YogaConfig config = YogaConfigFactory.create();
+
+    final YogaNode root = createNode(config);
+    root.setWidth(100f);
+    root.setHeight(100f);
+
+    final YogaNode root_child0 = createNode(config);
+    root_child0.setPositionType(YogaPositionType.ABSOLUTE);
+    root_child0.setWidth(100f);
+    root_child0.setHeight(100f);
+    root_child0.setDisplay(YogaDisplay.NONE);
+    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(0f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(0f, 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(0f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutHeight(), 0.0f);
+  }
+
   private YogaNode createNode(YogaConfig config) {
     return mNodeFactory.create(config);
   }

diff --git a/javascript/tests/Facebook.Yoga/YGDisplayTest.js b/javascript/tests/Facebook.Yoga/YGDisplayTest.js
@@ -1,9 +1,10 @@
 /**
  * Copyright (c) Facebook, Inc. and its affiliates.
  *
- * This source code is licensed under the MIT license found in the LICENSE
- * file in the root directory of this source tree.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
  */
+
 // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html
 
 var Yoga = Yoga || require("../../sources/entry-" + process.env.TEST_ENTRY);
@@ -342,3 +343,48 @@ it("display_none_with_position", function () {
     config.free();
   }
 });
+it("display_none_with_position_absolute", function () {
+  var config = Yoga.Config.create();
+
+  try {
+    var root = Yoga.Node.create(config);
+    root.setWidth(100);
+    root.setHeight(100);
+
+    var root_child0 = Yoga.Node.create(config);
+    root_child0.setPositionType(Yoga.POSITION_TYPE_ABSOLUTE);
+    root_child0.setWidth(100);
+    root_child0.setHeight(100);
+    root_child0.setDisplay(Yoga.DISPLAY_NONE);
+    root.insertChild(root_child0, 0);
+    root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_LTR);
+
+    console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")");
+    console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")");
+    console.assert(100 === root.getComputedWidth(), "100 === root.getComputedWidth() (" + root.getComputedWidth() + ")");
+    console.assert(100 === root.getComputedHeight(), "100 === root.getComputedHeight() (" + root.getComputedHeight() + ")");
+
+    console.assert(0 === root_child0.getComputedLeft(), "0 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")");
+    console.assert(0 === root_child0.getComputedTop(), "0 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")");
+    console.assert(0 === root_child0.getComputedWidth(), "0 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")");
+    console.assert(0 === root_child0.getComputedHeight(), "0 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")");
+
+    root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_RTL);
+
+    console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")");
+    console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")");
+    console.assert(100 === root.getComputedWidth(), "100 === root.getComputedWidth() (" + root.getComputedWidth() + ")");
+    console.assert(100 === root.getComputedHeight(), "100 === root.getComputedHeight() (" + root.getComputedHeight() + ")");
+
+    console.assert(0 === root_child0.getComputedLeft(), "0 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")");
+    console.assert(0 === root_child0.getComputedTop(), "0 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")");
+    console.assert(0 === root_child0.getComputedWidth(), "0 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")");
+    console.assert(0 === root_child0.getComputedHeight(), "0 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")");
+  } finally {
+    if (typeof root !== "undefined") {
+      root.freeRecursive();
+    }
+
+    config.free();
+  }
+});
diff --git a/tests/YGDisplayTest.cpp b/tests/YGDisplayTest.cpp
@@ -1,9 +1,10 @@
 /*
  * Copyright (c) Facebook, Inc. and its affiliates.
  *
- * This source code is licensed under the MIT license found in the LICENSE
- * file in the root directory of this source tree.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
  */
+
 // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html
 
 #include <gtest/gtest.h>
@@ -327,3 +328,45 @@ TEST(YogaTest, display_none_with_position) {
 
   YGConfigFree(config);
 }
+
+TEST(YogaTest, display_none_with_position_absolute) {
+  const YGConfigRef config = YGConfigNew();
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root, 100);
+  YGNodeStyleSetHeight(root, 100);
+
+  const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetPositionType(root_child0, YGPositionTypeAbsolute);
+  YGNodeStyleSetWidth(root_child0, 100);
+  YGNodeStyleSetHeight(root_child0, 100);
+  YGNodeStyleSetDisplay(root_child0, YGDisplayNone);
+  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(0, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(0, 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(0, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
+}
diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp
@@ -3557,7 +3557,8 @@ static void YGNodelayoutImpl(
   if (performLayout) {
     // STEP 10: SIZING AND POSITIONING ABSOLUTE CHILDREN
     for (auto child : node->getChildren()) {
-      if (child->getStyle().positionType() != YGPositionTypeAbsolute) {
+      if (child->getStyle().display() == YGDisplayNone ||
+          child->getStyle().positionType() != YGPositionTypeAbsolute) {
         continue;
       }
       YGNodeAbsoluteLayoutChild(