#18579 dijit/form/HorizontalSlider _bumpValue imprecise

[arunasr]

Using bump action (the increment/decrement buttons or keyboard arrows) results in floating values slightly off the expected value, resulting in really annoying long floating numbers with 15 digits after decimal point.
Strategically placed Math.round() fixes this behaviour.

[wkeese]

Wow, you are coming up w/a lot of patches. Seems reasonable although I wonder why this issue hasn't shown up in the regression tests. I see the Slider_a11y.html is sadly using a private inSlider._bumpValue(-1, false); method rather than actually clicking the left/right arrow keys. That doesn't reproduce the rounding problem?

The existing Slider_mouse.html test is also disappointing. ISTM it only tests the buttons as a way to set the slider to its min/max value, rather than a normal increment from (for example) 5 to 6. I also noticed though that it has this code:

doh.is(Number(finalValue).toFixed(2), Number(slider.get('value')).toFixed(2));

Maybe the toFixed() calls are no longer needed after your patch?

[arunasr]

The patches are out of my cumulative experience with Dojo over the last 6 months, and this one is the last so far. I have the same grievance with dojox/dgauges, but haven't got to the bottom of that one.
Not familiar with Dojo UT framework (yet), so unsure why it hasn't picked it up. Perhaps it depends on the min/max values used. Simply calling ._bumpValue() should pick it up, for some combinations of min/max/value at least. It could be that the test has exact same floating point error and therefore passes.
Perhaps "toFixed" were added by someone to work around the problem; in my experience the patch fixes the problem and rounding should not be required anymore.

[wkeese]

I see.

Unfortunately, thinking about this patch more, I'm worried that it will break sliders where min and/or max are fractional values. Haven't tried it, but if there's a slider from (for example) 0.00 to 0.10.

[arunasr]

Don't worry. At this point (line 258) "value" is the step number, i.e. by definition a natural number. The fractional slider value is worked out on line 265.
I have fractional sliders, they work as expected.

[arunasr]

re: toFixed() in the test case.
Having second thoughts on that one. It is hard to expect floating values to match precisely, so some rounding is perhaps required. .toFixed(2) is however far too crude, I should think; I'd suggest something like (finalValue-sliderValue).toPrecision(15) === 0

[wkeese]

So, can you give an example of what this fixes? I thought it was for when discreteValues is specified. The API doc says:

For example, if minimum=10, maxiumum=30, and discreteValues=3, then the slider handle has
three possible positions, representing values 10, 20, or 30.

I assumed that in the above case it wasn't working correctly and get("value") was actually returning 10.00000001, and that your patch fixed that.

[arunasr]

Youp, correct...to give a hypothetical example, if you have minimum 0, maximum 2 and discrete values 101 (i.e. step 0.02), on the 41st position you might get 0.819999999999999993 instead of 0.82. This is because the step number calculated was not natural 41, but a floating point number, something like 40.999...
The patch fixes this, and the multiplication later produces the correct value of 0.82.

[wkeese]

OK, understood. Hey, hopefully this patch fixes https://bugs.dojotoolkit.org/ticket/7176 too.

[arunasr]

I hope this makes #7176 irrelevant. I did not come across a combination of parameters that would produce imprecise values after the patch. Needless to say, I did not test every combination of ranges and steps.

[wkeese]

OK sounds good. I spent some time building a test case for this, and I pushed it along with your patch to the code in 12d4980. Thanks for the contribution.