I had a break through the JDK source.
The NPE is thrown from if (newValue < getMin()) { in the listener's lambda here:
javafx.scene.control.SpinnerValueFactory.java
public IntegerSpinnerValueFactory(@NamedArg("min") int min, @NamedArg("max") int max, @NamedArg("initialValue") int initialValue, @NamedArg("amountToStepBy") int amountToStepBy) { setMin(min); setMax(max); setAmountToStepBy(amountToStepBy); setConverter(new IntegerStringConverter()); valueProperty().addListener((o, oldValue, newValue) -> {
supposedly newValue is null , and automatically unlocking null causes NPE. Since the input comes from the editor, I suspect that IntegerStringConverter is the default converter.
Take a look at the implementation here:
javafx.util.converter.IntegerStringConverter
public class IntegerStringConverter extends StringConverter<Integer> { @Override public Integer fromString(String value) {
We see that it will happily return null for an empty string, which is reasonable given that there is no valid value for input.
By tracking the call stack, I find where this value comes from:
javafx.scene.control.Spinner
public Spinner() { getStyleClass().add(DEFAULT_STYLE_CLASS); setAccessibleRole(AccessibleRole.SPINNER); getEditor().setOnAction(action -> { String text = getEditor().getText(); SpinnerValueFactory<T> valueFactory = getValueFactory(); if (valueFactory != null) { StringConverter<T> converter = valueFactory.getConverter(); if (converter != null) { T value = converter.fromString(text); valueFactory.setValue(value); } } });
The value is set with the value obtained from the converter T value = converter.fromString(text); which is supposedly null. At this point, I believe that the spinner class should check that value not null , and if it restores the previous value in the editor.
Now I'm sure this is a mistake. Moreover, I don’t think that working with a converter that never returns null will work correctly, as this will only mask the problem and what value should be returned when the value cannot be converted?
Edit: workaround
Replacing the onAction counter editor to reject invalid input using the back to action policy fixes the problem:
public static <T> void fixSpinner2(Spinner<T> aSpinner) { aSpinner.getEditor().setOnAction(action -> { String text = aSpinner.getEditor().getText(); SpinnerValueFactory<T> factory = aSpinner.getValueFactory(); if (factory != null) { StringConverter<T> converter = factory.getConverter(); if (converter != null) { T value = converter.fromString(text); if (null != value) { factory.setValue(value); } else { aSpinner.getEditor().setText(converter.toString(factory.getValue())); } } } action.consume(); }); }
Unlike the listener on valueProperty , this allows you to run other listeners with invalid data. However, this highlights another problem in the spinner class. While the above fixes the problem by returning a valid value when pressing enter. Erasing the input without committing (pressing enter) and then incrementing or decreasing will result in the same NPE, but with a slightly different call stack.
Cause:
public void increment(int steps) { SpinnerValueFactory<T> valueFactory = getValueFactory(); if (valueFactory == null) { throw new IllegalStateException("Can't increment Spinner with a null SpinnerValueFactory"); } commitEditorText(); valueFactory.increment(steps); }
The reduction seems to be both invoked in commitEditorText below:
private void commitEditorText() { if (!isEditable()) return; String text = getEditor().getText(); SpinnerValueFactory<T> valueFactory = getValueFactory(); if (valueFactory != null) { StringConverter<T> converter = valueFactory.getConverter(); if (converter != null) { T value = converter.fromString(text); valueFactory.setValue(value); } } }
Note the copy-paste from onAction in the constructor:
getEditor().setOnAction(action -> { String text = getEditor().getText(); SpinnerValueFactory<T> valueFactory = getValueFactory(); if (valueFactory != null) { StringConverter<T> converter = valueFactory.getConverter(); if (converter != null) { T value = converter.fromString(text); valueFactory.setValue(value); } } });
I believe that commitEditorText should be changed to call onAction in the editor, and not like this:
private void commitEditorText() { if (!isEditable()) return; getEditor().getOnAction().handle(new ActionEvent(this, this)); }
then the behavior will be consistent and will give the editor the ability to process the input data before it moves to the factory value.