Alerts only work 70% of the time on JAWS and FF 60

[jessegreenberg]

While looking into #164 I noticed that alerts like "As letter rho grows, letter R grows" only come through sometimes. Possibly related to performance issues with JAWS and Firefox quantum these days, see phetsims/a11y-research#102.

[jessegreenberg]

@lmulhall-phet is this something you have noticed during recent QA testing?

[ghost]

I haven't noticed it during recent QA testing. That being said, @JRomero0613 does more Windows testing than I do, so perhaps he could add his two cents.

[JRomero0613]

@jessegreenberg During testing I have seen that JAWS and Firefox do not read out "As letter rho grows, letter R grows" about half the time. I have tested on the same machine with the same version of Firefox with NVDA and it reads out the correct alert every time, unlike JAWS.

[ghost]

I think we should try to come up with some real statistics for alerts on JAWS + FF 60. Not just ~50% of the time. @jessegreenberg, how did you arrive at 70%?

[jessegreenberg]

Thanks @JRomero0613, thats what I am seeing too.

@lmulhall-phet sounds good, just very rough number. I moved the slider thumb about 10 times and of those movements the alerts randomly failed to be announced at least 3 times.

My initial guess is that JAWS+Firefox 60+ has a hard time reading an alert immediately after announcing the change in aria-valuetext. To start, we could create a JSFiddle to test this theory outside of a PhET sim. Verifying this could assist in finding a workaround. Otherwise it could be performance related.

[JRomero0613]

@jessegreenberg I have just done some testing to get a more accurate statistic and your original estimation of ~70% is fairly accurate. After testing 113 times using all types of keyboard inputs (small steps, big steps, home, end, and multiple in a row) the screen reader functioned properly 77 times, around 68%. This was done using the latest version in master (1.5.0-rc.7). Also of note, I did notice the same bug seen in issue here #160.

[ghost]

@jessegreenberg, I'd still like to test this in a JSFiddle. I created an issue phetsims/qa/issues/142 in the QA repo.

[jessegreenberg]

In phetsims/qa#142 I said:

[[Here is a JSFiddle to test this]] http://jsfiddle.net/4znve8cr/34/
We should hear something like:
"New value for the slider is #".
"Alert: Slider value changed to #."

Testing it with JAWS on FF 61.0.1, I get both statements 100% of the time. This indicates that this is caused by performance in RIAW. @lmulhall-phet @JRomero0613 feel free to test if you wish but I believe that this verifies that this is not a general JAWS problem.

[zepumph]

@JRomero0613 said

@jessegreenberg I have just tested the fiddle and I am also getting both statements 100% of the time. I agree that this is an issue caused by the RIAW sim.

@jessegreenberg how do you think we should proceed. RIAW is not a performance heavy simulation, so I worry if JAWS in unable to keep up. Was there ever an issue like this in BASE? Do you think it could have to do with how we have implemented the utterance queue, perhaps it has to do with the Timer's setInterval?

[jessegreenberg]

Was there ever an issue like this in BASE? Do you think it could have to do with how we have implemented the utterance queue, perhaps it has to do with the Timer's setInterval?

An issue like this was not logged in BASE. I verified that all alerts are coming through in the a11y-view, they are just not being announced by JAWS, so I din't think it has to do with Timer's setInterval. The test in http://jsfiddle.net/4znve8cr/34/ used a setInterval as well.

[jessegreenberg]

Discussed 7/17/18 - This issue is more critical than the others because the user doesn't hear the physical description of the state of the sim. We should look into a performance improvement or potential workaround. A quirky workaround is not acceptable, so we would rather proceed with this bug than come up with some time consuming workaround.

[zepumph]

That fiddle is using setTimeout, there is no queue with backed up logging being set to aria-live over an interval. To me that seems like it could be a slightly different implementation.

[jessegreenberg]

@zepumph and I discussed and came up with a test that is closer in behavior to utteranceQueue/AriaHerald: http://jsfiddle.net/4znve8cr/43/

[jessegreenberg]

Tested with the AT, and the problem is not showing up in that updated JSFiddle. @zepumph and I also tried removing code related to the redrawing of the dots and the letters, and the problem was still there. Granted, tests were done with Chrome + FF + Zoom + JAWS + more, so JAWS could be struggling with all of that at once. Even so, we are thinking that the behavior may not be related to performance, but possibly related to the implementation of utteranceQueue/AriaHerald. Even though the alerts are coming through in the a11y-view, maybe JAWS doesn't like the delay/timing of utterances.

@lmulhall-phet @JRomero0613 can you please comment on whether lacking alerts of this nature showed up in BASE or other a11y testing you have done?

[JRomero0613]

@jessegreenberg In testing BASE with JAWS again I have noticed that similar to the issue we are seeing in RIAW the alerts are not read out every time.

[ghost]

I played with BASE for a while today and I noticed this as well. I have no idea what's causing it. I thought the lack of alerts was caused by too many user input events, but I tested this theory, and it doesn't seem to be correct. @jessegreenberg what else can we do to help you pin down what's causing this?

[jessegreenberg]

Thanks @lmulhall-phet and @JRomero0613. Pinging @emily-phet and @terracoda so they are aware that this issue is in the deployed version of BASE as well.

@zepumph and I verified that the issue is not related to the performance of the graphical rendering in #165 (comment). And @mbarlow12 and I verified that this is not related to the timing of utteranceQueue/AriaHerald in phetsims/scenery-phet#389.

I am still wondering about performance, and perhaps it is caused by our frequent a11y related DOM updates. Possible JAWS or Firefox struggles in keeping the Accessibility Tree up to date. Could test this by modifying the JSFiddle with lots of DOM manipulation, and be seeing if the problem goes away by disabling things that change innerContent/innerHTML in the sim. The latter is easier, Ill start there.

[jessegreenberg]

I cannot reproduce in Chrome or IE11 either.

[jessegreenberg]

Ok, can reproduce on rc.7 if I press the arrow key a number of times before JAWS can read out a value. Also on home/end it happened a few times. I also encountered the bug where JAWS randomly moves focus around the sim when trying to press reset all

[jessegreenberg]

I had to reset my computer and now the bug is happening much more frequently again.

[jessegreenberg]

I added return statements at the top of Accessibiliity.js setInnerContent, setLabelContent and setDescriptionContent, so the PDOM was essentially empty and I am still seeing this bug. I am going to try to look for differences between our implementation of AccessibleSlilder and the JSFiddle.

[jessegreenberg]

@zepumph recommended that we go into the sun demo and recreate a test case to see if we can isolate the issue there. Also, the PDOM will be much smaller in this case.

We also noticed that there doesn't seem to be a guard around the chnage event handler to not be called after keydown is handled. How are we not stepping twice every arrow key press? We talked about removing functionality from AccesibleSlider as a way to track down this issue further. Doing this in the sun demo will make it easier to figure out what is going on.

Thanks for the help @zepumph!

[jessegreenberg]

On my Windows 10 desktop I get alerts 99% of the time, playing with the sim for 5-7 minutes I only missed one alert. This continues to indicate that this is a performance problem.

[jessegreenberg]

I added these lines to the HSlider in the sun demo and the problem does not show up there at all.

      accessibleValuePattern: '{{value}} read by valueText',
      endDrag: function() {
        utteranceQueue.addToBack( new Utterance( 'Alert: new value is ' + property.value, {
          typeId: 'testAlert'
        } ) );
      }

[jessegreenberg]

If I remove the formula and the wire and the arrow from the ScreenView, 99% of the alerts come through.

[jessegreenberg]

I determined that the dotsNode was causing the greatest performance impact and removing it entirely from the ScreenView is what made the alerts come through much more consistently and JAWS to feel more responsive when moving the sliders.

I went ahead and re-implemented the dots with CanvasNode and this issue is much improved. There are no longer individual Nodes for each circle so this phet-io code no longer applies:

      var dotsNodeTandem = tandem.createTandem( 'dotsNode' );
      var dotsGroupTandem = dotsNodeTandem.createGroupTandem( 'dots' );

@zepumph is it OK if I just remove this?

[jessegreenberg]

This required a number of changes and two new files. I hope that the merge into the release branch will be straight forward but I don't know how it will go. @zepumph if I can help at all for #168 please let me know.

[zepumph]

This looks to be the only commit needing to be added to the release branch

ffcefb6

@jessegreenberg please confirm.

[jessegreenberg]

@zepumph that is correct. That commit covers #164 and this issue.

[zepumph]

Has this issue been confirmed fixed in rc.8 @KatieWoe? If so please close.

[KatieWoe]

Right. Didn't notice the problem in rc.8. Closing

[zepumph]

Great thanks!