phetsims / a11y-research

a repository to track PhETs research into accessibility, or "a11y" for short
MIT License
3 stars 0 forks source link

NVDA doesn't read aria-valuetext from keydown event #51

Closed jessegreenberg closed 6 years ago

jessegreenberg commented 7 years ago

NVDA doesn't read aria-valuetext when it is set from a keydown event listener. It assumes that the value is changing from the change or input events. This is problematic because our custom implementation of the slider changes the input value on the keydown and prevents default so that change and input are never fired. I wonder if this is a problem for JAWS.

jessegreenberg commented 7 years ago

JAWS reads the aria-valuetext correctly in Firefox.

jessegreenberg commented 7 years ago

I tried setting the aria-valuetext in both keydown and change while preventing default in the change event. It stopped working altogether because without event.preventDefault in the keydown listener, the AT sets the value again in the change listener.

jessegreenberg commented 7 years ago

We should send a bug report to NVDA.

jessegreenberg commented 7 years ago

In the meantime one possible workaround would be to read the updated value in an alert. This would only work if we can silence all other value updates from the screen reader, which means that we cannot use aria-valuetext and we probably do not want to update the input value. We will also need to make sure that the value text is read when the slider gets focus.

@terracoda do you feel OK with us trying out this workaround?

jessegreenberg commented 7 years ago

It isn't working very well, I am having a hard time making the screen reader not say anything when the value changes, event with aria-valuetext=" "

jessegreenberg commented 7 years ago

What if we stop using input type="range" PhET sliders? We are preventing default for all typical input events anyway.

jessegreenberg commented 7 years ago

If we replace <input type="range"> with <div role="slider"> everything is sounding great in both VoiceOver and NVDA.

jessegreenberg commented 7 years ago

And its sounding good in JAWS too. I feel much better about this than https://github.com/phetsims/a11y-research/issues/51#issuecomment-321022893, no hacky silencing or workarounds, just replacing input type with an aria role.

@terracoda can you please review this change and let me know if you think all is well? In particular, the conclusion is that cannot be used for PhET sliders because of this NVDA problem, and that <div role="slider"> must be used instead?

jessegreenberg commented 7 years ago

spot checking before commiting and the arrow keys are no longer working in NVDA 😠

terracoda commented 7 years ago

@jessegreenberg, I am reading through the issue. I am assuming this is related to sliders for Ohm's Law.

It might be helpful to see how sliders in JT were implemented (with or without aria-valuetext). I have a very vague memory that those sliders had trouble with aria-valuetext. Again, this is a vague memory.

jessegreenberg commented 7 years ago

It looks like role=slider only works in NVDA when in forms mode.

jessegreenberg commented 7 years ago

@terracoda john-travoltage used <input type=range> with aria-valuetext. However, the value text was set in the change and input events, not in the keydown event which is why it is broken in NVDA.

That time, the issue was that VoiceOver read the aria-valuetext twice.

jessegreenberg commented 7 years ago

@terracoda correct, this is for ohms-law and resistance-and-a-wire, but it will apply to all sliders used in PhET sims.

terracoda commented 7 years ago

@jessegreenberg re "forms mode" https://github.com/phetsims/a11y-research/issues/51#issuecomment-321035353, are you saying that <div role="slider"> is not recognized as an interactive element?

jessegreenberg commented 7 years ago

Correct @terracoda, NVDA does not recognize role=slider by default. It requires tabIndex="0" to receive focus and isn't even read as a slider unless the user is in forms mode :(

jessegreenberg commented 7 years ago

This is a consequence of using "special" sliders that support non standard ranges and steps.

jessegreenberg commented 7 years ago

So how about <div role="application" aria-roldescription="Slider">? And then we go through our alert system to announce the new value?

terracoda commented 7 years ago

@jessegreenberg, did you try <input type="range" role="slider">?

jessegreenberg commented 7 years ago

I did not, will try that first.

jessegreenberg commented 7 years ago

