JDK-8145567 : Slider: snapToTicks not honoured on changing to true
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: 8u60,9
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-12-16
  • Updated: 2020-01-31
  • Resolved: 2016-01-26
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u102Fixed 9Fixed
Description
To reproduce, run the example

- move thumb between two ticks 
- press button to enable snapToTicks
- expected: value (and with it the thumb) snapped to nearest tick
- actual: value/thumb unchanged, snapped at the next mouse touch only

Example:

public class SliderSnapToTickBug extends Application {

    @Override
    public void start(Stage primaryStage) {
        Slider slider = new Slider(5, 25, 15);
        // show ticks
        slider.setShowTickMarks(true);
        slider.setShowTickLabels(true);
        slider.setMajorTickUnit(10);
        
        Label valueLabel = new Label();
        valueLabel.textProperty().bind(slider.valueProperty().asString());
        
        Button snapTicks = new Button("snap to ticks");
        snapTicks.setOnAction(e -> {
            slider.setSnapToTicks(!slider.isSnapToTicks());
        });
        VBox root = new VBox(10, slider, valueLabel, snapTicks);
        primaryStage.setScene(new Scene(root, 500, 400));
        primaryStage.show();
    }
 
    public static void main(String[] args) {
        launch(args);
    }
    
}

Reason: SliderSkin doesn't listen to changes of the snapToTicks property
Fix: add listener and adjust value when changed to true

// in SliderSkin constructor
registerChangeListener(slider.snapToTicksProperty(), "SNAP_TO_TICKS");

// in handleControlProperty
} else if ("SNAP_TO_TICKS".equals(p)) {
    if (slider.isSnapToTicks()) {
        slider.adjustValue(slider.getValue());
    }
}

the fix is without any risk and well bounded - so could be added at once :-)




Comments
This looks safe and straight-forward enough to take for an 8u backport. I also verified that the new unit test catches the bug if the fix is not applied and passes with the fix applied. Approved to backport to 8u-dev (8u82).
27-01-2016

@Vadim, @Jonathan - whatever the final decision, thanks for your effort :-)
27-01-2016

I have reviewed the backport and am happy with the changes, so +1 on that. What I can't comment on is the scope for doing a backport at this point. I assume it is feasible, but Kevin knows better.
26-01-2016

Jonathan, Kevin, Could you please review and approve a backport for 8u82? It's not identical due to the Skin public API changes but essentially the same. http://cr.openjdk.java.net/~vadim/8145567/webrev.8u/
26-01-2016

how about a backport? Just the listener calling slider adjustValue, a one-liner no risk
26-01-2016

Changeset: df5df72e4a7a Author: vadim Date: 2016-01-26 12:04 +0300 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/df5df72e4a7a
26-01-2016

+1 Sorry about the delay reviewing this, it fell completely off my plate.
25-01-2016

Jonathan, Please review this fix: http://cr.openjdk.java.net/~vadim/8145567/webrev.00/ SliderBehavior.java change is not really relevant, I just found out that snapValueToTicks method there is not needed since its functionality is the same as Slider.adjustValue.
19-01-2016