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.
This is also the root cause of https://bugs.openjdk.org/browse/JDK-8291853.

### 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 hovers e.g. over table entries. So no objects are 
created in this case.

There is https://bugs.openjdk.org/browse/JDK-8187955 which has an idea how to 
get rid of a recursion which I think is a good step if we want to optimize this 
further. But the ticket has not much information, so it is hard to completely 
get what the author meant.

##### What I tried before

At first, I thought this problem is only related to the `Scene` root node. So I 
first implemented a fix on the `rootProperty`, always prcoessing css for the 
new root (immediately, so we create a new `styleHelper` now and not the next 
pulse) or nulling the `styleHelper` on the old root.

But then I checked if this issue can be reproduced with non-root `Node`s as 
well, so I wrote tests for it that proves that this is a generic problem not 
related to the root `Node` (it just so happens that `modena.css` and usually 
all other stylesheets usually set the colors on `.root` and node on a child 
node).

With that information, the problem must be inside `CssStyleHelper` and not in 
`Scene#rootProperty` or `Node#setScenes`.

---------
- [x] I confirm that I make this contribution in accordance with the [OpenJDK 
Interim AI Policy](https://openjdk.org/legal/ai).

-------------

Commit messages:
 - 8268657: Looked-up color fails for -fx-background-color in JavaFX CSS file

Changes: https://git.openjdk.org/jfx/pull/2201/files
  Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=2201&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8268657
  Stats: 447 lines in 3 files changed: 397 ins; 19 del; 31 mod
  Patch: https://git.openjdk.org/jfx/pull/2201.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/2201/head:pull/2201

PR: https://git.openjdk.org/jfx/pull/2201

Reply via email to