On Tue, 7 Jul 2026 14:59:41 GMT, Marius Hanl <[email protected]> wrote:
>> This one took me several days to fix. Below I will try to explain every >> information I found out while debugging and fixing this. >> >> Originally I found this weird CSS issue already years ago, hence my comment >> in the issue. >> It happened because `ControlsFX` secretly changes your `Scene` root to its >> `DecorationPane` (and attaches your old root under it). This seems to >> trigger CSS errors sometimes that do not make sense and the UI seems to be >> fine as well. >> >> I tried to write several tests and scenarios that should work or not work. >> So some tests succeed without the fix and also after. >> The fix is only in `CssStyleHelper` and consists of two changes. Both for >> the `firstStyleableAncestor`, which has some issues. >> >> The `firstStyleableAncestor` was introcuded in >> https://bugs.openjdk.org/browse/JDK-8090462, so this is technically somewhat >> a regression from there. >> >> ### The fix >> >> A child can change the scene structure (e.g. change the root) as a result >> from processing its CSS. I wrote multiple tests to show that. Because of >> this scenario, we need to change how the `firstStyleableAncestor` is found. >> >> 1. The `firstStyleableAncestor` is now found by eagerly creating its >> `styleHelper` (before, we just checked if it has one). If this worked (-> >> that node is styleable), we have our ancestor. >> - 1.1 In `createStyleHelper`, we already traverse the parents to get the >> `depth`, so we can optimize our `firstStyleableAncestor` search by hanging >> into the loop and check if this is our styleable ancestor >> >> 2. In very rare cases `firstStyleableAncestor` can be null for a non-root >> `Node`. This, again can happen if something changes while we are processing >> the CSS. Normally CSS processing is top-down. But when a child changes a >> parent (or root), the structure changes and in this case we could no longer >> have a `firstStyleableAncestor`. So both CSS processing methods `resolveRef` >> and `getInheritedStyle` will search for a styleable ancestor if there is >> none set. >> - 2.1. This usually never happens and for `Scene` roots this will be a >> noop. >> >> #### Performance >> >> Performance wise, I could not see any problem or regression. We still cache >> our `firstStyleableAncestor` (and even reuse a preexisting loop). >> We might create a parent `styleHelper` more early in rare scenarios, but >> those will be created anyway the next pulse and usually are reused then. >> >> I also avoid to create any null `WeakReference` objects, as `resolveRef` >> could be hot path if the user... > > Marius Hanl has updated the pull request incrementally with one additional > commit since the last revision: > > Use CssParser.errorProperty() Marked as reviewed by angorya (Reviewer). ------------- PR Review: https://git.openjdk.org/jfx/pull/2201#pullrequestreview-4646361871
