VoiceOver bug computing the "step" of AccessibleValueHandler

[zepumph]

In phetsims/molarity#57 @twant and I accidentally left out a11yDecimalPlaces in VerticalSlider.js. This made the "step" of the input set to 1, but the keyboard step size is .05 and this shift keyboard step size is .001. This didn't CAUSE any bugs, but it did seem buggy while trying to look into some other problems in accessible slider.

I am interested in the following assertions where step is set in AccessibleSlider.

Turn:
this.setAccessibleAttribute( 'step', Math.pow( 10, -this.a11yDecimalPlaces ) );

into

         const stepSize = Math.pow( 10, -this.a11yDecimalPlaces );
         assert && assert( stepSize <= options.keyboardStep );
         assert && assert( stepSize <= options.shiftKeyboardStep );
         this.setAccessibleAttribute( 'step', stepSize );

The issue with this is that a11yDecimalPlaces is overloaded to also be used when formatting the aria valuetext output in AccessibleValueHandler. Basically I'm worried about the following case:

the shift keyboard step size is .0001 because you can make micro adjustments. This means that you MUST make the a11yDemicalPlaces be 4. This means that you are trapping the look of the aria valuetext to have a lot of decimal places.
Likely these should be separate options to avoid this weirdness, but again I'm a bit wafty as to the actual problem I have in this issue. More it just feels like a code smell. Also note that when adding those assertions in, 6 a11y fuzzing sims failed, suggesting that this is worth looking into.

Another weirdness that I was finding was that it didn't seem like the step attribute was actually doing anything. Since our slider implementation is a custom one, the listeners adjust the Property value independent of the html.

@jessegreenberg what do you think? Should we tie these two options together with these assertions, or make a new step related option for AccessibleSlider that just makes the slider work as expected.

[jessegreenberg]

I am not sure if I understand the issue yet, but the step attribute set for the HTML element is an implementation detail of the AccessibleSlider, just for browser/AT compatibility. Sorry if this caused confusion.

// The step attribute must be non zero for the accessible input to receive all accessibility events, and
// only certain values are allowed depending on the step size, or else the AT will announce values as
// "Invalid". If the step size is equal to the precision of the slider readout, all values are allowed.

And so stepSize !== slider.keyboardStep.

I agree that these assertions would be more helpful, I can't think of any time that keyboardStep would be larger than shift or page keyboard steps.

     assert && assert( this.keyboardStep<= options.keyboardStep );
     assert && assert( this.keyboardStep <= options.shiftKeyboardStep );

This means that you MUST make the a11yDemicalPlaces be 4. This means that you are trapping the look of the aria valuetext to have a lot of decimal places.

This sounds correct to me, if the accessible readout can have 4 decimal places, the aria-valuetext should have 4 decimal places. Is there a case where that is not true?

[jessegreenberg]

I recently encountered a related issue in #509 and added a bunch of documentation about the why and how we currently set the step attribute in AccessibleValueHandler. Documentation added around updateSiblingStepAttribute. Would be great to discuss and/or reconsider sometime soon. It all feels like a workaround, but I haven't been able to think of anything better.

[zepumph]

Marking as a meeting for @jessegreenberg and I. At this point I think that is the fastest way to continue with this issue.

[jessegreenberg]

@zepumph and I talked about this fora while during a11y dev meeting. We reviewed the current workaround and discussed a number of alternatives.

The problem with the current workaround for VoiceOver is that it is much too fragile. It looks like

            stepValue = this._enabledRangeProperty.get().max / 100;

But for certain max values, everything will break. For example if the range is 10 to 1500, the step size will be 15, and only allow values 10, 25, 40, ...

We talked about improving this calculation so that it is more general, but most things were not general enough. The problem is that the precision for the value is a designed aspect and cannot be calculated from range or keyboard steps alone.

In the end, we considered redisigning a11yDecimalPlaces option. Perhaps it can be replaced with something else that will handle formatting values with decimals AND large values with lots zeroes.

We also considered adding another option to AccessibleValueHandler so that we can specify this on our own if we determine that this problem cannot be solved.

[jessegreenberg]

Further discussion...

Since what we are trying to solve is a VoiceOver bug, we aren't going to try to come up with a tricky workaround. We will instead 1) submit a bug report to apple about this and 2) add an option to AccessibleValueHandler to specify the basis for stepping in cases where the default calculation from a11yDecimalPlaces won't work for VoiceOver.

This option should have very clear documentation that it should only be used if the VoiceOver bug described above is encountered.

[zepumph]

Renamed post refactor.