8388378: [Regression] FX CSS: styled property is reset to its initial value on a pseudo-class-only transition after tree items change (TreeTableRow -fx-indent collapses) by hjohn · Pull Request #2218 · openjdk/jfx

hjohn · 2026-07-19T22:52:26Z

Fix CssStyleHelper treating absent cache values as "no style applies"

I uncovered a problem that has been in the CSS engine for a long time, even before #1076 was applied. However, #1076 made this problem more obvious because of a side fix that was done there:

The BitSet equals implementation was updated to NOT take the length of the allocated array (to store the bits in) into account for equality. As this array is just allocated on demand depending on what bits were set and reset, it should not be taken into account for equality

The above bug hid problems when nodes were supposed to share a CSS cache entry, but didn't because their BitSets were considered different (even though semantically, they were the same).

With the fix in #1076, a lot more cases were sharing CSS cache entries (as they should) but this now exposed a bug in how CssStyleHelper handled the absence of a cached property. Basically, absence could mean two things before this change:

The property was not computed at all: it either didn't exist at the time (due to CssMetaData changing!) or because the property was not settable (because it was bound)
The property was computed but no styles applied to it, so no need to cache anything...

The CSS engine always assumed the latter, which means that if for whatever reason the cache entry was created by a Node that had outdated CssMetaData (a Control that is yet to be skinned, or one where a CSS property was unsettable), the engine would assume that such a missing property was unstyled and can safely be reset. As entries are shared, this doesn't hold true for all nodes that share the same entry (if another node that shares the same entry has different CssMetaData or did not have the same property bound, then it may have been styled, and should not be reset!).

Tests that confirm the problem

I added 4 new test cases, for four paths that could potentially result in the wrong things being in the cache:

A cache entry being created for a Node that has bound properties (and is later shared with a different node)
A cache entry being created for a Node that isn't skinned yet (and so it's CssMetaData may change); the CSS engine simply can never assume that CssMetaData is stable (skins can also be changed at any time)
Ensuring that an exception during applyStyle still results in a SKIP entry in the cache
Ensuring that we still put a value in the cache, even if the Node can't use it itself directly as its value is bound (this test overlaps a bit with the first, perhaps one can be removed)

All of those tests should fail with the old implementation. The test that most closely resembles the problem with the regression reported in JDK-8388378 is the changing CSS meta data test (that simulates a skin replacement):