@terracoda that is working beautifully in NVDA + Firefox :)

NVDA moves to forms mode when focus lands on teh slider, and it is now correctly reading the aria-valuetext during the keydown event.

terracoda commented 7 years ago

@jessegreenberg, I was also going to suggest something similar to your using role="application" in https://github.com/phetsims/a11y-research/issues/51#issuecomment-321038262.

terracoda commented 7 years ago

hip, hip, horray!

terracoda commented 7 years ago

@jessegreenberg, there are so many attributes between ARIA and HTML5, it is very difficult to know when, where, and how to combine or NOT COMBINE them for the best results.

jessegreenberg commented 7 years ago

And its working great in VoiceOver and JAWS too! Thanks so much for the help @terracoda!

terracoda commented 7 years ago

Thank you @jessegreenberg! I learn so much by working with you!

jessegreenberg commented 7 years ago

there are so many attributes between ARIA and HTML5, it is very difficult to know when, where, and how to combine them for the best results.

Indeed. Do you happen to know if this combination is recommended by the ARIA spec? If not, the fact that one needs both type="range" and role="slider" for aria-valuetext to work properly makes me think this is still an NVDA bug.

terracoda commented 7 years ago

@jessegreenberg, no I don't know. It was just intuitive guess. Since range is not a role, I thought adding an explicit role could be a good thing. I am not sure what the default role is for <input type="range">.

terracoda commented 7 years ago

Hmm, according to the list of roles in ARIA 1.1 , the input and range roles are both abstract roles, so adding a more specific UI role such as slider on these types of elements is a good thing.

terracoda commented 7 years ago

@jessegreenberg, I'm not sure it is an actual NVDA bug, but we could still send a question to NVDA. It would be interesting to understand how NVDA's implementation differs from VO and JAWS. For example, are VO and JAWS assumingrole=slider by default on <input type="range">?

jessegreenberg commented 7 years ago

Thanks for the spec list @terracoda that is interesting. In my mind, input and range roles are abstract, but that doesn't mean that <input type="range"> should ever be abstract. It should be up to AT to assign the correct role to elements through the accessibility API.

I think we have found two shortcomings of NVDA in this issue 1) NVDA does not support role=slider on anything other than an input of type range. 2) aria-valuetext requires both role="slider" and type="range" to work properly in NVDA which seems incorrect.

terracoda commented 7 years ago

@jessegreenberg, when you put it like that, it does indeed seem like bugs in NVDA's implementation. Thanks for the clarification.

terracoda commented 6 years ago

@jessegreenberg, can this issue be closed? We have a work around for NVDA when it comes to sliders.

jessegreenberg commented 6 years ago

I encountered this bug again today when trying to test aria-valuetext in a JSFiddle. I submitted a bug report to Bugzilla about this: https://bugzilla.mozilla.org/show_bug.cgi?id=1475376. @terracoda sorry I didn't see that comment, I think we should leave this open until we get clarification about the nature of this behavior.

jessegreenberg commented 6 years ago

Removing assignments until we learn more.

jessegreenberg commented 6 years ago

Good news, got a response already:

Confirmed. We're not honouring aria-valuetext for the implicit role="slider" in this case.

terracoda commented 6 years ago

@jessegreenberg, but they will if the role is "explicit"?

jessegreenberg commented 6 years ago

Exactly, works OK when role="slider" is on the element.

terracoda commented 6 years ago

So, are we good? We just stick with <input type="range" role="slider">

jessegreenberg commented 6 years ago

Yes, I think we should be good with the workaround. Mozilla works fast, they just updated the status of the bug to fixed! It was marked with status-firefox63: --- → fixed. I think latest FF is 61.0.1, so maybe we will see this fixed when 63.0 is released? In the bug report I can see what looks like a commit message "Support aria-valuetext for implicit ARIA roles." So maybe aria-valuetext will work better on other native elements too.

I think this issue can now be closed, but @terracoda please reopen if you think there is anything else.