voicingManager can skip an utterance

[zepumph]

While working on #1287, @jessegreenberg and I found a bug in our assumptions about how utterances work through the "queue" to eventually end up being spoken by voicing. There are actually 3 queues for this feature, and our investigation really helped us understand this.

First we have the UtteranceQueue, once an utterance works its way through that async queue, stepped by animation frames, we announce it to voicingManager. Recently, for a safari workaround, we have a delayed "queue" of sorts that puts off each utterance on a delay of 250ms before sending the utterance to the browser synth (called the taskCallbackQueue). Once at the synth, the browser keeps a queue of what utterances will be spoken next.

The bug we found was that cancel would cancel an utterance while still in the voicingManager's taskCallbackQueue, and so it would be cancelled even though we thought that it had already gone to the browser queue. This is a bit simplified, but it is a good overall explanation.

More details about the bug:
voicingManager.previousUtterance is used to determine if voicingManager.cancel will be called. When cancel is called everything in the taskCallbackQueue is cleared, AND everything in the browser queue. The issue is that both of those queues are populated and maintained after an async step AFTER voicingManager.previousUtterance is set to the NEW utterance (sync relative to calling announce).

One actionable thing to do
One potential solution is to make sure that previousUtterance is set later on, sync to when the browser starts speaking it (in the startListener from the start event for a SpeechSynthesisUtterance).

This doesn't quite fix everything though. Let's look at one more case:

I add these three items to the queue (see #1287 for understanding about utterance priority):

1. Utterance with a high Priority
2. Utterance with a low Priority
3. Utterance with a middle Priority

(1) get's all the way to the browser, and is being spoken right now.
(2) get's all the way to the browser, and is in the queue there.
(3) is supposed to have a higher priority than (2), but shouldn't cancel (1).

We don't have this level of granularity once we pass things to the browser queue. synth.clear() stops the current speaking, AND clears the browser queue. Thus we need to implement our own queue.

Next piece of action to implement
We need to create our own queue, and instead of ever adding something to the browser queue, listen to the end event for the SpeechSynthesisUtterance, and then add the next item from there. @jessegreenberg wanted me to note that there already exists a list of SpeechSynthesisUtteraces on voicingManager. This is another safari workaround in which the end event is more often called if utterances are kept in memory.

Last piece of work here
Given the above example, it isn't enough to just control when we send items to the browser queue, we also want to utilize that in house queue to be able to adapt voicingManager.cancel to have support for removing any item ahead of it that is lower priority (or cancelOther:true if same priority).

I will work on a prototype, now but likely won't get comfortable enough to commit.

[zepumph]

OK. I didn't get far at all on this, but I did clean up the debugging code from the original bug investigation, and I added many TODOs with next steps. Basically from here I feel like we can fill in the TODOs with the three bolded tasks from the original 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 d73d6fc643306e149c3eede8706e6d107c978397)
+++ b/js/accessibility/voicing/voicingManager.js	(date 1632525466726)
@@ -118,11 +118,13 @@
     // Unfortunately, this also introduces a memory leak, we should be smarter about
     // clearing this, though it is a bit tricky since we don't have a way to know
     // when we are done with an utterance - see #215
-    this.utterances = [];
+    // TODO: JG said to keep these separate so that we don't accidentally lose track of this reference, https://github.com/phetsims/scenery/issues/1288
+    this.safariWorkaroundUtterances = [];
 
     // @private {TimeoutCallbackObject[]} - Array of objects with functions that are added to the stepTimer to request
     // speech with SpeechSynthesis.
     this.timeoutCallbackObjects = [];
+    // TODO: do we add another queue list, or should be just use the timeoutCallbackObjects as the wrappers? https://github.com/phetsims/scenery/issues/1288
   }
 
   /**
@@ -291,12 +293,17 @@
   requestSpeech( utterance, withCancel ) {
     assert && assert( this.isSpeechSynthesisSupported(), 'trying to speak with speechSynthesis, but it is not supported on this platform' );
 
+    // TODO: likely this will need to go, but it is nice to think about the potential for these to be aligned. Perhaps there is another place this could go after the async part of timeoutCallbackObjects, https://github.com/phetsims/scenery/issues/1288
+    // assert && assert( this.speakingProperty.value === this.getSynth().speaking, 'isSpeaking discrepancy' );
+
     // only cancel the previous alert if there is something new to speak
     if ( withCancel && utterance.alert ) {
+
+      // TODO: replace this with a "cleanupQueue" call that will not just clear everything, but only those that should be cancelled! https://github.com/phetsims/scenery/issues/1288
       this.cancel();
     }
 
-    // embeddding marks (for i18n) impact the output, strip before speaking
+    // embedding marks (for i18n) impact the output, strip before speaking
     const stringToSpeak = stripEmbeddingMarks( utterance.getTextToAlert( this.respectResponseCollectorProperties ) );
     const speechSynthUtterance = new SpeechSynthesisUtterance( stringToSpeak );
     speechSynthUtterance.voice = this.voiceProperty.value;
@@ -304,12 +311,16 @@
     speechSynthUtterance.rate = this.voiceRateProperty.value;
 
     // keep a reference to WebSpeechUtterances in Safari, so the browser doesn't dispose of it before firing, see #215
-    this.utterances.push( speechSynthUtterance );
+    this.safariWorkaroundUtterances.push( speechSynthUtterance );
 
-    // Keep this out of the start listener so that it can be synchrounous to the UtteranceQueue draining/announcing, see bug in https://github.com/phetsims/sun/issues/699#issuecomment-831529485
+    // TODO: don't we think that this should be moved into start listener? https://github.com/phetsims/scenery/issues/1288
     this.previousUtterance = utterance;
 
     const startListener = () => {
+
+      if ( stringToSpeak.includes( 'Atoms break away from chemist' ) || stringToSpeak.includes( 'y Hot. More atoms break ' ) ) {
+        console.log( 'startoh hello from break away', this.getSynth().speaking, this.speakingProperty.value );
+      }
       this.startSpeakingEmitter.emit( stringToSpeak, utterance );
       this.speakingProperty.set( true );
       speechSynthUtterance.removeEventListener( 'start', startListener );
@@ -317,15 +328,21 @@
 
     const endListener = () => {
 
+      if ( stringToSpeak.includes( 'Atoms break away from chemist' ) || stringToSpeak.includes( 'y Hot. More atoms break ' ) ) {
+        console.log( 'endoh hello from break away', this.getSynth().speaking, this.speakingProperty.value );
+      }
       this.endSpeakingEmitter.emit( stringToSpeak, utterance );
       this.speakingProperty.set( false );
       speechSynthUtterance.removeEventListener( 'end', endListener );
 
       // remove the reference to the SpeechSynthesisUtterance so we don't leak memory
-      const indexOfUtterance = this.utterances.indexOf( speechSynthUtterance );
+      const indexOfUtterance = this.safariWorkaroundUtterances.indexOf( speechSynthUtterance );
       if ( indexOfUtterance > -1 ) {
-        this.utterances.splice( indexOfUtterance, 1 );
+        this.safariWorkaroundUtterances.splice( indexOfUtterance, 1 );
       }
+
+      // TODO: is it correct to add this to the endListener? https://github.com/phetsims/scenery/issues/1288
+      this.previousUtterance = null;
     };
 
     speechSynthUtterance.addEventListener( 'start', startListener );
@@ -400,7 +417,64 @@
 
       // cancel clears all utterances from the internal SpeechSynthsis queue so we should
       // clear all of our references as well
-      this.utterances = [];
+      this.safariWorkaroundUtterances = [];
+    }
+  }
+
+  // @private
+  cancelThisOneQuestionMark( myUtternace, potentialToCancel ) {
+
+    // TODO: Move this logic here from speak?!?!?? Can cancelSelf and cancelOther be a bit more solidified as announcerOptions on Utterance so that we can use just two Utterances as parameters https://github.com/phetsims/scenery/issues/1288
+   /*
+    options = merge( {
+
+      // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
+      // to the queue again will immediately cancel the synth and new content will be
+      // spoken. Otherwise, new content for this utterance will be spoken whenever the old
+      // content has finished speaking
+      cancelSelf: true,
+
+      // {boolean} - If true and another Utterance is currently being spoken by the speech synth,
+      // announcing this Utterance will immediately cancel the other content being spoken by the synth.
+      // Otherwise, content for the new utterance will be spoken as soon as the browser finishes speaking
+      // the old content
+      cancelOther: true
+    }, options );
+
+    let withCancel = options.cancelOther;
+    if ( this.previousUtterance && this.previousUtterance === utterance ) {
+      withCancel = options.cancelSelf;
+    }
+     */
+  }
+
+  // TODO: copied from cancel, but needs to be adapted to use cancelThisOneQuestionMark to determine if timeoutCallbackObjects should be removed https://github.com/phetsims/scenery/issues/1288
+  // @private
+  cleanUpAndPotentiallyCancelOthers() {
+
+    if ( this.initialized ) {
+
+      // test against previousUtterance
+      this.getSynth().cancel();
+
+      // iterate over a copy of the timeoutCallbackObjects because we will remove elements as we go through
+      this.timeoutCallbackObjects.slice().forEach( ( callbackObject, index ) => {
+
+        // TODO: test again this callback utterance to see if it specifically should be cleared. https://github.com/phetsims/scenery/issues/1288
+        // callbackObject
+
+        // Do not clear the timeout if we are cancelling as a side effect from the timeout listener being called,
+        // in that case stepTimer clear the timeout and the TimeoutCallbackObject is removed from within
+        // the listener.
+        if ( !callbackObject.timerCallingListener ) {
+          stepTimer.clearTimeout( callbackObject.stepTimerListener );
+          this.timeoutCallbackObjects.splice( index, 1 );
+        }
+      } );
+
+      // cancel clears all utterances from the internal SpeechSynthsis queue so we should
+      // clear all of our references as well
+      this.safariWorkaroundUtterances = [];
     }
   }
 }
