Skip to content

Commit

Permalink
Fix for nodes with display: contents not being cleaned in some cases (
Browse files Browse the repository at this point in the history
#1729)

Summary:
X-link: facebook/react-native#47194

Fixes a case where a node with `display: contents` would not be cleaned up in some cases. This was caused by it being called after some early returns handling different quick paths. This PR moves the call to `cleanupContentsNodesRecursively` earlier so that it's always called.

The problem here wasn't mutating before cloning, but leaving a node marked as dirty after the layout has finished.

The exact case in which I found this was a node with a single `display: contents` child which needs to be a leaf. Then in the parent node [this](https://github.com/facebook/yoga/blob/b0b842d5e75d041e3af7e0ac55abfb8929fbbf21/yoga/algorithm/CalculateLayout.cpp#L1339) condition is true, so `cleanupContentsNodesRecursively` doesn't get called and the child node is never visited and cleaned. I assume the same will happen in the other paths with an early return here.

Changelog:
[General][Fixed] - Fix for nodes with `display: contents` not being cleaned in some cases

Pull Request resolved: #1729

Reviewed By: rozele

Differential Revision: D64910099

Pulled By: NickGerleman

fbshipit-source-id: 6d56f8fbf687b7ee5af889c0b868406213c9cee8
  • Loading branch information
j-piasecki authored and facebook-github-bot committed Oct 26, 2024
1 parent e696b8c commit 8dc2fa5
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions yoga/algorithm/CalculateLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,10 @@ static void calculateLayoutImpl(
flexColumnDirection, direction, ownerWidth),
PhysicalEdge::Bottom);

// Clean and update all display: contents nodes with a direct path to the
// current node as they will not be traversed
cleanupContentsNodesRecursively(node);

if (node->hasMeasureFunc()) {
measureNodeWithMeasureFunc(
node,
Expand Down Expand Up @@ -1366,9 +1370,6 @@ static void calculateLayoutImpl(
// Reset layout flags, as they could have changed.
node->setLayoutHadOverflow(false);

// Clean and update all display: contents nodes with a direct path to the
// current node as they will not be traversed
cleanupContentsNodesRecursively(node);
// STEP 1: CALCULATE VALUES FOR REMAINDER OF ALGORITHM
const FlexDirection mainAxis =
resolveDirection(node->style().flexDirection(), direction);
Expand Down

0 comments on commit 8dc2fa5

Please sign in to comment.