Replaced bindBidirectional calls by a pair of ChangeListener to bind scrollbar coordinates and content position. #98
Conversation
…scrollbar coordinates and content position. This is needed because original solution no longer works after https://bugs.openjdk.java.net/browse/JDK-8264770
|
Thank you very much for reporting, researching, and PR submission !!! |
| Bindings.bindBidirectional(hbarValue, hPosEstimate); | ||
| Bindings.bindBidirectional(vbarValue, vPosEstimate); | ||
| hbarValue.addListener((observable, oldValue, newValue) -> hPosEstimate.setValue(newValue)); | ||
| hPosEstimate.addListener((observable, oldValue, newValue) -> hbarValue.setValue(newValue)); |
There was a problem hiding this comment.
It looks like the object referenced by hPosEstimate is eligible for garbage collection once the variable goes out of scope. The bidirectional binding will not make the object strongly reachable, since bidirectional bindings in JavaFX use weak listeners (and have always done so).
The new code captures a reference to hPosEstimate and strongly references it from the listener of hbarValue, which itself is strongly referenced. This is probably why it works, and why the original doesn't (and should never have, so there's the question why it once worked in the first place).
There was a problem hiding this comment.
Thanks for looking into this, @mstr2
Following your remark, I did a quick test reverting all my previous change and instead declaring hPosEstimate and vPosEstimate as final members of the class to capture a strong ref to these.
It did not, however, have the expected behavior (i.e. scoll bar was broken, as with the original code).
This PR addresses issue #97
It replaces bindBidirectional calls by a pair of ChangeListener to bind scrollbar coordinates and content position.
This is needed because original solution no longer works after https://bugs.openjdk.java.net/browse/JDK-8264770