@@ -426,6 +500,7 @@
     // of this object is being called and should not be removed from stepTimer's listeners
     // because the stepTimer will automatically try to remove it after calling the callback.
     this.timerCallingListener = false;
+    this.x = speechSynthUtterance;
 
     // In Safari, the `start` and `end` listener does not fire consistently, especially after interruption with
     // cancel. But speaking behind a timeout improves the behavior significantly. A reference to the listener

[zepumph]

Here are some bad notes I took with JG about some of the hard cases here.

add utterance to queue
async churn queue

announce
requestSpeech (withCancel) <---- cancel every timeoutCallback
previosuUtterance
timeoutCallback()

browser.synth.go baby








previosuUtterance = atoms break
timeoutCallback()
request (rub fast)
prebiousUtter( rub fast)
timeoutCallback
request ( other) cancel? compare to rub fast or slow YES
cancel atoms break away


async

never get tot  atom break away.





loooooooooooongAlert
fast
fast
fsat
fast


speak( fast1) <- loopAlert previous
timeoutCallback
speak( fast2 ) <- loopAlert previous
timeoutcallback




loooooooooooongAlert 5
fast 5 cancelOTher false
fast 6
fsat
fast


speak( fast1) <- loopAlert previous
timeoutCallback
speak( fast2 ) <- fast1 previous
no i interrupt fast1




loooooooooooongAlert 6
fast 4 cancelOTher false  timeoutCallback
fast 5 timeoutCallback
fsat
fast8


NOT SOLVED
loooooooooooongAlert 6
fast 4 cancelOTher false  in browser queue (not cleared)
fast 5 timeoutCallback
fsat
fast8

high
low
<- + 250 ms so that low is in the browser queue
add medium

[zepumph]

Starting on prototyping this now.

[zepumph]

Probably I should commit to a branch, but since it is just a single file, I'm going to put my progress in here. Basically I have speech working running off of the "end" listener. What I don't have is cancelling working, really at all. I hope to be able to bring @jessegreenberg on this, and do a sort of co-review + bug fixing session on Friday, because that will help me in understanding the file even more, and potentially where things are going wrong. I am worried a bit about this patch because there were three large chunks of change to do, and there was really no way to separate it, so it really is quite the batch patch.

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 32bcc114feb8a747bea705fdb15cd200d59515da)
+++ b/js/accessibility/voicing/voicingManager.js	(date 1632871971386)
@@ -25,6 +25,22 @@
 import globalKeyStateTracker from '../globalKeyStateTracker.js';
 import KeyboardUtils from '../KeyboardUtils.js';
 
