On Wed, 11 Mar 2020 12:51:12 GMT, Kevin Rushforth <[email protected]> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java
>> line 550:
>>
>>> 549: }
>>> 550:
>>> 551: private void setFillOverride(Paint fillOverride) {
>>
>> The issue can be alternatively fixed using below change at line#510
>> text.fontProperty().addListener(new
>> WeakChangeListener<>((observable, oldValue, newValue) -> {
>> doneTextWidth = Utils.computeTextWidth(text.getFont(), DONE,
>> 0);
>> doneTextHeight = Utils.computeTextHeight(text.getFont(),
>> DONE, 0, TextBoundsType.LOGICAL_VERTICAL_CENTER);
>> }));
>>
>> I am not sure which one is better.
>> As there are no objections, I am good with the any.
>
> Nope. This won't work, since the listener will be garbage collected and stop
> being called. It could be done by keeping
> the listener is an instance variable, but I recommend the original fix.
@arapte the (mine :) general rule to follow is to use the highest abstraction
available - here that would be to favour
skin's api to un/register listeners to properties (which ideally is thoroughly
tested) over ad-hoc manual code (which
might not work as expected <g>)
sry if I confused anybody with my earlier comment ...
-------------
PR: https://git.openjdk.java.net/jfx/pull/71