On Wed, 15 Apr 2020 23:18:46 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8193286 >> >> Root Cause : >> Incorrect implementation. >> Current implementation of int wrapValue(int,int,int) in Spinner.java works >> well if min is 0. >> Hence this implementation works with ListSpinnerValueFactory, but fails with >> IntegerSpinnerValueFactory. >> >> Fix : >> Added a method for IntegerSpinnerValueFactory with separate implementation. >> >> Testing : >> Added unit tests to test increment/decrement in multiple steps and with >> different amountToStepBy values. >> >> Also, identified a related behavioral issue >> https://bugs.openjdk.java.net/browse/JDK-8242553, which will be addressed >> separately. > > I'm not sure this is quite right, but I need to look at it more closely. It > should fix the bug in question for > well-behaved values, but might make it possible to return a value outside the > range of [min, max]. > I also want to think about it in light of > [JDK-8242553](https://bugs.openjdk.java.net/browse/JDK-8242553). While a > partial fix seems OK, we need to avoid the situation where some values cause > a change from one buggy behavior to a > different buggy behavior on the way to fixing it right. The fix works for well behaved values - and - maintains buggy behavior (to be addressed in JDK-8242553) if (amountToStepBy * no of steps) is greater than the total numbers between min and max. The Spinner.wrapValue() does return value outside min-max range if (amountToStepBy * steps) is greater than total numbers between min and max. It gets clamped to either min or max based on condition in IntegerSpinnerValueFactory valueProperty() listener. I am planning to change Spinner.wrapValue() to use modulo as part of JDK-8242553. This fix is restricted to addressing the issue of stepping through of valid (amountToStepBy * no of steps) which was reported as bug. ------------- PR: https://git.openjdk.java.net/jfx/pull/174
