Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VoicingManager] Can we get rid of the end listener in exchange for step properties? #1344

Closed
zepumph opened this issue Jan 28, 2022 · 0 comments

Comments

@zepumph
Copy link
Member

zepumph commented Jan 28, 2022

@jessegreenberg and I thought we may be able to, but it just wasn't quite working as we wanted it to. So we gave up. Here is a 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 d9475d7a375c6fc0c3be79f98712653fc2c000e0)
+++ b/js/accessibility/voicing/voicingManager.js	(date 1643404131036)
@@ -75,6 +75,9 @@
     // first request for speech.
     this.hasSpoken = false;
 
+    // @private {boolean} - the value of this.getSything().speaking last time this class was stepped
+    this.wasSpeaking = false;
+
     // @private {number} - In ms, how long to go before "waking the SpeechSynthesis" engine to keep speech
     // fast on Chromebooks, see documentation around ENGINE_WAKE_INTERVAL.
     this.timeSinceWakingEngine = 0;
@@ -145,6 +148,7 @@
     // @public {Utterance|null} - Only public for unit tests! 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.
+    // TODO: what is different about this and speakingSpeechSynthesisUtteranceWrapper?
     this.currentlySpeakingUtterance = null;
 
     // @private {SpeechSynthesisUtteranceWrapper[]} - Fixes a bug on Safari where the `start` and `end` Utterances
@@ -236,9 +240,14 @@
       // we also don't have to rely at all on the SpeechSynthesisUtterance 'end' event, which is inconsistent on
       // certain platforms.
       if ( this.timeSinceUtteranceEnd > VOICING_UTTERANCE_INTERVAL ) {
+        this.tryToHandleSpeechSynthesisEnd();
         this.readyToAnnounce = true;
       }
 
+      if ( this.wasSpeaking && !this.getSynth().speaking ) {
+        this.tryToHandleSpeechSynthesisEnd();
+      }
+
       // If our queue is empty and the synth isn't speaking, then clear safariWorkaroundUtteranceWrappers to prevent memory leak.
       // This handles any uncertain cases where the "end" callback on SpeechSynthUtterance isn't called.
       if ( !this.getSynth().speaking && queue.length === 0 && this.safariWorkaroundUtteranceWrappers.length > 0 ) {
@@ -255,6 +264,8 @@
         this.timeSinceWakingEngine = 0;
         this.getSynth().speak( new SpeechSynthesisUtterance( '' ) );
       }
+
+      this.wasSpeaking = this.getSynth().speaking;
     }
   }
 
@@ -363,30 +374,22 @@
     speechSynthUtterance.rate = this.voiceRateProperty.value;
     speechSynthUtterance.volume = this.voiceVolumeProperty.value;
 
+    // keep a reference to WebSpeechUtterances in Safari, so the browser doesn't dispose of it before firing, see #215
+    const speechSynthesisUtteranceWrapper = new SpeechSynthesisUtteranceWrapper( utterance, speechSynthUtterance, stringToSpeak );
+    this.safariWorkaroundUtteranceWrappers.push( speechSynthesisUtteranceWrapper );
+
+    this.speakingSpeechSynthesisUtteranceWrapper = speechSynthesisUtteranceWrapper;
+
     const startListener = () => {
       this.startSpeakingEmitter.emit( stringToSpeak, utterance );
       this.currentlySpeakingUtterance = utterance;
 
-      assert && assert( this.speakingSpeechSynthesisUtteranceWrapper === null, 'Wrapper should be null, we should have received an end event to clear it.' );
-      this.speakingSpeechSynthesisUtteranceWrapper = speechSynthesisUtteranceWrapper;
+      this.tryToHandleSpeechSynthesisEnd();
 
       speechSynthUtterance.removeEventListener( 'start', startListener );
     };
 
-    const endListener = () => {
-      this.handleSpeechSynthesisEnd( stringToSpeak, speechSynthesisUtteranceWrapper );
-    };
-
     speechSynthUtterance.addEventListener( 'start', startListener );
-    speechSynthUtterance.addEventListener( 'end', endListener );
-
-    // keep a reference to WebSpeechUtterances in Safari, so the browser doesn't dispose of it before firing, see #215
-    const speechSynthesisUtteranceWrapper = new SpeechSynthesisUtteranceWrapper( utterance, speechSynthUtterance, endListener );
-    this.safariWorkaroundUtteranceWrappers.push( speechSynthesisUtteranceWrapper );
-
-    // In Safari the `end` listener does not fire consistently, (especially after cancel)
-    // but the error event does. In this case signify that speaking has ended.
-    speechSynthUtterance.addEventListener( 'error', endListener );
 
     // Signify to the utterance-queue that we cannot speak yet until this utterance has finished
     this.readyToAnnounce = false;
