[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: JDK-8245919: Region#padding property rendering error
From:       John Hendrikx <jhendrikx () openjdk ! org>
Date:       2023-03-27 23:16:43
Message-ID: -ypPhDce19uqwTo5kOpthcChMemjQFBrF4vBr2wmaZ4=.07008d37-adb2-4bd7-8f61-39099d3e11ee () github ! com
[Download RAW message or body]

On Mon, 27 Mar 2023 23:05:00 GMT, John Hendrikx <jhendrikx@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.

Note for reviewers:

The problem of one Node affecting another unrelated Node is because all properties of \
the same type, at the same level and same pseudo state set share a cache entry. When \
the cache entry is constructed based on a Node with an overridden property, the \
calculated value for the overridden property would be omitted incorrectly -- it is \
incorrect because other Nodes that would share the same entry may not have this \
property overridden!  The cache entry always should have a complete set of calculated \
values, regardless of which Node was used to construct the initial cache entry.

I removed the user overridden property check in `lookup`. I think this is harmless \
because both callers of `lookup` do the same check themselves again to avoid applying \
a style that would override a user set value.  However, in `transitionToState` this \
is just **after** the calculated value is added to the cache, which is exactly what \
you want.  A test verifies that the fix works.

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

PR Comment: https://git.openjdk.org/jfx/pull/1072#issuecomment-1485983070


[prev in list] [next in list] [prev in thread] [next in thread]