RFR: 8345188: Support tree-structural pseudo-classes [v6]

[Andy Goryachev]

On Wed, 8 Jan 2025 17:28:04 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/docs/javafx/scene/doc-files/cssref.html line 1530:
>> 
>>> 1528:     <p>In the following example, all background color of all buttons uses the
>>> 1529:       looked up color "abc".</p>
>>> 1530:     <p class="example">:root { abc: #f00 }<br>
>> 
>> I see many places where `.root` is removed in favor of `:root`, but both are supported still?  At least, the code in `Scene` leads me to believe that it will also apply the `.root` style class still.  Should this legacy style class be mentioned in the CSS documentation still?
>
> It will still be supported for backwards compatibility, but I see no reason to mention that now that we have the standard-conformant `:root` pseudo-class.

I think we do need to mention the old selector in the CSS Ref because it is still supported.

We should also tell the developers what they need to do with the existing stylesheets, whether to update them to use the new selector, or, if we plan to support the old one indefinitely, explain that.

>> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 341:
>> 
>>> 339: 
>>> 340:         @Override
>>> 341:         protected void onChanged(Change<Node> c) {
>> 
>> I hate to bring this up, and it perhaps should be dealt with separately, but how safe is this code in `onProposedChange` / `onChanged` when it is re-entered due to the many potential listener callbacks?  For example, the removal of a child on `parent` can trigger list change listeners from the user that can trigger all kinds of other changes that eventually lead to more children changes on this node, triggering a nested `onChanged`; the new changes are adding more opportunities in the form of listeners on pseudo state changes.
>> 
>> For example, `onProposedChange` will blatantly reset `geomChanged` to `false` in all cases, even if it rejects the change later.  It doesn't check if an `onChange` is in progress higher up the call stack, effectively resetting a flag that is in active use...
>> 
>> I believe this may be quite a difficult problem to solve if nested calls remain allowed, and one of the easiest ways to fix it IMHO is to reject any changes (in `onProposedChange`) when a change is already in progress. For users this would effectively mean that listeners triggered during a child modification will not be allowed to make further modifications to the same children list until the first one completes.  This is not a common situation, but can happen with very dynamic UI's that construct new nodes in response to triggers; without such protection for nested changes, the errors that crop up are often of a form that seem to be multi-threading issues (where certain invariants seem to be broken), while in reality they're just re-entrant code calls that use instance fields in an unsafe manner.
>
> One option might be to move all pseudo-class modifications to the end of the `onChange` method. This would make the code equivalent to being called after the callback has completed.

Is there a possibility of introducing an infinite loop?

And if the code is moved to the end of `onChange`, might that cause a regression with the existing pseudo-classes?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907676024
PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907683667