+const UTTERANCE_OPTION_DEFAULTS = {
+
+  // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
+  // to the queue again will immediately cancel the synth and new content will be
+  // spoken. Otherwise, new content for this utterance will be spoken whenever the old
+  // content has finished speaking
+  cancelSelf: true,
+
+  // {boolean} - If true and another Utterance is currently being spoken by the speech synth,
+  // announcing this Utterance will immediately cancel the other content being spoken by the synth.
+  // Otherwise, content for the new utterance will be spoken as soon as the browser finishes speaking
+  // the old content
+  cancelOther: true
+};
+
+
 class VoicingManager extends Announcer {
   constructor() {
     super( {
@@ -42,6 +58,7 @@
     // {NumberProperty} - controls the pitch of the synth
     this.voicePitchProperty = new NumberProperty( 1.0, { range: new Range( 0.5, 2 ) } );
 
+    // TODO: update doc, doesn't quite use a timeout anymore, https://github.com/phetsims/scenery/issues/1288
     // @private {boolean} - Indicates whether or not speech using SpeechSynthesis has been requested at least once.
     // The first time speech is requested it must be done synchronously from user input with absolutely no delay.
     // requestSpeech() uses a timeout to workaround browser bugs, but those cannot be used until after the first
@@ -106,9 +123,10 @@
     // initialized.
     this.boundHandleCanSpeakChange = this.handleCanSpeakChange.bind( this );
 
-    // @private {Utterance} - A reference to the last utterance spoken, so we can determine
-    // cancelling behavior when it is time to speak the next utterance. See VoicingUtterance options.
-    this.previousUtterance = null;
+    // @private {Utterance|null} - A reference to the utterance currently in the synth being spoken by the browser, so
+    // we can determine cancelling behavior when it is time to speak the next utterance. See voicing's supported
+    // announcerOptions for details.
+    this.currentlySpeakingUtterance = null;
 
     // fixes a bug on Safari where the `start` and `end` Utterances don't fire! The
     // issue is (apparently) that Safari internally clears the reference to the
@@ -118,11 +136,15 @@
     // Unfortunately, this also introduces a memory leak, we should be smarter about
     // clearing this, though it is a bit tricky since we don't have a way to know
     // when we are done with an utterance - see #215
-    this.utterances = [];
+    // Blown away regularly, don't keep a reference to it.
+    this.safariWorkaroundUtterances = [];
 
-    // @private {TimeoutCallbackObject[]} - Array of objects with functions that are added to the stepTimer to request
-    // speech with SpeechSynthesis.
-    this.timeoutCallbackObjects = [];
+    // TODO:
+    // Blown away regularly, don't keep a reference to it.
+    this.voicingQueue = [];
+
+    // TODO: do we need to dispose like UtteranceQueue supports? https://github.com/phetsims/scenery/issues/1288
+    stepTimer.addListener( this.stepQueue.bind( this ) );
   }
 
   /**
@@ -176,6 +198,64 @@
     this.initialized = true;
   }
 
+  /**
+   * Remove an element from the voicingQueue
+   * @private
+   * @param {VoicingQueueElement} voicingQueueElement
+   */
+  removeFromVoicingQueue( voicingQueueElement ) {
+
+    // remove from voicingManager list after speaking
+    const index = voicingManager.voicingQueue.indexOf( voicingQueueElement );
+    assert && assert( index >= 0, 'trying to remove a voicingQueueElement that doesn\'t exist' );
+    voicingManager.voicingQueue.splice( index, 1 );
+  }
+
+
+  /**
+   * IF there is an element in the queue that has been in long enough to support the safari workaround, then alert the first
+   * one.
+   * @private
+   */
+  alertNow() {
+    const synth = voicingManager.getSynth();
+    if ( synth ) {
+      for ( let i = 0; i < this.voicingQueue.length; i++ ) {
+        const voicingQueueElement = this.voicingQueue[ i ];
+        if ( voicingQueueElement.timeInQueue >= voicingQueueElement.minTimeInQueue ) {
+
+          synth.speak( voicingQueueElement.speechSynthUtterance );
+
+          // remove from voicingManager list after speaking
+          this.removeFromVoicingQueue( voicingQueueElement );
+          break;
+        }
+      }
+
+    }
+  }
+
+  /**
+   * @private
+   */
+  onSpeechSynthesisUtteranceEnd() {
+    console.log( 'previous ended, trying the next one' );
+    this.alertNow();
+  }
+
+  /**
+   * @private
+   * @param {number} dt
+   */
+  stepQueue( dt ) {
+
+    // increase the time each element has spent in queue
+    for ( let i = 0; i < this.voicingQueue.length; i++ ) {
+      const voicingQueueElement = this.voicingQueue[ i ];
+      voicingQueueElement.timeInQueue += dt;
+    }
+  }
+
   /**
    * When we can no longer speak, cancel all speech to silence everything.
    * @private
@@ -230,28 +310,7 @@
    */
   announce( utterance, options ) {
     if ( this.initialized ) {
-
-      options = merge( {
-
-        // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
-        // to the queue again will immediately cancel the synth and new content will be
-        // spoken. Otherwise, new content for this utterance will be spoken whenever the old
-        // content has finished speaking
-        cancelSelf: true,
-
-        // {boolean} - If true and another Utterance is currently being spoken by the speech synth,
-        // announcing this Utterance will immediately cancel the other content being spoken by the synth.
-        // Otherwise, content for the new utterance will be spoken as soon as the browser finishes speaking
-        // the old content
-        cancelOther: true
-      }, options );
-
-      let withCancel = options.cancelOther;
-      if ( this.previousUtterance && this.previousUtterance === utterance ) {
-        withCancel = options.cancelSelf;
-      }
-
-      this.speak( utterance, withCancel );
+      this.speak( utterance );
     }
   }
 
@@ -261,13 +320,10 @@
    * @public
    *
    * @param {Utterance} utterance
-   * @param {boolean} withCancel - if true, any utterances remaining in the queue will be removed and this utterance
-   *                               will take priority. Hopefully this works on all platforms, if it does not we
-   *                               need to implement our own queing system.
    */
-  speak( utterance, withCancel = true ) {
+  speak( utterance ) {
     if ( this.initialized && this._canSpeakProperty.value ) {
-      this.requestSpeech( utterance, withCancel );
+      this.requestSpeech( utterance );
     }
   }
 
@@ -280,11 +336,10 @@
    * @public
    *
    * @param {Utterance} utterance
-   * @param {boolean} [withCancel] - if true, any utterances before this one will be cancelled and never spoken
    */
-  speakIgnoringEnabled( utterance, withCancel = true ) {
+  speakIgnoringEnabled( utterance ) {
     if ( this.initialized ) {
-      this.requestSpeech( utterance, withCancel );
+      this.requestSpeech( utterance );
     }
   }
 
@@ -293,17 +348,22 @@
    * @private
    *
    * @param {Utterance} utterance
-   * @param {boolean} [withCancel]
    */
-  requestSpeech( utterance, withCancel ) {
+  requestSpeech( utterance ) {
     assert && assert( this.isSpeechSynthesisSupported(), 'trying to speak with speechSynthesis, but it is not supported on this platform' );
 
+    // TODO: likely this will need to go, but it is nice to think about the potential for these to be aligned. Perhaps there is another place this could go after the async part of queue stepping, https://github.com/phetsims/scenery/issues/1288
+    // assert && assert( this.speakingProperty.value === this.getSynth().speaking, 'isSpeaking discrepancy' );
+
     // only cancel the previous alert if there is something new to speak
-    if ( withCancel && utterance.alert ) {
-      this.cancel();
+    // TODO: any other checks on the alert, like omitting an empty alert?, https://github.com/phetsims/scenery/issues/1288
+    if ( utterance.alert ) {
+
+      // TODO: eventually, add in support for priority.
+      this.cleanUpAndPotentiallyCancelOthers( utterance );
     }
 
-    // embeddding marks (for i18n) impact the output, strip before speaking
+    // embedding marks (for i18n) impact the output, strip before speaking
     const stringToSpeak = stripEmbeddingMarks( utterance.getTextToAlert( this.respectResponseCollectorProperties ) );
     const speechSynthUtterance = new SpeechSynthesisUtterance( stringToSpeak );
     speechSynthUtterance.voice = this.voiceProperty.value;
@@ -311,28 +371,32 @@
     speechSynthUtterance.rate = this.voiceRateProperty.value;
 
     // keep a reference to WebSpeechUtterances in Safari, so the browser doesn't dispose of it before firing, see #215
-    this.utterances.push( speechSynthUtterance );
-
-    // Keep this out of the start listener so that it can be synchrounous to the UtteranceQueue draining/announcing, see bug in https://github.com/phetsims/sun/issues/699#issuecomment-831529485
-    this.previousUtterance = utterance;
+    this.safariWorkaroundUtterances.push( speechSynthUtterance );
 
     const startListener = () => {
+      console.log( 'start' );
       this.startSpeakingEmitter.emit( stringToSpeak, utterance );
+      this.currentlySpeakingUtterance = utterance;
       this.speakingProperty.set( true );
       speechSynthUtterance.removeEventListener( 'start', startListener );
     };
 
     const endListener = () => {
-
+      console.log( 'end' );
       this.endSpeakingEmitter.emit( stringToSpeak, utterance );
       this.speakingProperty.set( false );
       speechSynthUtterance.removeEventListener( 'end', endListener );
 
       // remove the reference to the SpeechSynthesisUtterance so we don't leak memory
-      const indexOfUtterance = this.utterances.indexOf( speechSynthUtterance );
+      const indexOfUtterance = this.safariWorkaroundUtterances.indexOf( speechSynthUtterance );
       if ( indexOfUtterance > -1 ) {
-        this.utterances.splice( indexOfUtterance, 1 );
+        this.safariWorkaroundUtterances.splice( indexOfUtterance, 1 );
       }
+
+      this.currentlySpeakingUtterance = null;
+
+      // kick off the next element now that this one is done.
+      this.onSpeechSynthesisUtteranceEnd();
     };
 
     speechSynthUtterance.addEventListener( 'start', startListener );
@@ -342,20 +406,18 @@
     // but the error event does. In this case signify that speaking has ended.
     speechSynthUtterance.addEventListener( 'error', endListener );
 
+    const options = this.hasSpoken ? null : { minTimeInQueue: 0 };
+    const voicingQueueElement = new VoicingQueueElement( utterance, speechSynthUtterance, options );
+    this.voicingQueue.push( voicingQueueElement );
+
     if ( !this.hasSpoken ) {
-
-      // for the first time speaking it must be synchronous and we cannot use TimeoutCallbackObject workarounds yet
-      this.getSynth().speak( speechSynthUtterance );
-      this.hasSpoken = true;
-
+      this.alertNow(); // TODO: I sure do hope this would work, but it is a bit of an obfuscation that it is actually happening "NOW".  https://github.com/phetsims/scenery/issues/1288
     }
-    else {
-
-      // Create and add the callback object which will request speech from SpeechSynthesis behind a small delay
-      // (as a workaround for Safari), and also track when the timeout callback is being fired so that listeners
-      // can be safely removed. See TimeoutCallbackObject for more information.
-      this.timeoutCallbackObjects.push( new TimeoutCallbackObject( speechSynthUtterance ) );
-    }
+    // Create and add the callback object which will request speech from SpeechSynthesis behind a small delay
+    // (as a workaround for Safari), and also track when the timeout callback is being fired so that listeners
+    // can be safely removed. See TimeoutCallbackObject for more information.
+    // TODO: keep this doc somewhere, https://github.com/phetsims/scenery/issues/1288
+
   }
 
   /**
@@ -391,28 +453,77 @@
    */
   cancel() {
     if ( this.initialized ) {
+
+      // Cancel anything that is being spoken currently.
       this.getSynth().cancel();
 
-      // iterate over a copy of the timeoutCallbackObjects because we will remove elements as we go through
-      this.timeoutCallbackObjects.slice().forEach( ( callbackObject, index ) => {
-
-        // Do not clear the timeout if we are cancelling as a side effect from the timeout listener being called,
-        // in that case stepTimer clear the timeout and the TimeoutCallbackObject is removed from within
-        // the listener.
-        if ( !callbackObject.timerCallingListener ) {
-          stepTimer.clearTimeout( callbackObject.stepTimerListener );
-          this.timeoutCallbackObjects.splice( index, 1 );
-        }
-      } );
+      // clear everything queued to be voiced.
+      this.voicingQueue = [];
 
       // cancel clears all utterances from the internal SpeechSynthsis queue so we should
       // clear all of our references as well
-      this.utterances = [];
+      this.safariWorkaroundUtterances = [];
+    }
+  }
+
+  /**
+   * Given one utterance, should it cancel another provided utterance?
+   * @param {Utterance} myUtterance
+   * @param {Utterance} potentialToCancelUtterance
+   * @returns {boolean}
+   * @private
+   */
+  cancelThisOneQuestionMark( myUtterance, potentialToCancelUtterance ) {
+    assert && assert( myUtterance instanceof Utterance );
+    assert && assert( potentialToCancelUtterance instanceof Utterance );
+
+    // TODO: Move this logic here from speak?!?!?? Can cancelSelf and cancelOther be a bit more solidified as announcerOptions on Utterance so that we can use just two Utterances as parameters https://github.com/phetsims/scenery/issues/1288
+    // TODO: this is waaaay too much garbage, perhaps when an utterance is sent to us, we can fill in defaults once, https://github.com/phetsims/scenery/issues/1288
+    const myUtteranceOptions = merge( UTTERANCE_OPTION_DEFAULTS, myUtterance.announcerOptions );
+    // const potentialToCancelUtteranceOptions = merge( UTTERANCE_OPTION_DEFAULTS, potentialToCancelUtterance.announcerOptions );
+
+    let shouldCancel = myUtteranceOptions.cancelOther;
+    if ( potentialToCancelUtterance && potentialToCancelUtterance === myUtterance ) {
+      shouldCancel = myUtteranceOptions.cancelSelf;
+    }
+    return shouldCancel;
+  }
+
+  // @private
+  cleanUpAndPotentiallyCancelOthers( utteranceThatMayCancelOthers ) {
+
+    if ( this.initialized ) {
+
+
+      for ( let i = this.voicingQueue.length - 1; i >= 0; i-- ) {
+        const voicingQueueElement = this.voicingQueue[ i ];
+
+        if ( this.cancelThisOneQuestionMark( utteranceThatMayCancelOthers, voicingQueueElement.utterance ) ) {
+
+
+          this.removeFromVoicingQueue( voicingQueueElement );
+
+          // remove from safari workaround list to avoid memory leaks, if available
+          const index = this.safariWorkaroundUtterances.indexOf( voicingQueueElement.speechSynthUtterance );
+          this.safariWorkaroundUtterances.splice( index, 1 );
+        }
+      }
+
+      if ( this.currentlySpeakingUtterance && this.cancelThisOneQuestionMark( utteranceThatMayCancelOthers, this.currentlySpeakingUtterance ) ) {
+
+        console.log( this.currentlySpeakingUtterance.toString(), '\nis currently speaking, cancelling it for', utteranceThatMayCancelOthers.toString(), '\n\n' );
+
+        // test against what is currently being spoken by the synth (currentlySpeakingUtterance)
+        // TODO: does this call the `error` callback, or the `end` callback on a speechSynthUtterance? that would be important to know, https://github.com/phetsims/scenery/issues/1288
+        this.getSynth().cancel();
+        console.log( 'after cancel' );
+      }
     }
   }
 }
 
 /**
+ * TODO: Update documentation, https://github.com/phetsims/scenery/issues/1288
  * An inner class that is responsible for adding a listener to the stepTimer that will request
  * speech, but is also aware of when the listener is being called by the stepTimer. When voicingManager
  * is cancelled, we need to clear all timeout callbacks that will request speech from the stepTimer.
@@ -422,38 +533,33 @@
  *
  * See documentation in constructor for why a timeout is required in the first place.
  */
-class TimeoutCallbackObject {
+class VoicingQueueElement {
 
   /**
+   * @param {Utterance} utterance
    * @param {SpeechSynthesisUtterance} speechSynthUtterance
+   * @param {Object} [options]
    */
-  constructor( speechSynthUtterance ) {
+  constructor( utterance, speechSynthUtterance, options ) {
 
-    // @public (read-only) {boolean} - A field that indicates the timeout listener
-    // of this object is being called and should not be removed from stepTimer's listeners
-    // because the stepTimer will automatically try to remove it after calling the callback.
-    this.timerCallingListener = false;
+    options = merge( {
 
-    // In Safari, the `start` and `end` listener does not fire consistently, especially after interruption with
-    // cancel. But speaking behind a timeout improves the behavior significantly. A reference to the listener
-    // is saved so that it can be removed if we cancel speech. timeout of 250 ms was determined with testing
-    // to be a good value to use. Values less than 250 broke the workaround, while larger values feel too
-    // sluggish. See https://github.com/phetsims/john-travoltage/issues/435
-    this.stepTimerListener = stepTimer.setTimeout( () => {
-      const synth = voicingManager.getSynth();
-      if ( synth ) {
-        this.timerCallingListener = true;
-
-        synth.speak( speechSynthUtterance );
+      // TODO: update doc further, https://github.com/phetsims/scenery/issues/1288
+      // In Safari, the `start` and `end` listener does not fire consistently, especially after interruption with
+      // cancel. But speaking behind a timeout improves the behavior significantly. A reference to the listener
+      // is saved so that it can be removed if we cancel speech. timeout of 250 ms was determined with testing
+      // to be a good value to use. Values less than 250 broke the workaround, while larger values feel too
+      // sluggish. See https://github.com/phetsims/john-travoltage/issues/435
+      minTimeInQueue: 250
+    }, options );
 
-        // remove from voicingManager list after speaking
-        const index = voicingManager.timeoutCallbackObjects.indexOf( this );
-        assert && assert( index >= 0, 'trying to remove a callback that doesn\'t exist' );
-        voicingManager.timeoutCallbackObjects.splice( index, 1 );
-      }
-    }, 250 );
+    this.utterance = utterance;
+    this.speechSynthUtterance = speechSynthUtterance;
+    this.timeInQueue = 0;
+    this.minTimeInQueue = options.minTimeInQueue;
   }
 }
+
 
 const voicingManager = new VoicingManager();
 

[jessegreenberg]

@zepumph and I worked on this together for a while this evening and made some good progress, here is the change set, incorporating fixes into the patches from above:

Index: js/accessibility/voicing/voicingManager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/accessibility/voicing/voicingManager.js	(revision e2619a51919fb6921bdfa4a67dcada557f36baee)
+++ js/accessibility/voicing/voicingManager.js	(date 1632955825095)
@@ -25,6 +25,22 @@
 import globalKeyStateTracker from '../globalKeyStateTracker.js';
 import KeyboardUtils from '../KeyboardUtils.js';
 
+const UTTERANCE_OPTION_DEFAULTS = {
+
+  // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
+  // to the queue again will immediately cancel the synth and new content will be
+  // spoken. Otherwise, new content for this utterance will be spoken whenever the old
+  // content has finished speaking
+  cancelSelf: true,
+
+  // {boolean} - If true and another Utterance is currently being spoken by the speech synth,
+  // announcing this Utterance will immediately cancel the other content being spoken by the synth.
+  // Otherwise, content for the new utterance will be spoken as soon as the browser finishes speaking
+  // the old content
+  cancelOther: true
+};
+
+
 class VoicingManager extends Announcer {
   constructor() {
     super( {
@@ -42,6 +58,7 @@
     // {NumberProperty} - controls the pitch of the synth
     this.voicePitchProperty = new NumberProperty( 1.0, { range: new Range( 0.5, 2 ) } );
 
+    // TODO: update doc, doesn't quite use a timeout anymore, https://github.com/phetsims/scenery/issues/1288
     // @private {boolean} - Indicates whether or not speech using SpeechSynthesis has been requested at least once.
     // The first time speech is requested it must be done synchronously from user input with absolutely no delay.
     // requestSpeech() uses a timeout to workaround browser bugs, but those cannot be used until after the first
@@ -106,9 +123,10 @@
     // initialized.
     this.boundHandleCanSpeakChange = this.handleCanSpeakChange.bind( this );
 
-    // @private {Utterance} - A reference to the last utterance spoken, so we can determine
-    // cancelling behavior when it is time to speak the next utterance. See VoicingUtterance options.
-    this.previousUtterance = null;
+    // @private {Utterance|null} - A reference to the utterance currently in the synth being spoken by the browser, so
+    // we can determine cancelling behavior when it is time to speak the next utterance. See voicing's supported
+    // announcerOptions for details.
+    this.currentlySpeakingUtterance = null;
 
     // fixes a bug on Safari where the `start` and `end` Utterances don't fire! The
     // issue is (apparently) that Safari internally clears the reference to the
@@ -118,11 +136,14 @@
     // Unfortunately, this also introduces a memory leak, we should be smarter about
     // clearing this, though it is a bit tricky since we don't have a way to know
     // when we are done with an utterance - see #215
-    this.utterances = [];
+    // Blown away regularly, don't keep a reference to it.
+    this.safariWorkaroundUtterances = [];
 
-    // @private {TimeoutCallbackObject[]} - Array of objects with functions that are added to the stepTimer to request
-    // speech with SpeechSynthesis.
-    this.timeoutCallbackObjects = [];
+    // Blown away regularly, don't keep a reference to it.
+    this.voicingQueue = [];
+
+    // TODO: do we need to dispose like UtteranceQueue supports? https://github.com/phetsims/scenery/issues/1288
+    stepTimer.addListener( this.stepQueue.bind( this ) );
   }
 
   /**
@@ -176,6 +197,71 @@
     this.initialized = true;
   }
 
+  /**
+   * Remove an element from the voicingQueue
+   * @private
+   * @param {VoicingQueueElement} voicingQueueElement
+   */
+  removeFromVoicingQueue( voicingQueueElement ) {
+
+    // remove from voicingManager list after speaking
+    const index = voicingManager.voicingQueue.indexOf( voicingQueueElement );
+    assert && assert( index >= 0, 'trying to remove a voicingQueueElement that doesn\'t exist' );
+    voicingManager.voicingQueue.splice( index, 1 );
+  }
+
+
+  /**
+   * IF there is an element in the queue that has been in long enough to support the safari workaround, then alert the first
+   * one.
+   * @private
+   */
+  alertNow() {
+    const synth = voicingManager.getSynth();
+    if ( synth ) {
+      for ( let i = 0; i < this.voicingQueue.length; i++ ) {
+        const voicingQueueElement = this.voicingQueue[ i ];
+
+        // if minTimeInQueue is zero, it should be alerted synchronously by calling alertNow
+        if ( voicingQueueElement.timeInQueue >= voicingQueueElement.minTimeInQueue ) {
+
+          synth.speak( voicingQueueElement.speechSynthUtterance );
+
+          // remove from voicingManager list after speaking
+          this.removeFromVoicingQueue( voicingQueueElement );
+          break;
+        }
+      }
+
+    }
+  }
+
+  /**
+   * @private
+   */
+  onSpeechSynthesisUtteranceEnd() {
+    // console.log( 'previous ended, trying the next one' );
+    this.alertNow();
+  }
+
+  /**
+   * @private
+   * @param {number} dt
+   */
+  stepQueue( dt ) {
+
+    // increase the time each element has spent in queue
+    for ( let i = 0; i < this.voicingQueue.length; i++ ) {
+      const voicingQueueElement = this.voicingQueue[ i ];
+      voicingQueueElement.timeInQueue += dt * 1000;
+    }
+
+    // TODO: If we are nervous about the `end` events coming thorugh we could do something like speak the next queue element if the synth is not speaking (if there is still something in our queue).
+    if ( !this.getSynth().speaking && this.voicingQueue.length > 0 ) {
+      this.alertNow();
+    }
+  }
+
   /**
    * When we can no longer speak, cancel all speech to silence everything.
    * @private
@@ -230,28 +316,7 @@
    */
   announce( utterance, options ) {
     if ( this.initialized ) {
-
-      options = merge( {
-
-        // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
-        // to the queue again will immediately cancel the synth and new content will be
-        // spoken. Otherwise, new content for this utterance will be spoken whenever the old
-        // content has finished speaking
-        cancelSelf: true,
-
-        // {boolean} - If true and another Utterance is currently being spoken by the speech synth,
-        // announcing this Utterance will immediately cancel the other content being spoken by the synth.
-        // Otherwise, content for the new utterance will be spoken as soon as the browser finishes speaking
-        // the old content
-        cancelOther: true
-      }, options );
-
-      let withCancel = options.cancelOther;
-      if ( this.previousUtterance && this.previousUtterance === utterance ) {
-        withCancel = options.cancelSelf;
-      }
-
-      this.speak( utterance, withCancel );
+      this.speak( utterance );
     }
   }
 
@@ -261,13 +326,10 @@
    * @public
    *
    * @param {Utterance} utterance
-   * @param {boolean} withCancel - if true, any utterances remaining in the queue will be removed and this utterance
-   *                               will take priority. Hopefully this works on all platforms, if it does not we
-   *                               need to implement our own queing system.
    */
-  speak( utterance, withCancel = true ) {
+  speak( utterance ) {
     if ( this.initialized && this._canSpeakProperty.value ) {
-      this.requestSpeech( utterance, withCancel );
+      this.requestSpeech( utterance );
     }
   }
 
@@ -280,11 +342,10 @@
    * @public
    *
    * @param {Utterance} utterance
-   * @param {boolean} [withCancel] - if true, any utterances before this one will be cancelled and never spoken
    */
-  speakIgnoringEnabled( utterance, withCancel = true ) {
+  speakIgnoringEnabled( utterance ) {
     if ( this.initialized ) {
-      this.requestSpeech( utterance, withCancel );
+      this.requestSpeech( utterance );
     }
   }
 
@@ -293,17 +354,22 @@
    * @private
    *
    * @param {Utterance} utterance
-   * @param {boolean} [withCancel]
    */
-  requestSpeech( utterance, withCancel ) {
+  requestSpeech( utterance ) {
     assert && assert( this.isSpeechSynthesisSupported(), 'trying to speak with speechSynthesis, but it is not supported on this platform' );
 
+    // TODO: likely this will need to go, but it is nice to think about the potential for these to be aligned. Perhaps there is another place this could go after the async part of queue stepping, https://github.com/phetsims/scenery/issues/1288
+    // assert && assert( this.speakingProperty.value === this.getSynth().speaking, 'isSpeaking discrepancy' );
+
     // only cancel the previous alert if there is something new to speak
-    if ( withCancel && utterance.alert ) {
-      this.cancel();
+    // TODO: any other checks on the alert, like omitting an empty alert?, https://github.com/phetsims/scenery/issues/1288
+    if ( utterance.alert ) {
+
+      // TODO: eventually, add in support for priority.
+      this.cleanUpAndPotentiallyCancelOthers( utterance );
     }
 
-    // embeddding marks (for i18n) impact the output, strip before speaking
+    // embedding marks (for i18n) impact the output, strip before speaking
     const stringToSpeak = stripEmbeddingMarks( utterance.getTextToAlert( this.respectResponseCollectorProperties ) );
     const speechSynthUtterance = new SpeechSynthesisUtterance( stringToSpeak );
     speechSynthUtterance.voice = this.voiceProperty.value;
@@ -311,28 +377,34 @@
     speechSynthUtterance.rate = this.voiceRateProperty.value;
 
     // keep a reference to WebSpeechUtterances in Safari, so the browser doesn't dispose of it before firing, see #215
-    this.utterances.push( speechSynthUtterance );
-
-    // Keep this out of the start listener so that it can be synchrounous to the UtteranceQueue draining/announcing, see bug in https://github.com/phetsims/sun/issues/699#issuecomment-831529485
-    this.previousUtterance = utterance;
+    this.safariWorkaroundUtterances.push( speechSynthUtterance );
 
     const startListener = () => {
+      console.log( 'start' );
       this.startSpeakingEmitter.emit( stringToSpeak, utterance );
+      this.currentlySpeakingUtterance = utterance;
       this.speakingProperty.set( true );
       speechSynthUtterance.removeEventListener( 'start', startListener );
     };
 
     const endListener = () => {
-
+      console.log( 'end' );
       this.endSpeakingEmitter.emit( stringToSpeak, utterance );
       this.speakingProperty.set( false );
       speechSynthUtterance.removeEventListener( 'end', endListener );
 
       // remove the reference to the SpeechSynthesisUtterance so we don't leak memory
-      const indexOfUtterance = this.utterances.indexOf( speechSynthUtterance );
+      // TODO: We are worried that we won't always get 'end' events. If our queue is empty and the synth isn't speaking
+      // TODO: clear safariWorkaroundUtterances to prevent memory leak.
+      const indexOfUtterance = this.safariWorkaroundUtterances.indexOf( speechSynthUtterance );
       if ( indexOfUtterance > -1 ) {
-        this.utterances.splice( indexOfUtterance, 1 );
+        this.safariWorkaroundUtterances.splice( indexOfUtterance, 1 );
       }
+
+      this.currentlySpeakingUtterance = null;
+
+      // kick off the next element now that this one is done.
+      this.onSpeechSynthesisUtteranceEnd();
     };
 
     speechSynthUtterance.addEventListener( 'start', startListener );
@@ -342,20 +414,20 @@
     // but the error event does. In this case signify that speaking has ended.
     speechSynthUtterance.addEventListener( 'error', endListener );
 
+    const options = this.hasSpoken ? null : { minTimeInQueue: 0 };
+    const voicingQueueElement = new VoicingQueueElement( utterance, speechSynthUtterance, options );
+    this.voicingQueue.push( voicingQueueElement );
+
     if ( !this.hasSpoken ) {
+      this.alertNow();
 
-      // for the first time speaking it must be synchronous and we cannot use TimeoutCallbackObject workarounds yet
-      this.getSynth().speak( speechSynthUtterance );
       this.hasSpoken = true;
-
     }
-    else {
-
-      // Create and add the callback object which will request speech from SpeechSynthesis behind a small delay
-      // (as a workaround for Safari), and also track when the timeout callback is being fired so that listeners
-      // can be safely removed. See TimeoutCallbackObject for more information.
-      this.timeoutCallbackObjects.push( new TimeoutCallbackObject( speechSynthUtterance ) );
-    }
+    // Create and add the callback object which will request speech from SpeechSynthesis behind a small delay
+    // (as a workaround for Safari), and also track when the timeout callback is being fired so that listeners
+    // can be safely removed. See TimeoutCallbackObject for more information.
+    // TODO: keep this doc somewhere, https://github.com/phetsims/scenery/issues/1288
+
   }
 
   /**
@@ -391,28 +463,82 @@
    */
   cancel() {
     if ( this.initialized ) {
+
+      // Cancel anything that is being spoken currently.
       this.getSynth().cancel();
 
-      // iterate over a copy of the timeoutCallbackObjects because we will remove elements as we go through
-      this.timeoutCallbackObjects.slice().forEach( ( callbackObject, index ) => {
-
-        // Do not clear the timeout if we are cancelling as a side effect from the timeout listener being called,
-        // in that case stepTimer clear the timeout and the TimeoutCallbackObject is removed from within
-        // the listener.
-        if ( !callbackObject.timerCallingListener ) {
-          stepTimer.clearTimeout( callbackObject.stepTimerListener );
-          this.timeoutCallbackObjects.splice( index, 1 );
-        }
-      } );
+      // clear everything queued to be voiced.
+      this.voicingQueue = [];
 
       // cancel clears all utterances from the internal SpeechSynthsis queue so we should
       // clear all of our references as well
-      this.utterances = [];
+      this.safariWorkaroundUtterances = [];
+    }
+  }
+
+  /**
+   * Given one utterance, should it cancel another provided utterance?
+   * @param {Utterance} myUtterance
+   * @param {Utterance} potentialToCancelUtterance
+   * @returns {boolean}
+   * @private
+   */
+  cancelThisOneQuestionMark( myUtterance, potentialToCancelUtterance ) {
+    assert && assert( myUtterance instanceof Utterance );
+    assert && assert( potentialToCancelUtterance instanceof Utterance );
+
+    const myUtteranceOptions = merge( {}, UTTERANCE_OPTION_DEFAULTS, myUtterance.announcerOptions );
+    // const potentialToCancelUtteranceOptions = merge( UTTERANCE_OPTION_DEFAULTS, potentialToCancelUtterance.announcerOptions );
+
+    let shouldCancel = myUtteranceOptions.cancelOther;
+    if ( potentialToCancelUtterance && potentialToCancelUtterance === myUtterance ) {
+      shouldCancel = myUtteranceOptions.cancelSelf;
+    }
+
+    console.log( shouldCancel );
+    if ( !shouldCancel ) {
+      // debugger;
+    }
+    return shouldCancel;
+  }
+
+  // @private
+  cleanUpAndPotentiallyCancelOthers( utteranceThatMayCancelOthers ) {
+
+    if ( this.initialized ) {
+
+
+      // TODO: Should this go after calling getSynth().cancel()? We want to make sure that our internal queue
+      // is up to date before we start speaking the next utterance.
+      for ( let i = this.voicingQueue.length - 1; i >= 0; i-- ) {
+        const voicingQueueElement = this.voicingQueue[ i ];
+
+        if ( this.cancelThisOneQuestionMark( utteranceThatMayCancelOthers, voicingQueueElement.utterance ) ) {
+
+
+          this.removeFromVoicingQueue( voicingQueueElement );
+
+          // remove from safari workaround list to avoid memory leaks, if available
+          const index = this.safariWorkaroundUtterances.indexOf( voicingQueueElement.speechSynthUtterance );
+          this.safariWorkaroundUtterances.splice( index, 1 );
+        }
+      }
+
+      if ( this.currentlySpeakingUtterance && this.cancelThisOneQuestionMark( utteranceThatMayCancelOthers, this.currentlySpeakingUtterance ) ) {
+
+        console.log( this.currentlySpeakingUtterance.toString(), '\nis currently speaking, cancelling it for', utteranceThatMayCancelOthers.toString(), '\n\n' );
+
+        // test against what is currently being spoken by the synth (currentlySpeakingUtterance)
+        // TODO: does this call the `error` callback, or the `end` callback on a speechSynthUtterance? that would be important to know, https://github.com/phetsims/scenery/issues/1288
+        this.getSynth().cancel();
+        console.log( 'after cancel' );
+      }
     }
   }
 }
 
 /**
+ * TODO: Update documentation, https://github.com/phetsims/scenery/issues/1288
  * An inner class that is responsible for adding a listener to the stepTimer that will request
  * speech, but is also aware of when the listener is being called by the stepTimer. When voicingManager
  * is cancelled, we need to clear all timeout callbacks that will request speech from the stepTimer.
@@ -422,38 +548,33 @@
  *
  * See documentation in constructor for why a timeout is required in the first place.
  */
-class TimeoutCallbackObject {
+class VoicingQueueElement {
 
   /**
+   * @param {Utterance} utterance
    * @param {SpeechSynthesisUtterance} speechSynthUtterance
+   * @param {Object} [options]
    */
-  constructor( speechSynthUtterance ) {
+  constructor( utterance, speechSynthUtterance, options ) {
 
-    // @public (read-only) {boolean} - A field that indicates the timeout listener
-    // of this object is being called and should not be removed from stepTimer's listeners
-    // because the stepTimer will automatically try to remove it after calling the callback.
-    this.timerCallingListener = false;
+    options = merge( {
 
-    // In Safari, the `start` and `end` listener does not fire consistently, especially after interruption with
-    // cancel. But speaking behind a timeout improves the behavior significantly. A reference to the listener
-    // is saved so that it can be removed if we cancel speech. timeout of 250 ms was determined with testing
-    // to be a good value to use. Values less than 250 broke the workaround, while larger values feel too
-    // sluggish. See https://github.com/phetsims/john-travoltage/issues/435
-    this.stepTimerListener = stepTimer.setTimeout( () => {
-      const synth = voicingManager.getSynth();
-      if ( synth ) {
-        this.timerCallingListener = true;
-
-        synth.speak( speechSynthUtterance );
+      // TODO: update doc further, https://github.com/phetsims/scenery/issues/1288
+      // In Safari, the `start` and `end` listener does not fire consistently, especially after interruption with
+      // cancel. But speaking behind a timeout improves the behavior significantly. A reference to the listener
+      // is saved so that it can be removed if we cancel speech. timeout of 250 ms was determined with testing
+      // to be a good value to use. Values less than 250 broke the workaround, while larger values feel too
+      // sluggish. See https://github.com/phetsims/john-travoltage/issues/435
+      minTimeInQueue: 250
+    }, options );
 
-        // remove from voicingManager list after speaking
-        const index = voicingManager.timeoutCallbackObjects.indexOf( this );
-        assert && assert( index >= 0, 'trying to remove a callback that doesn\'t exist' );
-        voicingManager.timeoutCallbackObjects.splice( index, 1 );
-      }
-    }, 250 );
+    this.utterance = utterance;
+    this.speechSynthUtterance = speechSynthUtterance;
+    this.timeInQueue = 0;
+    this.minTimeInQueue = options.minTimeInQueue;
   }
 }
+
 
 const voicingManager = new VoicingManager();
 

[zepumph]

The main bug fixes found:

• in the cancel logic, we were merging into the defaults instead of creating a new object to merge options into.
• We noted that we had timeInQueue is seconds (250), but meant to have it be ms.
• We had to make sure to had hasSpoken to be true so that everything wasn't just sync, but can run off of the end.

We tested this further and like the results! I may commit this to master soon.

[zepumph]

I also just found one more bug where we need to wrap the end callback in a conditional that makes sure that hasSpoken is true, otherwise you can get an infinite loop if no user input occurs on a webpage (because in that case the browser speak() function immediately triggers the end callback)

[zepumph]

@jessegreenberg and I felt good enough to commit this! The commit only really differs from @jessegreenberg patch in 3 ways:

• Lots of doc updates and removing TODOs
• A clause in stepQueue that will clear the safariWorkaroundUtterances when appropriate to avoid a memory leak
• The implementation of voicingManager can skip an utterance #1288 (comment).

There are still two TODOs that I would like to discuss with @jessegreenberg on friday, but I think we are ready to add in complexity over in #1287. Marking that off hold now.

[jessegreenberg]

@KatieWoe noticed that sims were failing on CT with

Uncaught TypeError: Cannot read properties of null (reading 'speaking')

The queue was being stepped always, so I moved adding the listener to initialize and made sure to only step the queue if the voicingManager has been initialized.

[jessegreenberg]

Some discussion in meeting today with @zepumph

• We should test in Safari and iOS Safari to make sure that start and end events are working consistently.
• We need to rename functions that have some "prototypy" names currently.
• Revisit the fix for the issue mentioned in voicingManager can skip an utterance #1288 (comment)
• We would like to add an assertion that enforces that speakingProperty value matches synth.speaking. Why does it fail?

assert && assert( this.speakingProperty.value === this.getSynth().speaking, `isSpeaking discrepancy, Property value: ${this.speakingProperty.value}, synth field: ${this.getSynth().speaking}` );

[jessegreenberg]

We would like to add an assertion that enforces that speakingProperty value matches synth.speaking. Why does it fail?

I took a deeper dive on this. I discovered that synth.speaking and the start/end events just happen at different times (at least for Chrome). Once synth.speak is called, the synth.speaking flag is set to true synchronously. But the start event happens later, asynchronously. So this will happen if requestSpeech can be called in between when we call synth.speak and we receive the first start event.

Describing the above:

• synth.speak() was called on frame 56
• synth.speaking was synchronously set to true
• requestSpeech is called for the next Utterance between frames 56 and 67 <---- Where the assertion fails
• Get a start event for the first speak on frame 67, setting speakingProperty to true

This is problematic because the currentlySpeakingUtterance is set in the start listener so the first Utterance is never cancelled in this case.

[jessegreenberg]

During meeting today @zepumph and I did the last items in #1288 (comment), other than testing in iOS Safari. I will do that now. Otherwise we are ready to close this issue.

[jessegreenberg]

I just played with Friction and GFL:B with Voicing on iOS 14.6 VoiceOver for 10 minutes using lots of ReadingBlocks, interruption and queuing up alerts. Everything worked well. I think if it were broken we would see some ReadingBlocks stay highlighted as if the synth were still speaking. This catch catch in stepQueue should keep us safe.

      // This manages the case where the 'end' event came from an utterance, but there was no next utterance ready to be
      // spoken. Make sure that we support anytime that utterances are ready but there is no "end" callback that would
      // trigger `alertNow()`.
      if ( !this.getSynth().speaking && this.voicingQueue.length > 0 ) {
        this.alertNow();
      }

I think we are ready to close.