Xamarin.forms: Slider crashes when Minimum value greater than zero and set before Max value in XAML

[Paul-Brenner-Tangoe]

Description

Migrating https://bugzilla.xamarin.com/show_bug.cgi?id=43435 here

The difference between the working and crashing versions is the order of the Minimum and Maximum properties in the XAML. Note that if Minimum is set to zero, the order is no longer an issue.

Steps to Reproduce

Run app, note that it works fine
Uncomment line #8 in MainPage.xaml
Run app, note it crashes with "System.ArgumentException: Value was an invalid value for Minimum"

Expected Behavior

App does not crash

Actual Behavior

App crashes

Basic Information

• Version with issue: 2.5.0.280555
• Last known good version: N/A
• IDE: VS2017
• Platform Target Frameworks:
  
  
  
  • iOS: yes
  
  
  • Android: yes
  
  
  • UWP: not tested
  
  
• Android Support Library Version: 25
• Nuget Packages: just forms
• Affected Devices: tested on Nexus 5x running Android 7, and iPhone 5s simulator running iOS 9.3

Reproduction Link

https://www.dropbox.com/s/qdweqnvh1hsj4wh/slidercrashsample.zip?dl=0

[CZEMacLeod]

I had a similar issue in pure code with something like

Slider s = new Slider() { Minimum = 1000, Maximum = 2000 };

I had to change it to

Slider s = new Slider() { Maximum = 2000, Minimum = 1000 };

I have a feeling that the blocks/errors should not be raised until it is fully initialized.
Also what if you want a slider going from 100->0?

[sayedihashimi]

A customer I'm working with has also run into this. He said it took him about an hour to figure out he needed to set Maximum before Minimum. Also he stated that when developing in XAML with UWP there was no such requirement for Maximum to be before Minimum.

[charlesroddie]

The current behavior is absurd. https://docs.microsoft.com/en-us/xamarin/xamarin-forms/user-interface/slider
To set (Minimum, Maximum) = (2,3), you have to set maximum first, then minimum.
To set (Minimum, Maximum) = (-3,-2), you have to set minimum first, then maximum.