VirtualFlow creates new TreeTableRows during layout (it really shouldn't, but whatever)
Such new rows are added to the scene graph, and have CSS applied by some logic in Node when its parent/scene is assigned -- however, these are still unskinned as the skin logic in Control didn't have a chance to run yet, meaning their CSS metadata does not have the property -fx-indent in them -- the CSS engine therefore doesn't know of that property and created a cache entry lacking it
When later another node shares the same cache entry, the absence of the -fx-indent property was treated as "unstyled" and it got its value reset to whatever the default indent was...

The Change

I adjusted CssStyleHelper to only ever use explicitly set values in the cache; this can be a non-null CalculatedValue or the special value SKIP. If nothing was in the cache at all, it is interpreted as "never evaluated" and so must be looked up. It no longer conflates absence of a value with being unstyled.

This however does mean that we must store more values in the cache. Any value that was evaluated, but did not result in a value that came from CSS is now set to SKIP, as otherwise it would be absent and the new logic would then re-evaluate it each time.

So what does that effectively mean? It means that where before a cached entry only contained entries that were explicitly assigned a CSS value, it now always contains all properties found in the CSS metadata -- most of those will be set to the sentinel SKIP, so the cost for this is low. As the entry is also shared (and is now much safer to share) the total cost for these extra SKIP values in the cached entry is probably neglible, but it is certainly more than it was before.

I think correctness here is more important than slightly more overhead, and since it was the only solution I could think of that would solve these problems, I went ahead with this fix.

If later it proves the overhead is not as neglible as I thought, we can adjust it knowing that we have several tests that catch problems with incorrect use of caches. One possible direction to go here is to use a more efficient HashMap implementation, possibly even just a simple list of values instead of a map, but that's definitely out of scope IMHO.

Note

It's possible this may fix other hard to pin down and obscure CSS bugs, but I haven't done a search for possible other tickets that may be resolved by this fix.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8388378: [Regression] FX CSS: styled property is reset to its initial value on a pseudo-class-only transition after tree items change (TreeTableRow -fx-indent collapses) (Bug - P3)

Reviewers

Marius Hanl (@Maran23 - Reviewer)
Andy Goryachev (@andy-goryachev-oracle - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2218/head:pull/2218
$ git checkout pull/2218

Update a local copy of the PR:
$ git checkout pull/2218
$ git pull https://git.openjdk.org/jfx.git pull/2218/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2218

View PR using the GUI difftool:
$ git pr show -t 2218

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2218.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-07-19T22:53:33Z

👋 Welcome back jhendrikx! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-07-19T22:53:58Z

@hjohn This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8388378: [Regression] FX CSS: styled property is reset to its initial value on a pseudo-class-only transition after tree items change (TreeTableRow -fx-indent collapses)

Reviewed-by: mhanl, angorya

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 24 new commits pushed to the master branch:

b2a98f3: 8388451: Additional WebKit 625.1 fixes from WebKitGTK 2.52.5
1e7e7d0: 8380556: Update WebKit to 625.1
7d39117: Merge
... and 21 more: https://git.openjdk.org/jfx/compare/279cafb495a64adf34fae42dfe53a2aecf06c1d2...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2026-07-19T22:55:40Z

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

mlbridge · 2026-07-19T22:58:56Z

Webrevs

Maran23

Overall, this looks good. Nice to see some documentation for the CssStyleHelper and related classes! Tests look nice as well and you explained all scenarios very well.

And it is great to see the removal of forceSlowpath, which during my debugging was hard for me to understand why it even exists.

Will do more testing later this week!

Maran23 · 2026-07-20T10:07:38Z

 /**
- *
+ * Caches the calculated value of each styleable property a specific combination of
+ * pseudo-class states and font size (see {@link Key}).  An entry is shared by every


Minor: Two spaces.

Suggested change

* pseudo-class states and font size (see {@link Key}). An entry is shared by every

* pseudo-class states and font size (see {@link Key}). An entry is shared by every

some people write two spaces after the period (I do). but a) it's an internal class and b) even if it were a public javadoc, two spaces will not render in html.

Maran23 · 2026-07-20T10:08:06Z

+ * {@code Styleable} that has that same combination.
+ * <p>
+ * {@link #get(String)} returning {@code null} means only that no one has evaluated that
+ * property for this entry yet; it should never be treated as "no style applies"(!). This


Minor: Grammar

Suggested change

* property for this entry yet; it should never be treated as "no style applies"(!). This

* property for this entry yet; it should never be treated as "no style applies"(!). This is

Maran23 · 2026-07-20T10:09:24Z

+             * The cached value is still useful for others sharing this entry.
+             */
+
+            if (!cssMetaData.isSettable(node)) continue;


I would prefer this style, but again, minor and is all of the place in this file anyway

Suggested change

if (!cssMetaData.isSettable(node)) continue;

if (!cssMetaData.isSettable(node)) {

continue;

}

Maran23 · 2026-07-20T10:13:07Z

+        other.attachExtraProperty();
+
+        /*
+         * The "other" dummy control has the extra property attached to it immediately


Those inline doc comments are very helpful!
One idea: I wonder if we should put (most of) them at the test method as javadoc.

This is up to you, just a potential suggestion.

hjohn · 2026-07-20T15:34:08Z

Overall, this looks good. Nice to see some documentation for the CssStyleHelper and related classes! Tests look nice as well and you explained all scenarios very well.

And it is great to see the removal of forceSlowpath, which during my debugging was hard for me to understand why it even exists.

It existed for when CSS metadata changed for a Node (so almost specifically to support what Control does with its changeable CSS metadata). When force slow path is on, it would lookup all defined CSS properties for that Node, and add them to the shared cache entry (the shared cache entry is not reset for this as many other nodes can be using it) to ensure that all CSS properties that should have a value are part of the shared cache entry, specifically if the Node's CSS metadata change introduced new CSS properties not seen before.

If it didn't do this, then a newly discovered CSS property would not be added to the shared cache entry, and then its absence would later be interpreted as "unstyled" while it really was simply "not evaluated".

This problem is gone in the new implementation because absence is always treated as "must be re-evaluated", resulting in either a CalculatedValue or SKIP being added for any CSS properties that were missing (even after a CSS metadata change).

Will do more testing later this week!

Thanks, I appreciate the quick review!

Maran23 · 2026-07-21T11:36:49Z

It existed for when CSS metadata changed for a Node (so almost specifically to support what Control does with its changeable CSS metadata). When force slow path is on, it would lookup all defined CSS properties for that Node, and add them to the shared cache entry (the shared cache entry is not reset for this as many other nodes can be using it) to ensure that all CSS properties that should have a value are part of the shared cache entry, specifically if the Node's CSS metadata change introduced new CSS properties not seen before.

Ah that makes sense.

Sounds hacky, as you also described in your ticket.
Since the CssMetadata amount can change anytime, calling transitionToState again will not detect new CssMetadata - only when forceSlowPath is true or the style helper is newly created before.

So this new behavior is always more correct.

hjohn · 2026-07-21T12:38:04Z

It existed for when CSS metadata changed for a Node (so almost specifically to support what Control does with its changeable CSS metadata). When force slow path is on, it would lookup all defined CSS properties for that Node, and add them to the shared cache entry (the shared cache entry is not reset for this as many other nodes can be using it) to ensure that all CSS properties that should have a value are part of the shared cache entry, specifically if the Node's CSS metadata change introduced new CSS properties not seen before.

Ah that makes sense.

Sounds hacky, as you also described in your ticket. Since the CssMetadata amount can change anytime, calling transitionToState again will not detect new CssMetadata - only when forceSlowPath is true or the style helper is newly created before.

So this new behavior is always more correct.

Yeah -- before it was actually trivial to get the CSS system confused... all you had to do is bind a property, and then change to a pseudo class state combination that it hadn't seen before yet. It would create a new cache entry, but would omit the "locked" property. If a sibling later switched to the same pseudo class state combination, it would use the cache entry with the omitted property and incorrectly reset it...

That was the first problem I investigated, but it turned out the problem from the ticket was even more complex (but similar in nature).

I understand that when the CSS system was designed, they probably made the assumption that getCssMetaData is immutable (and it is in most cases); then when that assumption failed because of controls and skins, a forceSlowPath solution was added...

andy-goryachev-oracle

I like the removal of all this conditionals around the caching, the new code is much more straightforward. Found a couple of possible issues (inline).

One persistent thought I had is whether or not it is possible to have a more formalized approach to testing the CSS subsystem. Like, exhaustively enumerate all possible transitions and develop a test for each.

andy-goryachev-oracle · 2026-07-21T20:58:33Z

 /**
- *
+ * Caches the calculated value of each styleable property a specific combination of
+ * pseudo-class states and font size (see {@link Key}).  An entry is shared by every


some people write two spaces after the period (I do). but a) it's an internal class and b) even if it were a public javadoc, two spaces will not render in html.

andy-goryachev-oracle · 2026-07-21T21:00:34Z

+                /*
+                 * JDK-8127435: If there is no style for the property (SKIP), then check if it must be reset
+                 * to its initial value. Otherwise, continue with CSS application.
+                 */


minor: I would have used // style comments

andy-goryachev-oracle · 2026-07-21T21:54:37Z

-
            final String property = cssMetaData.getProperty();

            CalculatedValue calculatedValue = cacheEntry.get(property);


it looks like the value is stored by property name, which means if the SKIP value was stored and then the metadata was replaced (by setting a skin, for example, which uses the same property names), the old SKIP would prevent re-evaluation.

A property always evaluates the same, as this is dictated by the stylesheet, not by the presence or absence of a property in the CSS metadata. So the stylesheet says: xyz must be 5 here, and if you replace the skin and it has the same property, that evaluation is still correct.

There may be a problem if that new property is of a different type (I didn't test), but I consider that beyond the scope of this fix as that has never worked. If you see an easy solution, I can consider adding it -- my primary goal was to fix the customer found regression that seemed to have been due to #1076 but in reality has been there much longer, only less visible.

I have to admit it's a rather contrived scenario, so I am not concerned. The devs can always work around this by using different property names.

Maybe just add a comment pointing to this scenario?

I added this to StyleCacheEntry:

Entries are (currently) keyed purely by property name, with no check that a cached value's type still matches what the current lookup expects. This is harmless as long as every {@code Styleable} that could share an entry agrees on the type behind a given property name (which is generally standard for CSS properties). It is not a problem for two different {@code CssMetaData} instances to have the same property name, only if their types would also differ.

andy-goryachev-oracle · 2026-07-21T22:08:51Z

+
+        /*
+         * The other node is now transitioned to the same state. Since the value of -fx-boom
+         * could not be evaluated, the value is reset to its default value (1.0).


shouldn't it be the value specified by the stylesheet? 41.0, since the styling of the other has not encountered an error?

You are right that 41.0 would make more sense here, but that's assuming we could get that value at all. The source of the exception used here is just to test the SKIP adding code path (as it is crucial now that we add SKIP or a value for every value in the CSS metadata). The path I was testing however is normally only triggered when lookup fails, and when that fails there simply isn't a value to put in the cache at all (ie. wrong type in the CSS or you used a reference that doesn't exist). applyStyle isn't allowed to throw exception, so results are "best effort" (and how this worked is pre-existing, I didn't change it).

My understanding is that if one control fails to applyStyle, the other control (which encountered no issues) should pick up the values from the stylesheet, and certainly not the default.

Or am I missing something?

I must point out: applyStyle is not allowed to fail, so this is pretty much an edge case already (which I just exploit in the test to ensure the exceptional path is adding SKIP in exceptional cases).

The exceptions are supposed to come from lookup, which if it fails with an exception means that you simply do not know what the CSS value could be. This is then stored in the cache entry, and any sharing of that entry assumes the entry is created correctly (which it was if lookup was failing, as there is no retrying here as the CSS evaluation will just return the same exception).

So that it resets the value for other Nodes is expected, as there is no CSS value known as computation failed (artificially in this case, but failed nonetheless).

Again, in this specific case, I agree we could do better, but it seems beyond the scope of this fix (it has always worked like this -- an exception in applyStyle resulted in the system assuming that for that cache entry there is no CSS based value -- no matter how wrong that assumption is).

I can fix this as well if you want, but:

applyStyle is not allowed to throw exceptions, so what are we truly fixing here?

I didn't change this

It has nothing to do with the problem that triggered the creation of this fix (which IMHO is already not something I caused... so I'm being nice here).

andy-goryachev-oracle · 2026-07-21T22:25:35Z

+             * The cached value is still useful for others sharing this entry.
+             */
+
+            if (!cssMetaData.isSettable(node)) continue;


the contract for CssMetaData.isSettable() says "This method is called before any styles are looked up for the given property." but here it's called after the lookup() in L929

That's an odd amount of detail that is rather implementation specific.

I would suggest we remove it, as I don't think there are good reasons for this other than "that's how it works currently", and it actually is an active source of problems.

A property being unsettable does not mean you can't evaluate the stylesheet to find what value it would have gotten (and that's what we do now). The only reason this method exists is that is faster than catching "RuntimeException: A bound value cannot be set.".

There are two good reasons to still evaluate the CSS value (even if we won't set it):

The property may not be bound forever; if the cache entry was created without the calculation of the value for a "locked" property, then we later won't know what CSS based value to put in there (note: unbinding a property is not detected, so binding/unbinding already plays very badly with the CSS system).

The cache entry may be shared with siblings in the same state; if the cache entry was initialized by a Node with bound properties, and we then just skip those, then the other siblings would not have that value either -- this causes incorrect value resets and properties to not be styled, which this PR specifically wants to address

If there are good reasons to keep it, then the only other option we have is to NOT create a cache entry at all when a Node has any CSS property bound (a sibling that is in a better state may create it still though). This could be a performance hazard though, because if it is the only node of its kind, it would always need to go through the slow lookup path until there are no more bound properties.

+1 for removing it.

you mean removing the spec "This method is called before any styles are looked up for the given property." ?

Yes, I don't think that should have been in the spec in the first place, as it reveals basically the inner workings of the CSS system. Also, we're still respecting the return value of isSettable -- we're not calling applyStyle (which would indeed result in the "expansion of the property" the documentation alludes to). We're merely looking up the styles, something the documentation specifically says it won't do -- but why guarantee that in the first place?

So I suggest rewriting the docs to:

* Check to see if the corresponding property on the given Node is * settable. This method is called before any call to {@code applyStyle} is * made. It is abstract so that the code can check if the property * is settable without expanding the property. Generally, the property is * settable if it is not null or is not bound.

Changing the sentence "This method is called before any styles are looked up for the given property." to "This method is called before any call to {@code applyStyle} is made." -- in both cases this ensures the property is not expanded unnecessarily, but gives the CSS system the freedom to still evaluate the stylesheet so it can create a shared cache entry that is complete.

Maran23 · 2026-07-23T10:01:06Z

Tested on multiple applications yesterday and today, all good. Also no performance issues or anything noticeable.

hjohn · 2026-07-23T13:47:26Z

Tested on multiple applications yesterday and today, all good. Also no performance issues or anything noticeable.

Thanks Marius :)

Maran23

Looks good. The new documentation is also welcome!
Will reapprove if we agree to change CssMetadata.isSettable here.

andy-goryachev-oracle · 2026-07-27T15:00:40Z

Agree with @Maran23 , this is an improvement and looks good. I would like to know your take on these two comments:

is it possible to have a more formalized approach to testing the CSS subsystem. Like, exhaustively enumerate all possible transitions and develop a test for each?
could relaxing the constraint on isSettable() API backfire somehow?

And, a question for @kevinrushforth : would the isSettable() javadoc change require a CSR?

hjohn · 2026-07-27T15:15:49Z

Agree with @Maran23 , this is an improvement and looks good. I would like to know your take on these two comments:

is it possible to have a more formalized approach to testing the CSS subsystem. Like, exhaustively enumerate all possible transitions and develop a test for each?

It might be possible, but it is hard to say how far that could go (CSS is quite complex). If you're looking to make things more testable, then I would start by splitting up CssStyleHelper in a few smaller classes that focus on specific aspects that can then be independently tested.

For testing transitions, I'd probably not use JUnit for something like this but a more dedicated state transition test system (I've developed an in-house one) where one simply specifies actions (with preconditions), and assertions that must hold; the framework then figures out what combinations to test. Otherwise you either will make your own "loops" in JUnit, or find that using @Nested quickly becomes a terrible way of testing potentially 10.000's state transitions...

could relaxing the constraint on isSettable() API backfire somehow?

I doubt it, it is an API not that much used, and the opposite approach is actually what was doing the backfiring.

andy-goryachev-oracle · 2026-07-27T17:20:14Z

re: testing

Just thinking out loud, we could make an attempt at exhaustive testing by first enumerating the possible scenarios, followed by iterating over combinations of those.

For example:

S01 - style change in owner
S02 - pseudostyle change in owner
S03 - style change in parent
S04 - pseudostyle change in parent
S05 - reparenting

and then iterate over 2^5 combinations of these, or probably less because some combinations might not be valid.

This is just an example, I am sure there are many more possible scenarios, the main benefit I see is that we can test all the valid combinations.

What do you think?

Maran23 · 2026-07-28T09:46:32Z

re: testing

What do you think?

I added some tests in #2225 regarding testing different child selectors and pseudoclasses.

andy-goryachev-oracle

Question: should this change be mentioned in the release notes (along with other CSS improvements)?

hjohn · 2026-07-28T22:29:32Z

Question: should this change be mentioned in the release notes (along with other CSS improvements)?

I don't know, it is mostly a fix for a very old bug, that became a bit more visible later on. The CSS system hasn't changed. So if you list these kinds of bugs then I guess it could become part of the release notes.

I've updated the isSettable docs now since we seem to be in agreement that it is going into too much detail there and offering guarantees it doesn't need to offer. Let me know what you think.

Maran23

New CssMetaData doc change looks good.
We should wait for @kevinrushforth if this needs a CSR or not.

kevinrushforth · 2026-07-29T22:32:35Z

New CssMetaData doc change looks good. We should wait what @kevinrushforth if this needs a CSR or not.

The doc change by itself seems reasonable, and I'd be OK doing it without a CSR. The only question I have then is regarding the behavioral change itself: would a "reasonable application" be surprised by the new behavior? If not, then we can skip the CSR.

hjohn · 2026-07-29T23:29:09Z

New CssMetaData doc change looks good. We should wait what @kevinrushforth if this needs a CSR or not.

The doc change by itself seems reasonable, and I'd be OK doing it without a CSR. The only question I have then is regarding the behavioral change itself: would a "reasonable application" be surprised by the new behavior? If not, then we can skip the CSR.

I don't think so, in fact, I don't think a reasonable application could even detect the difference without digging deep into CSS specifics -- the isSettable method is called regardless in both new and old version, the only difference is in how the CSS system acts on it (still looking up the resolved CSS value, instead of not doing anything at all). The applyStyle would not be called in either case, so there is no risk that a lazy property would be created early either.

kevinrushforth · 2026-07-30T00:12:57Z

The doc change by itself seems reasonable, and I'd be OK doing it without a CSR. The only question I have then is regarding the behavioral change itself: would a "reasonable application" be surprised by the new behavior? If not, then we can skip the CSR.

I don't think so, in fact, I don't think a reasonable application could even detect the difference without digging deep into CSS specifics -- the isSettable method is called regardless in both new and old version, the only difference is in how the CSS system acts on it (still looking up the resolved CSS value, instead of not doing anything at all). The applyStyle would not be called in either case, so there is no risk that a lazy property would be created early either.

That's what I thought. I think this can go in without a CSR.

Maran23 · 2026-07-30T10:15:35Z

@hjohn Then I think the last 'blocker' is the warning in the PR body: Title mismatch between PR and JBS for issue JDK-8388378.

hjohn · 2026-07-30T11:23:17Z

/integrate

openjdk · 2026-07-30T11:25:03Z

Going to push as commit 74e934a.
Since your change was applied there have been 24 commits pushed to the master branch:

b2a98f3: 8388451: Additional WebKit 625.1 fixes from WebKitGTK 2.52.5
1e7e7d0: 8380556: Update WebKit to 625.1
7d39117: Merge
... and 21 more: https://git.openjdk.org/jfx/compare/279cafb495a64adf34fae42dfe53a2aecf06c1d2...master

Your commit was automatically rebased without conflicts.

openjdk · 2026-07-30T11:25:13Z

@hjohn Pushed as commit 74e934a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Fix CssStyleHelper treating absent cache values as "no style applies"

3c52fd0

openjdk Bot added the rfr Ready for review label Jul 19, 2026

Maran23 reviewed Jul 20, 2026

View reviewed changes

andy-goryachev-oracle reviewed Jul 21, 2026

View reviewed changes

hjohn added 2 commits July 24, 2026 01:13

Add more information about how the CSS information works...

8739699

Fix typos

5dcd54c

Maran23 approved these changes Jul 24, 2026

View reviewed changes

Maran23 mentioned this pull request Jul 27, 2026

8388277: [REDO] Looked-up color fails for -fx-background-color in JavaFX CSS file #2225

Open

4 tasks

andy-goryachev-oracle approved these changes Jul 28, 2026

View reviewed changes

Update javadoc

905bda3

andy-goryachev-oracle approved these changes Jul 29, 2026

View reviewed changes

Maran23 approved these changes Jul 29, 2026

View reviewed changes

hjohn changed the title ~~JDK-8388378: Fix CssStyleHelper treating absent cache values as "no style applies"~~ JDK-8388378: [Regression] FX CSS: styled property is reset to its initial value on a pseudo-class-only transition after tree items change (TreeTableRow -fx-indent collapses) Jul 30, 2026

openjdk Bot added the ready Ready to be integrated label Jul 30, 2026

openjdk Bot added the integrated Pull request has been integrated label Jul 30, 2026

openjdk Bot closed this Jul 30, 2026

openjdk Bot removed ready Ready to be integrated rfr Ready for review labels Jul 30, 2026

	* pseudo-class states and font size (see {@link Key}). An entry is shared by every
	* pseudo-class states and font size (see {@link Key}). An entry is shared by every

	* property for this entry yet; it should never be treated as "no style applies"(!). This
	* property for this entry yet; it should never be treated as "no style applies"(!). This is


		final String property = cssMetaData.getProperty();

		CalculatedValue calculatedValue = cacheEntry.get(property);

Uh oh!

Conversation

hjohn commented Jul 19, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix CssStyleHelper treating absent cache values as "no style applies"

Tests that confirm the problem

The Change

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper Bot commented Jul 19, 2026

Uh oh!

openjdk Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk Bot commented Jul 19, 2026

Uh oh!

mlbridge Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Maran23 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hjohn commented Jul 20, 2026

Uh oh!

Maran23 commented Jul 21, 2026

Uh oh!

hjohn commented Jul 21, 2026

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hjohn Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hjohn Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Maran23 commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

hjohn commented Jul 19, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented Jul 19, 2026 •

edited

Loading

mlbridge Bot commented Jul 19, 2026 •

edited

Loading

hjohn Jul 21, 2026 •

edited

Loading

hjohn Jul 23, 2026 •

edited

Loading

Maran23 commented Jul 23, 2026 •

edited

Loading

hjohn commented Jul 27, 2026 •

edited

Loading

Maran23 left a comment •

edited

Loading

hjohn commented Jul 29, 2026 •

edited

Loading