On Tue, 18 Apr 2023 12:17:53 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Fix bug in CSS caching code that could reset values on unrelated nodes.
>>
>> The bug occurs due to a cache entry being constructed incorrectly when the
>> initial node that triggered the cache entry creation has user set values.
>> The calculated values for properties with a user set value were omitted in
>> the cache entry, and other nodes that later share the same entry would
>> incorrectly assume the omitted property was unstyled and were therefore
>> reset to their default values.
>
> John Hendrikx has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - Merge branch 'openjdk:master' into feature/css-cache-bug
> - Fix typo
> - Fix bug in CSS caching code that could reset values on unrelated nodes
>
> The bug occurs due to a cache entry being constructed incorrectly when
> the initial node that triggered the cache entry creation has user set
> values. The calculated values for properties with a user set value were
> omitted in the cache entry, and other nodes that later share the same
> entry would incorrectly assume the omitted property was unstyled and
> were therefore reset to their default values.
minor comments, otherwise LGTM
modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line
1185:
> 1183: if (cssValue != null && "inherit".equals(cssValue.getValue())) {
> 1184: style = getInheritedStyle(styleable, property);
> 1185: if (style == null) return SKIP;
minor: could we surround return with { }'s please? To keep one statement per
line.
modules/javafx.graphics/src/test/java/test/javafx/scene/CssStyleHelperTest.java
line 622:
> 620: }
> 621: """
> 622: );
is there a chance we would backport this fix (to < java15)?
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1072#pullrequestreview-1426954523
PR Review Comment: https://git.openjdk.org/jfx/pull/1072#discussion_r1194089298
PR Review Comment: https://git.openjdk.org/jfx/pull/1072#discussion_r1194092591
