Remove formatting tags before speaking with SpeechSynthesis

[jessegreenberg]

Found in phetsims/friction#261 - We need to make sure that formatting tags are removed before speaking something with voicingManager/SpeechSynthesis. Formatting tags are read in full using this technology. They are beneficial for screen readers which is why they have generally been left in place until now.

[jessegreenberg]

@jonathanolson said that RichText uses himalaya for this kind of thing we can start there for a solution.

[jessegreenberg]

@zepumph mentioned that himalaya.parse may have what we need for this!

[zepumph]

It looks like chrome and firefox are both sanitizing the
tags for us, so I think this is only an issue about safari.

[zepumph]

This line of code successfully removes all br tags. It doesn't matter if it is <br> or <br/>.

himalaya.stringify( himalaya.parse( STRING ).filter( something => !( something.type.toLowerCase() === 'element' && something.tagName.toLowerCase() === 'br' ) ) )

[zepumph]

But stringify doesn't exist in our current himalaya version, so I need to update versions.

[zepumph]

Once #1318 is all done, it should be as easy as testing and applying this patch to fix this issue.

Index: js/accessibility/voicing/voicingManager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/accessibility/voicing/voicingManager.js b/js/accessibility/voicing/voicingManager.js
--- a/js/accessibility/voicing/voicingManager.js	(revision 514dd74096f914eb187943272dbfb1bbf7be4e97)
+++ b/js/accessibility/voicing/voicingManager.js	(date 1635552636275)
@@ -418,7 +418,8 @@
 
     // embedding marks (for i18n) impact the output, strip before speaking
     const stringToSpeak = stripEmbeddingMarks( utterance.getTextToAlert( this.respectResponseCollectorProperties ) );
-    const speechSynthUtterance = new SpeechSynthesisUtterance( stringToSpeak );
+    const brTagsRemoved = himalaya.stringify( himalaya.parse( stringToSpeak ).filter( something => !( something.type.toLowerCase() === 'element' && something.tagName.toLowerCase() === 'br' ) ) );
+    const speechSynthUtterance = new SpeechSynthesisUtterance( brTagsRemoved );
     speechSynthUtterance.voice = this.voiceProperty.value;
     speechSynthUtterance.pitch = this.voicePitchProperty.value;
     speechSynthUtterance.rate = this.voiceRateProperty.value;
@@ -427,13 +428,13 @@
     this.safariWorkaroundUtterances.push( speechSynthUtterance );
 
     const startListener = () => {
-      this.startSpeakingEmitter.emit( stringToSpeak, utterance );
+      this.startSpeakingEmitter.emit( brTagsRemoved, utterance );
       this.currentlySpeakingUtterance = utterance;
       speechSynthUtterance.removeEventListener( 'start', startListener );
     };
 
     const endListener = () => {
-      this.endSpeakingEmitter.emit( stringToSpeak, utterance );
+      this.endSpeakingEmitter.emit( brTagsRemoved, utterance );
       speechSynthUtterance.removeEventListener( 'end', endListener );
 
       // remove the reference to the SpeechSynthesisUtterance so we don't leak memory

[zepumph]

Looks like #1318 is shaping up well, and I hope this would be a pretty straightforward change.

[zepumph]

@jessegreenberg can you please review this.

[zepumph]

Just confirming here that #1318 has been reviewed and all is good.

[zepumph]

@jessegreenberg, this review is blocking friction, raising priority.

[jessegreenberg]

Yes - using himalaya for this seems great to me. Verified just now that this is working well, but I know this has been working well since you made this change.

Note that since SpeechSynthesisAnnouncer has been moved to utterance-queue, himalaya has become a dependency of utterance-queue. Back to @zepumph to close and possibly proceed with phetsims/friction#261

[zepumph]

Ahh, you are so right. Is this grace good enough (your grace), or do we need to better support bundling it in when building utterance-queue?

[zepumph]

I'm actually wondering if this would ever happen, because himalaya is a preload in utterance-queue.

https://github.com/phetsims/utterance-queue/blob/ef60adedc96ada8ebf1998114ffaa1de36a33c05/package.json#L23

[zepumph]

Even so, I think I like the commit, just to be safe.

[jessegreenberg]

True, hopefully it stands alone. But I am fine that that guard too. Alright, closing.

[jessegreenberg]

In the following commit we will replace himalaya with split/join which works just as well and lets us easily add spaces where the breaks were.