On Tue, 29 Sep 2026 14:50:27 GMT, Marius Hanl <[email protected]> wrote:
>> Found while implementing a fix in https://github.com/openjdk/jfx/pull/2225 >> and also marked in the review by @andy-goryachev-oracle. >> >> `CssStyleHelper` holds the `triggerStates`. Those are needed for pseudoclass >> matching. >> >> Example: `.parent:ps > .leaf { ... }` >> What happens: >> `leaf` is adding `ps` into the triggerStates of `parent`. >> This is done so that `parent` will know: If my pseudoclass `ps` changes, I >> have children that are interested in that change, so I must update the CSS. >> >> The problem with that currently: >> We might need to create an empty `CssStyleHelper` 'shell' to hold the >> `triggerStates`. >> >> In the example above, we will create a `CssStyleHelper` for `parent` only >> for the `triggerStates`. >> Because `parent` is not styled, it normally has no `CssStyleHelper`. >> >> Because of that mechanism, the code in `CssStyleHelper` has multiple >> problems: >> 1. We need to check if we have a real `CssStyleHelper` or just a 'shell' for >> the `triggerStates` >> 1.1. This is done by various checks for `cacheContainer != null` >> 2. `findFirstStyleableAncestor()` can return a `Node` with such an empty >> `CssStyleHelper` (that was only created for the `triggerStates`), which is >> wrong >> 2.1. I managed to write a test that actually fails because of this >> misbehavior. With this refactor, the test succeeds >> 3. We create more `CssStyleHelper` objects than we actually need to >> >> Solution: Move the `triggerStates` to `Node`. This is also where the other >> css flags live in. >> >> Overall, this will improve performance a bit, make the `CssStyleHelper` a >> bit cleaner, especially regarding separation of concerns. >> Now the following is always true: If a `CssStyleHelper` exists, the `Node` >> is styled and always has a `CacheContainer` that is never null. >> Will help towards finishing: https://github.com/openjdk/jfx/pull/2225. >> >> I wrote a bunch of tests that will succeed before and after. >> One test fails before and succeeds now. >> No change in behavior is expected other than being more correct and fixing >> one misbehavior. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Marius Hanl has updated the pull request incrementally with one additional > commit since the last revision: > > remove final, fix javadoc This look like a right move. _We_ couldn't find any problems with. modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 201: > 199: final PseudoClassState triggerState = triggerStates[n]; > 200: > 201: // TODO: this means that a style like .menu-item:hover won't > work. Need to separate CssStyleHelper tree from scene-graph tree is this still a TODO? to be addressed in a follow-up? ------------- Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/2333#pullrequestreview-5368098164 PR Review Comment: https://git.openjdk.org/jfx/pull/2333#discussion_r4146102957