@@ -409,23 +412,23 @@
    * Emits events signifying that we are done with speech and does some disposal.
    * @private
    *
-   * @param {string} stringToSpeak
-   * @param {SpeechSynthesisUtteranceWrapper} speechSynthesisUtteranceWrapper
    */
-  handleSpeechSynthesisEnd( stringToSpeak, speechSynthesisUtteranceWrapper ) {
-    this.endSpeakingEmitter.emit( stringToSpeak, speechSynthesisUtteranceWrapper.utterance );
-    this.announcementCompleteEmitter.emit( speechSynthesisUtteranceWrapper.utterance );
+  tryToHandleSpeechSynthesisEnd() {
+    this.speakingSpeechSynthesisUtteranceWrapper && console.log( this.speakingSpeechSynthesisUtteranceWrapper.handled, this.speakingSpeechSynthesisUtteranceWrapper.stringToSpeak );
+    if ( this.speakingSpeechSynthesisUtteranceWrapper && !this.speakingSpeechSynthesisUtteranceWrapper.handled ) {
 
-    speechSynthesisUtteranceWrapper.speechSynthesisUtterance.removeEventListener( 'end', speechSynthesisUtteranceWrapper.endListener );
+      this.endSpeakingEmitter.emit( this.speakingSpeechSynthesisUtteranceWrapper.stringToSpeak, this.speakingSpeechSynthesisUtteranceWrapper.utterance );
+      this.announcementCompleteEmitter.emit( this.speakingSpeechSynthesisUtteranceWrapper.utterance );
 
-    // remove the reference to the SpeechSynthesisUtterance so we don't leak memory
-    const indexOfPair = this.safariWorkaroundUtteranceWrappers.indexOf( speechSynthesisUtteranceWrapper );
-    if ( indexOfPair > -1 ) {
-      this.safariWorkaroundUtteranceWrappers.splice( indexOfPair, 1 );
-    }
+      // remove the reference to the SpeechSynthesisUtterance so we don't leak memory
+      const indexOfPair = this.safariWorkaroundUtteranceWrappers.indexOf( this.speakingSpeechSynthesisUtteranceWrapper );
+      if ( indexOfPair > -1 ) {
+        this.safariWorkaroundUtteranceWrappers.splice( indexOfPair, 1 );
+      }
 
-    this.speakingSpeechSynthesisUtteranceWrapper = null;
-    this.currentlySpeakingUtterance = null;
+      this.speakingSpeechSynthesisUtteranceWrapper.handled = true;
+      this.currentlySpeakingUtterance = null;
+    }
   }
 
   /**
@@ -484,13 +487,9 @@
   cancelUtterance( utterance ) {
     if ( this.currentlySpeakingUtterance === utterance ) {
 
-      // eagerly remove the end event, the browser can emit this asynchronously and we do not want to get
-      // the end event after we have finished speaking and it has been removed from the queue
-      if ( this.speakingSpeechSynthesisUtteranceWrapper ) {
-        this.handleSpeechSynthesisEnd( utterance.getAlertText(), this.speakingSpeechSynthesisUtteranceWrapper );
-      }
+      this.tryToHandleSpeechSynthesisEnd();
 
-      // silence all speech - after handleSpeechSynthesisEnd so we don't do that work twice in case the end
+      // silence all speech - after tryToHandleSpeechSynthesisEnd so we don't do that work twice in case the end
       // event is synchronous on this browser
       this.cancelSynth();
     }
@@ -554,10 +553,17 @@
  * of the SpeechSynthesisUtterance in memory long enough for the 'end' event to be emitted.
  */
 class SpeechSynthesisUtteranceWrapper {
-  constructor( utterance, speechSynthesisUtterance, endListener ) {
+
+  /**
+   * @param {Utterance} utterance
+   * @param {SpeechSynthesisUtterance} speechSynthesisUtterance
+   * @param {string} stringToSpeak
+   */
+  constructor( utterance, speechSynthesisUtterance, stringToSpeak ) {
     this.utterance = utterance;
     this.speechSynthesisUtterance = speechSynthesisUtterance;
-    this.endListener = endListener;
+    this.stringToSpeak = stringToSpeak;
+    this.handled = false; // set to true when it is done speaking or cancelled.
   }
 }
 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant