On Tue, 4 Aug 2026 15:03:04 GMT, Andy Goryachev <[email protected]> wrote:
>> Marius Hanl has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains two commits: >> >> - Merge branch 'master' of https://github.com/openjdk/jfx into >> 8388277-REDO]-Looked-up-color-fails-for--fx-background-color-in-JavaFX-CSS-file >> >> # Conflicts: >> # >> modules/javafx.graphics/src/test/java/test/javafx/scene/CssStyleHelperTest.java >> - 8388277: [REDO] Looked-up color fails for -fx-background-color in JavaFX >> CSS file > > modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line > 285: > >> 283: private static boolean isStyleableAncestor(Node parentNode) { >> 284: if (parentNode.cssHelperState == Node.CssHelperState.STALE) { >> 285: parentNode.cssHelperState = >> Node.CssHelperState.RESOLVED_EARLY; > > there might be a problem here: this code sets `RESOLVED_EARLY` before > creating the `styleHelper`. createStyleHelper() can invoke application > listeners that might, for example, add new nodes and force `applyCss()` > immediately. The re-entrance descendants see `RESOLVED_EARLY` and trust the > old, stale, `styleHelper`. > > would it make sense to add another state, or perhaps move the state into > `styleHelper` ? Moving to `styleHelper` was my very first idea but many nodes will have not have one (mostly layout containers). So it is not possible to do that. I can change the order of the statements though. A layout container may have no style helper but because a parent style changed and now affects it, it needs one (and we need to set the state accordingly). In the future, I would really like to revisit this. I hope this can be simplified, but there are more changes needed. Especially the creation of an empty `CssStyleHelper` just to hold parent states is weird. Maybe, on the way we will find an even better way, when other things are refactored/improved first. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2225#discussion_r3741387302
