Change how we hide ariaHerald alert content

[jessegreenberg]

It was discovered in phetsims/ohms-law#136 that the way we remove alert content from the DOM interferes with how mobile VO reads alerts.

  timer.setTimeout( () => { liveElement.textContent = ''; }, 200 );

The purpose of this is to make it impossible to read alert content again with the virtual cursor once it is spoken. But it will interrupt mobile VO from completing the alert. We need another way to do something similar.

I tried adding aria-relevant="additions" which should theoretically make it so that only additions to the aria-live region are announced. But it had no effect...

I discovered that replacing liveElement.textContent = ''; with liveElement.hidden = true fixed the problem on mobile VO, but it prevented alerts from working correctly with NVDA. However, setting hidden behind a timeout seems to work OK on NVDA. Maybe this gives the a11y tree time to update, who knows.

      liveElement.hidden = false;

      // after a short delay, set the textContent for the screen reader to anounce, must be done
      // asyncrounously from setting hidden above or else the screen reader will fail to read
      // the content
      timer.setTimeout( () => {
        AccessibilityUtil.setTextContent( liveElement, textContent );

        // after a short delay, hide the content so that it cant be read with the virtual cursor
        // Using `hidden` rather than setting textContent works better than clearing the textContent on mobileVO, see https://github.com/phetsims/scenery-phet/issues/490
        timer.setTimeout( () => {
          liveElement.hidden = true;
        }, 200 );
      }, 200 );

We should try this with

• NVDA
• JAWS
• VO
• Mobile VO

[jessegreenberg]

@KatieWoe could you please test VO on MacOS with this version and make sure alerts are working OK?

https://phet-dev.colorado.edu/html/ohms-law/1.4.0-dev.32/phet/ohms-law_en_phet.html

Maybe just make sure that ~10 alerts come through.

[KatieWoe]

Looks ok from my tests @jessegreenberg

[jessegreenberg]

Cool, thank you @KatieWoe!

[jessegreenberg]

@zepumph and I noticed today that alerts are duplicated, at least in the a11y view and I think it is because of this. My theory is that the MutationObserver of the a11y view is registering the new change as we remove alert content from the DOM.

[zepumph]

@jessegreenberg, please note that in GFLB, when toggling the constant size checkbox, I get between 2 and 3 of the same alert. This is not in the a11y view.

[zepumph]

When I use this patch, JAWS correctly speaks alerts, but iOS VO again gets cut off mid alert.

Index: js/AriaHerald.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/AriaHerald.js b/js/AriaHerald.js
--- a/js/AriaHerald.js	(revision a146f5ba46cf4974ffb98b0e3cb175b416b52d34)
+++ b/js/AriaHerald.js	(date 1613162119557)
@@ -151,7 +151,7 @@
     liveElement.textContent = '';
 
     // element must be visible for alerts to be spoken
-    liveElement.hidden = false;
+    // liveElement.hidden = false;
 
     // must be done asynchronously from setting hidden above or else the screen reader
     // will fail to read the content
@@ -162,10 +162,11 @@
       // behind at least 200 ms delay or else alerts may be missed by NVDA and VoiceOver, see
       // https://github.com/phetsims/scenery-phet/issues/491
       stepTimer.setTimeout( () => {
+        liveElement.textContent = '';
 
         // Using `hidden` rather than clearing textContent works better on mobile VO,
         // see https://github.com/phetsims/scenery-phet/issues/490
-        liveElement.hidden = true;
+        // liveElement.hidden = true;
       }, 200 );
     }, 0 );
   }

We seem to be at some sort of impasse on how to make AriaHerald behave well for all screen readers.

[zepumph]

So it looks like JAWS will say the first 4 alerts, cooresponding to the number of polite live elements we have, but once they have all been hidden, they no longer work with JAWS.

[zepumph]

This completely fixes JAWS, but doesn't work on iOS at all.

Index: js/AriaHerald.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/AriaHerald.js b/js/AriaHerald.js
--- a/js/AriaHerald.js	(revision a146f5ba46cf4974ffb98b0e3cb175b416b52d34)
+++ b/js/AriaHerald.js	(date 1613163496098)
@@ -166,6 +166,14 @@
         // Using `hidden` rather than clearing textContent works better on mobile VO,
         // see https://github.com/phetsims/scenery-phet/issues/490
         liveElement.hidden = true;
+        stepTimer.setTimeout( () => {
+          liveElement.textContent = '';
+
+          // Using `hidden` rather than clearing textContent works better on mobile VO,
+          // see https://github.com/phetsims/scenery-phet/issues/490
+          liveElement.hidden = false;
+
+        }, 200 );
       }, 200 );
     }, 0 );
   }

[zepumph]

This patch didn't fix JAWS at all:

Index: js/AriaHerald.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/AriaHerald.js b/js/AriaHerald.js
--- a/js/AriaHerald.js	(revision a146f5ba46cf4974ffb98b0e3cb175b416b52d34)
+++ b/js/AriaHerald.js	(date 1613163725474)
@@ -146,13 +146,13 @@
    */
   updateLiveElement( liveElement, textContent ) {
 
+    // element must be visible for alerts to be spoken
+    liveElement.hidden = false;
+
     // fully clear the old textContent so that sequential alerts with identical text will be announced, which
     // some screen readers might have prevented
     liveElement.textContent = '';
 
-    // element must be visible for alerts to be spoken
-    liveElement.hidden = false;
-
     // must be done asynchronously from setting hidden above or else the screen reader
     // will fail to read the content
     stepTimer.setTimeout( () => {

[zepumph]

I also found that JAWS alerts were totally fixed by phetsims/ratio-and-proportion#348 (comment) (removing the hiding of elements), but then you can access stale alerts at the end of the DOM (after the PDOM). This got me wondering if it is possible to hide this region in another way. aria-disabled didn't work.

[zepumph]

I would have thought that display wouldn't effect screen reader output, but this was just as broken with JAWS, as setting hidden

Index: js/AriaHerald.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/AriaHerald.js b/js/AriaHerald.js
--- a/js/AriaHerald.js	(revision a146f5ba46cf4974ffb98b0e3cb175b416b52d34)
+++ b/js/AriaHerald.js	(date 1613164512203)
@@ -151,7 +151,7 @@
     liveElement.textContent = '';
 
     // element must be visible for alerts to be spoken
-    liveElement.hidden = false;
+    liveElement.style.display = 'block';
 
     // must be done asynchronously from setting hidden above or else the screen reader
     // will fail to read the content
@@ -165,7 +165,7 @@
 
         // Using `hidden` rather than clearing textContent works better on mobile VO,
         // see https://github.com/phetsims/scenery-phet/issues/490
-        liveElement.hidden = true;
+        liveElement.style.display = 'none';
       }, 200 );
     }, 0 );
   }

[zepumph]

using aria-busy seems to work as a way to have the virtual cursor not be able to access the aria live elements. This was only tested in chrome, but it worked very well! https://www.w3.org/TR/wai-aria-1.2/#aria-busy

Index: js/AriaHerald.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/AriaHerald.js b/js/AriaHerald.js
--- a/js/AriaHerald.js	(revision a146f5ba46cf4974ffb98b0e3cb175b416b52d34)
+++ b/js/AriaHerald.js	(date 1613164729414)
@@ -151,7 +151,7 @@
     liveElement.textContent = '';
 
     // element must be visible for alerts to be spoken
-    liveElement.hidden = false;
+    liveElement.setAttribute( 'aria-busy', false );
 
     // must be done asynchronously from setting hidden above or else the screen reader
     // will fail to read the content
@@ -165,7 +165,7 @@
 
         // Using `hidden` rather than clearing textContent works better on mobile VO,
         // see https://github.com/phetsims/scenery-phet/issues/490
-        liveElement.hidden = true;
+        liveElement.setAttribute( 'aria-busy', true );
       }, 200 );
     }, 0 );
   }

The main issue with the above patch is that we aren't clearing the text content after setting, and so JAWS won't say an alert if it is the same as the alert content that is in the element from the last time. You can reproduce this by pressing the reset all button 5 times (because there are 4 polite elements). On the 5th time, the alert isn't spoken.

[zepumph]

I also want to note that there are some similar-ish open issues in the JAWS issue tracker. Like FreedomScientific/standards-support#296

[jessegreenberg]

I just tried JAWS in Firefox and it isn't happening there. I am also seeing the bug in NVDA on Chrome now (but not Firefox). So I am thinking that it may actually be Chrome bug.

[zepumph]

@jessegreenberg and I were able to confirm that this issue seems to be with all alerting done on Chrome (JAWS and NVDA).

@jessegreenberg and I figured out a platform specific solution that works on everything we have tested thus far. It also notest that setting hidden is more of a workaround for VO anyways, and that it shouldn't be standard.

We will make sure that QA hits this hard in the next RC for Ration and Proportion. We will also do extensive testing on all platforms first.

[jessegreenberg]

If this works well, we probably want to redeploy all other sims with interactive-descriptions in a batch maintenance release. But we wanted to check in with @emily-phet first since this will take development and QA time. I would guess it will take 2-3 hours to go through the batch maintenance release process, and I would think about an hour of QA time to spot check the dozen or so sims that would be redeployed.

@emily-phet can you confirm that this is OK?

[emily-phet]

Yes, please go ahead with the maintenance release.

[terracoda]

Awesome! I am so happy for maintenance releases on these sims, in particular BASE and Friction as they both now have accessible grab and drag on mobile. Not sure if that automatically gets included, but it would be great if it did (no pressure).

Just an FYI, I did notice an issue with BASE last week - the charges were not displaying properly in the wall. See issue phetsims/balloons-and-static-electricity#481

[jessegreenberg]

OK thanks! A maintenance release for an unrelated issue is currently in progress, when that is done I will start this one (probably Wed 2/17).

@terracoda unfortunately this batch maintenance release won't include mobile accessibility or other improvements. Including mobile accessibility would be a much more destabilizing change and isn't something I would be comfortable doing in a batch of maintenance releases. This maintenance release will only apply the verified fix to each sim. But that also means that it is very unlikely for each sim to pick up new bugs in master, such as the one you pointed out in phetsims/balloons-and-static-electricity#481.

[terracoda]

Ok, gotcha, @jessegreenberg! BASE will get a good maintenance release when the sound design is done. I'll have to be patient.

[zepumph]

phetsims/ratio-and-proportion#348 has been tested and closed, so I think our patch worked.

I created https://github.com/phetsims/scenery-phet/issues/671 to do a batch MR to support alerts. I think this issue is ready to close. Please reopen if you feel differently.