On Sat, 10 Oct 2026 18:32:18 GMT, Marius Hanl <[email protected]> wrote:

> The third split PR of the CSS saga™.
> 
> In this episode, I have  a cornercase, but still a valid bug, found in 
> https://github.com/openjdk/jfx/pull/2225.
> 
> In JavaFX, there is this rule where `USER < AUTHOR` style. That is, CSS will 
> set the property value, e.g. opacity, to the value defined in the `Scene` 
> stylesheet and ignore if the value was set by a setter.
> 
> It is still possible to overwrite such value by a setter, but it will be 
> reapplied in the very next CSS pass.
> 
> Now, when we apply a style in a listener which is called during CSS reset, 
> this style may override the CSS value.
> This only happens when you style a property that is defined before the 
> property you are listening to.
> 
> A CSS style reset is the operation to set the property value to the original 
> value because it is not styled anymore (e.g. because we removed or added a 
> `PseudoClass`).
> 
> ---
> 
> Example: `Node`'s `CssMetaData` adds `OPACITY` first and later `TRANSLATE_X`.
> 
> So opacity is reset first, then `translateX`. If we now set the `opacity` in 
> a `translateX` listener, we can successfully override the `opacity`.
> 
> The very next CSS pass will however recover this (e.g. calling `applyCss` 
> again).
> This is not needed anymore with the fix.
> 
> ---
> 
> I added two tests, they are basically the same, just the order is changed.
> - The first test failed before, because it sets the `opacity` in a 
> `translateX` listener
> - The second test succeeds before and after, because it sets the `translateX` 
> in a `opacity` listener
> 
> Again, shows: `OPACITY` is processed first, later `TRANSLATE_X`, which with 
> the fix does not matter anymore.
> I reused the existing reset property logic. It is now called like before, and 
> as part of this fix.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Found a couple of possible issues (inline).

Thank you for adding tests!  Only one fails in master - 

`testStyleOverrideByTranslateListenerOnPseudoClassChange()` is this expected?

modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 995:

> 993:             }
> 994: 
> 995:             it.remove();

is it possible for a property to be (temporarily) non-settable?
once it's removed the later css pass won't be able to restore its value.
should the entry be removed only after the successful reset?

modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 
1009:

> 1007:                 }
> 1008:             } catch (Exception e) {
> 1009:                 PlatformLogger logger = Logging.getCSSLogger();

this exception is only logged.  should it propagate to 
`CssParser.errorsProperty`?

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

PR Review: https://git.openjdk.org/jfx/pull/2343#pullrequestreview-5493899283
PR Review Comment: https://git.openjdk.org/jfx/pull/2343#discussion_r4250546115
PR Review Comment: https://git.openjdk.org/jfx/pull/2343#discussion_r4250552364

Reply via email to