Skip to content

Fix stale layout when a child switches to display: contents after insertion - #58721

Closed
cipolleschi wants to merge 1 commit into
react:mainfrom
cipolleschi:export-D122122520
Closed

cipolleschi wants to merge 1 commit into
react:mainfrom
cipolleschi:export-D122122520

Conversation

@cipolleschi

Copy link
Copy Markdown
Contributor

Summary:
Each node caches how many of its children use display: contents (contentsChildrenCount_, added in react/yoga#1726). The count is updated in insertChild, removeChild, replaceChild and setChildren, but not when the display of a child that is already attached changes.

So if YGNodeStyleSetDisplay(child, YGDisplayContents) is called after the child was inserted, the owner still reports hasContentsChildren() == false. The layout algorithm then skips cleanupContentsNodesRecursively for it, and the contents node keeps isDirty == true after layout. When a descendant of that node changes later, markDirtyAndPropagate stops at the contents node because it is already dirty, the root is never marked dirty, and the next YGNodeCalculateLayout returns the cached layout.

Minimal repro:

YGNodeRef root = YGNodeNew();
YGNodeStyleSetWidth(root, 100);
YGNodeStyleSetHeight(root, 100);
YGNodeRef child = YGNodeNew();
YGNodeInsertChild(root, child, 0);
YGNodeRef grandchild = YGNodeNew();
YGNodeStyleSetWidth(grandchild, 10);
YGNodeStyleSetHeight(grandchild, 10);
YGNodeInsertChild(child, grandchild, 0);

YGNodeStyleSetDisplay(child, YGDisplayContents);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
// YGNodeIsDirty(child) is still true

YGNodeStyleSetWidth(grandchild, 20);
// YGNodeIsDirty(root) is false
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
// YGNodeLayoutGetWidth(grandchild) is 10, expected 20

This affects any binding that mutates nodes in place (the JS and Java bindings both go through YGNodeStyleSetDisplay). Setting the display before inserting the child works, which is what the existing tests do.

The fix recomputes the owner's count in YGNodeStyleSetDisplay when a node that has an owner switches to or from display: contents.

Changelog: [General][Fixed] - Fix stale layout when a child switches to display: contents after being inserted

X-link: react/yoga#2028

Reviewed By: javache

Differential Revision: D122122520

Pulled By: cipolleschi

…nsertion

Summary:
Each node caches how many of its children use `display: contents` (`contentsChildrenCount_`, added in react/yoga#1726). The count is updated in `insertChild`, `removeChild`, `replaceChild` and `setChildren`, but not when the display of a child that is already attached changes.

So if `YGNodeStyleSetDisplay(child, YGDisplayContents)` is called after the child was inserted, the owner still reports `hasContentsChildren() == false`. The layout algorithm then skips `cleanupContentsNodesRecursively` for it, and the contents node keeps `isDirty == true` after layout. When a descendant of that node changes later, `markDirtyAndPropagate` stops at the contents node because it is already dirty, the root is never marked dirty, and the next `YGNodeCalculateLayout` returns the cached layout.

Minimal repro:

```cpp
YGNodeRef root = YGNodeNew();
YGNodeStyleSetWidth(root, 100);
YGNodeStyleSetHeight(root, 100);
YGNodeRef child = YGNodeNew();
YGNodeInsertChild(root, child, 0);
YGNodeRef grandchild = YGNodeNew();
YGNodeStyleSetWidth(grandchild, 10);
YGNodeStyleSetHeight(grandchild, 10);
YGNodeInsertChild(child, grandchild, 0);

YGNodeStyleSetDisplay(child, YGDisplayContents);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
// YGNodeIsDirty(child) is still true

YGNodeStyleSetWidth(grandchild, 20);
// YGNodeIsDirty(root) is false
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
// YGNodeLayoutGetWidth(grandchild) is 10, expected 20
```

This affects any binding that mutates nodes in place (the JS and Java bindings both go through `YGNodeStyleSetDisplay`). Setting the display before inserting the child works, which is what the existing tests do.

The fix recomputes the owner's count in `YGNodeStyleSetDisplay` when a node that has an owner switches to or from `display: contents`.

Changelog: [General][Fixed] - Fix stale layout when a child switches to `display: contents` after being inserted

X-link: react/yoga#2028

Reviewed By: javache

Differential Revision: D122122520

Pulled By: cipolleschi
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 28, 2026
@meta-codesync

meta-codesync Bot commented Sep 28, 2026

Copy link
Copy Markdown

@cipolleschi has exported this pull request. If you are a Meta employee, you can view the originating Diff in D122122520.

@meta-codesync meta-codesync Bot closed this in 19a6722 Sep 28, 2026
@meta-codesync meta-codesync Bot added the Merged This PR has been merged. label Sep 28, 2026
@meta-codesync

meta-codesync Bot commented Sep 28, 2026

Copy link
Copy Markdown

@cipolleschi merged this pull request in 19a6722.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants