Checkboxes won't check in Firefox and NVDA

[zepumph]

@kainip mentioned to @terracoda and @zepumph that this was a problem for her. I am able to reproduce with GFLB with

https://phet-dev.colorado.edu/html/phet-io-wrapper-sonification/1.0.0-dev.101/phet-io-wrapper-sonification/gravity-force-lab-basics/html/gravity-force-lab-basics-sonification.html?sonificationFile=sonificationOptions&massSound=0&forceSound=3&massBoundaryLimitSound=2&accessibility

as well as in master.

Again this is only an issue with Firefox with NVDA turned on. I can't reproduce in Chrome.

[jessegreenberg]

I am able to reproduce in FF + NVDA with checkboxes in other sims too.

EDIT: I copied the checkbox HTML form GFL:B and tested in a JSFiddle, they are working outside of the sim: https://jsfiddle.net/wm3vf02o/

EDIT: the first time I tab to the check box and try to activate it I see this in the console:

If I use the virtual cursor to navigate to the check box and activate it I see this error:

So to me this is looking like NVDA is sending fake pointer events on activation, rather than keyboard events.

EDIT: When NVDA is enabled the fire listener is getting triggered twice. Likely once for the change event, and once for the touchStart.

EDIT: Indeed, we have both change and touchStart events when using NVDA, but only change events without NVDA with this JSFiddle. https://jsfiddle.net/hw53xdgu/1/

This is looking related to https://github.com/phetsims/scenery/issues/852, but I would have expected all pointer related events to be blocked on PDOM elements.

EDIT: This is not happening for the Checkbox (TwoSceneSelectionNode) in BASE. The difference is that that checkbox uses aria-label instead of a label element.

EDIT: The target of the touch start event is the input element in the PDOM. Would it be acceptable to block all touch events on those?

[jessegreenberg]

From the above comments, I am more certain that this is caused by https://github.com/phetsims/scenery/issues/852 so moving to that issue.

[jessegreenberg]

@zepumph I think this is fixed in master after the above commit. Can you please verify with NVDA? Also, do you have any concerns about the fix?

[jessegreenberg]

Sorry, not ready for review. I noticed that adding these event listeners to all elements make NVDA say that they are "clickable". So I am hearing "Clickable heading level 1" and so on.

So maybe the listeners that preventDefault and stopPropagation can be added to just certain elements or just focusable elements.

[jessegreenberg]

It appears that preventing pointer like events on the root of the PDOM will also fix this ('touchstart', 'touchend', 'mousedown', 'mouseup')

[terracoda]

@jessegreenberg, do you think preventing pointer-like events will also fix this issue about everything being "clickable"? https://github.com/phetsims/gravity-force-lab-basics/issues/110

[jessegreenberg]

I found on this thread that just adding an event listener for click, mouseDown or mouseUp will make NVDA report that the element is "clickable". Moving the listeners with preventDefault off of individual DOM elements and just onto the PDOM root should prevent the AT from saying clickable while reading through content.

[terracoda]

@jessegreenberg, I also found some information in the WebAIM email archive (posted in https://github.com/phetsims/gravity-force-lab-basics/issues/110).

Great we'll see if that fixes it.

In the WebAim archive links I think Mike Moore mentioned that an OnClick event handler added to the body element can also cause this undesired behaviour in screen readers (WebAim email archive, April 2015).

We don't put an OnClick event on the body do we?

[jessegreenberg]

We don't put click listeners on the body, but we do put click listeners on the root element of the PDOM. Do you happen to a have a link to that archive? Or recall what some of the undesired behaviors were?

[jessegreenberg]

Sorry, I misread, you said

this undesired behaviour in screen readers

I thought you were referring to other undesired behaviors.

[terracoda]

In this comment I posted the links to the webaim arch https://github.com/phetsims/gravity-force-lab-basics/issues/110#issuecomment-475705630

I may not fully understand the event listener situation, but this behaviour of VoiceOver saying "clickable" on almost everything is relatively new.

[jessegreenberg]

After the above commit, NVDA is no longer saying "clickable" on every readable element. But since the click listener is still on the root of the PDOM, NVDA does say "clickable" the first time (and only the first time) the virtual cursor enters the simulation.

@terracoda can you please test master and see what VO does?

According to the NVDA thread above, clickable won't be reported on the following roles:

However, if the element is inherently clickable or it just isn't useful to report clickable for that element, it won't be reported. Currently, this means that it isn't reported for the following control types: ROLE_LINK, ROLE_BUTTON, ROLE_CHECKBOX, ROLE_RADIOBUTTON, ROLE_TOGGLEBUTTON, ROLE_MENUITEM, ROLE_TAB, ROLE_SLIDER, ROLE_DOCUMENT, ROLE_CHECKMENUITEM, ROLE_RADIOMENUITEM

I tried adding role="document" to the root of the PDOM but I still hear clickable when entering the sim for the first time.

I don't know if this is something we need to prevent. I feel (as do others in https://github.com/nvaccess/nvda/issues/5830) that it is an incorrect screen reader design decision to read things as clickable when an ancestor has a click listener.

[terracoda]

@zepumph and @jessegreenberg, I am not hearing "clickable" any more while using the cursor keys to read through the PDOM.

[jessegreenberg]

Great to hear, thanks for testing. @terracoda regarding https://github.com/phetsims/a11y-research/issues/133#issuecomment-478011466 do you have any large concerns about NVDA saying "clickable" when the user first enters the simulation?

[terracoda]

Personally, if NVDA only says it once, I think we are good to go, and hopefully this issue will disappear as screen readers improve.

[jessegreenberg]

OK, in that case I think we have a reasonable solution. @zepumph can you please also review https://github.com/phetsims/scenery/commit/9339a9c5c1a130c66ea93b26a4ee6890fc3f1ed5 and verify this is fixed?

[zepumph]

This has been working now for a while for me. Thanks for investigating this!