My suggestion (expanded from https://github.com/xamarin/Xamarin.Forms/pull/2765#issuecomment-420835011):

Min, Value, Max are _valid_ when Min <= Value <= Max. A _temporary glitch_ happens when in the process of setting the properties in some order, there is a momentary incorrectness in one value. Temporary glitches are OK but invalid state (this issue) is not since it may crash the UI. Permanent glitches (https://github.com/xamarin/Xamarin.Forms/issues/3521) are also not OK.

Setting Min, Value, Max should set internal mutable variables representing the currently requested values. Getting Value, Min, Max gets a valid function of the these requested values. This would not prevent temporary glitches but does prevent permanent glitches and invalid state.

Glitch-free operation can be done with an extra property containing all three values.

type SliderValues(minimum:float, value:float, maximum:float) =
    member t.Minimum = minimum
    member t.Value = value
    member t.Maximum = maximum
    member t.ForceValid =
        SliderValues(
            Math.Min(Math.Min(minimum,value),maximum),
            Math.Min(Math.Max(minimum,value),maximum),
            Math.Max(Math.Max(minimum,value),maximum)
            )
    member t.IsValid = minimum <= value && value <= maximum

type Slider() =

    let mutable reqValues = SliderValues(0.,0.,1.)
    let mutable validValues = SliderValues(0.,0.,1.)

    /// Moving from old valid values to new valid values, changes UI/binding properties
    /// one at a time, keeping validity at each update.
    let updateSliderValues(newValues:SliderValues) =
        validValues <- newValues
        let o1,o2,o3 = validValues.Minimum, validValues.Value, validValues.Maximum
        let n1,n2,n3 = newValues.Minimum, newValues.Value, newValues.Maximum
        if   n3 >= o3 && n2 >= o2 then () // update UI with 3 then 2 then 1
        elif n3 >= o3 && n2 < o2 then () // 3 then 1 then 2
        elif n1 < o1 && n2 < o2 then () // 1 then 2 then 3
        elif n1 < o1 && n2 >= o2 then () // 1 then 3 then 2
        else // n1 >= o1 && n3 < o3
            () // 2 then 1 and 3

    member t.Minimum
        with get() = validValues.Minimum
        and set v =
            reqValues <- SliderValues(v,reqValues.Value,reqValues.Maximum)
            updateSliderValues(reqValues.ForceValid)
    member t.Value
        with get() = validValues.Value
        and set v =
            reqValues <- SliderValues(reqValues.Minimum,v,reqValues.Maximum)
            updateSliderValues(reqValues.ForceValid)
    member t.Maximum
        with get() = validValues.Maximum
        and set v =
            reqValues <- SliderValues(reqValues.Minimum,reqValues.Value,v)
            updateSliderValues(reqValues.ForceValid)
    // set all values in a glitch free way with proper error handling
    member t.SliderValues
        with get() = validValues
        and set (v:SliderValues) =
            if v.IsValid then
                reqValues <- v
                updateSliderValues(v)
            else failwith "Incorrectly ordered slider values"

[charlesroddie]

I have updated my proposal for clarity. @samhouts @StephaneDelcroix any comments?

[jwosty]

I would even suggest that minimum be allowed to be greater than maximum. It _does_ make sense to have a slider that directly outputs values from, say, 10 to 0, without having to map it myself in an unnecessary step. Honestly, what kinds of bugs is throwing this error even preventing, anyway?

[charlesroddie]

@jwosty I agree somewhat but to do that would require renaming Minimum and Maximum and avoiding usage of native platform controls, i.e. a change in both api and philisophy. Probably a good change, but not a quick fix.

[krdmllr]

I would suggest to simply set the max value to the min value if the min value is greater then the max value and vise versa to avoid the crash when setting up the control.

[chucker]

@krdmllr yes, well, it's not very intuitive that this will throw:

<Slider Grid.Column="1" Grid.Row="1" Minimum="1" Maximum="10" Value="3" />

But this won't:

<Slider Grid.Column="1" Grid.Row="1" Maximum="10" Minimum="1" Value="3" />

I would even argue the former is a more natural way of writing it.

[tompi]

Just wasted some time on this as well(latest forms version)...

[thomasgalliker]

Is there anyone working on this issue? Or is there at least a cheap workaround for this problem?

[charlesroddie]

@thomasgalliker people have been complaining about this for many years but it just gets ignored. Using my method above you can workaround the bugs by creating a safe version of a slider:

/// A slider with SafeMinimum, SafeValue, SafeMaximum properties which allows setting these values without crashing.
type SafeSlider() =
    inherit Slider()

    let mutable reqValues = SliderValues(0.,0.,1.)
    let mutable validValues = SliderValues(0.,0.,1.)

    /// Moving from old valid values to new valid values, changes UI/binding properties
    /// one at a time, keeping validity at each update.
    let updateSliderValues(newValues:SliderValues, s:Slider) =
        let o1,o2,o3 = validValues.Minimum, validValues.Value, validValues.Maximum
        validValues <- newValues
        let n1,n2,n3 = newValues.Minimum, newValues.Value, newValues.Maximum
        let set1() = s.Minimum <- n1
        let set2() = s.Value <- n2
        let set3() = s.Maximum <- n3
        if   n3 >= o3 && n2 >= o2 then set3(); set2(); set1()
        elif n3 >= o3 && n2 < o2 then set3(); set1(); set2()
        elif n1 < o1 && n2 < o2 then set1(); set2(); set3()
        elif n1 < o1 && n2 >= o2 then set1(); set3(); set2()
        else // n1 >= o1 && n3 < o3
            set2(); set1(); set3()

    member t.SafeMinimum
        with get() = validValues.Minimum
        and set v =
            reqValues <- SliderValues(v,reqValues.Value,reqValues.Maximum)
            updateSliderValues(reqValues.ForceValid, t)
    member t.SafeValue
        with get() = validValues.Value
        and set v =
            reqValues <- SliderValues(reqValues.Minimum,v,reqValues.Maximum)
            updateSliderValues(reqValues.ForceValid, t)
    member t.SafeMaximum
        with get() = validValues.Maximum
        and set v =
            reqValues <- SliderValues(reqValues.Minimum,reqValues.Value,v)
            updateSliderValues(reqValues.ForceValid, t)
    /// Set all values in a glitch free way with proper error handling
    member t.SliderValues
        with get() = validValues
        and set (v:SliderValues) =
            if v.IsValid then
                reqValues <- v
                updateSliderValues(v, t)
            else failwith "Incorrectly ordered slider values"