phetsims / utterance-queue

Alerting library powered by aria-live
MIT License
0 stars 2 forks source link

priorityProperty doesn't interrupt currently voiced utterance on Firefox #40

Closed zepumph closed 2 years ago

zepumph commented 2 years ago

Utterance priority because observable via Utterance.priorityProperty over in https://github.com/phetsims/joist/issues/752, but @jessegreenberg and I found that the current implementation doesn't seem to work on Firefox. This will need more investigation.

jessegreenberg commented 2 years ago

Here is the breaking case in Firefox:

// We identified a bug on Firefox

const noCancelOptions = {
  cancelSelf: false,
  cancelOther: false
};

const firstUtterance = new phet.utteranceQueue.Utterance( {
  alert: 'first utterance is long enough that it should take some time to be interrupted.',
  alertStableDelay: 0,
  announcerOptions: noCancelOptions
} );
const secondUtterance = new phet.utteranceQueue.Utterance( {
  alert: 'second utterance',
  alertStableDelay: 0,
  announcerOptions: noCancelOptions
} );

const thirdUtterance = new phet.utteranceQueue.Utterance( {
  alert: 'third utterance',
  alertStableDelay: 0,
  announcerOptions: noCancelOptions
} );

secondUtterance.priorityProperty.value = 1;
phet.scenery.voicingUtteranceQueue.addToBack( firstUtterance );
phet.scenery.voicingUtteranceQueue.addToBack( secondUtterance );
phet.scenery.voicingUtteranceQueue.addToBack( thirdUtterance );
( async () => {
  await timeout( 1000 );
  secondUtterance.priorityProperty.value = 2;
} )()

The secondUtterance priority is set such that it should interrupt the first utterance, and then we should hear the third utterance.

EDIT: The async wasn't getting called in Firefox, I changed it to this but it still isn't working:

secondUtterance.priorityProperty.value = 1;
phet.scenery.voicingUtteranceQueue.addToBack( firstUtterance );
phet.scenery.voicingUtteranceQueue.addToBack( secondUtterance );
phet.scenery.voicingUtteranceQueue.addToBack( thirdUtterance );
setTimeout( () => {
  secondUtterance.priorityProperty.value = 2;
  console.log( 'Setting priority of secondUtterance' );
}, 1000)
jessegreenberg commented 2 years ago

In Chrome, we eventually call voicingManager.cancelSynth() when changing secondUtterance.priorityProperty. We never hit that in Firefox.

EDIT: For some reason the UtteranceQueue.queue is empty in Firefox by the time we set the Priority. In Chrome there are still 2 utterances in the queue (which makes sense).

jessegreenberg commented 2 years ago

const noCancelOptions = { cancelSelf: false, cancelOther: false };

const firstUtterance = new phet.utteranceQueue.Utterance( { alert: 'first utterance is long enough that it should take some time to be interrupted.', alertStableDelay: 0, announcerOptions: noCancelOptions } ); const secondUtterance = new phet.utteranceQueue.Utterance( { alert: 'second utterance', alertStableDelay: 0, announcerOptions: noCancelOptions } );

const thirdUtterance = new phet.utteranceQueue.Utterance( { alert: 'third utterance', alertStableDelay: 0, announcerOptions: noCancelOptions } );

secondUtterance.priorityProperty.value = 1; phet.scenery.voicingUtteranceQueue.addToBack( firstUtterance ); phet.scenery.voicingUtteranceQueue.addToBack( secondUtterance ); phet.scenery.voicingUtteranceQueue.addToBack( thirdUtterance ); setTimeout( () => { secondUtterance.priorityProperty.value = 2; console.log( 'Setting priority of secondUtterance' ); }, 1000)

jessegreenberg commented 2 years ago

The problem comes from using this.getSynth().speaking and readyToSpeak

      // Increment the amount of time since the synth has stopped speaking the previous utterance, but don't
      // start counting up until the synth has finished speaking its current utterance.
      this.timeSinceUtteranceEnd = this.getSynth().speaking ? 0 : this.timeSinceUtteranceEnd + dt;

      // Wait until VOICING_UTTERANCE_INTERVAL to speak again for more consistent behavior on certain platforms,
      // see documentation for the constant for more information. By setting readyToSpeak in the step function
      // 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.readyToSpeak = true;
      }

In Firefox, this.getSynth().speaking is false for a while after the synth starts speaking (it must be set asyncronously). It is true immediately after speech starts in Chrome, it must be set true synchronously.

So in Firefox, readyToSpeak will almost always be true, and Utterances will get pulled from the UtteranceQueue really fast.

jessegreenberg commented 2 years ago

The fix I applied was to eagerly update a timing variable for VOICING_UTTERANCE_INTERVAL so that when we call requestSpeech readyToSpeak will be false until the interval is up, giving the browser time to set SpeechSynthesis.speaking to true.

Then I went back to the unit tests and updated the timeouts so that we wait for times that are ~half of the time it takes the Utterance to speak instead of just waiting close to VOICING_UTTERANCE_INTERVAL. With SpeechSynthesis.speaking set at inconsistent times, we need the tests and times to be much longer than than that interval and however long it will take the browser to actually start speaking. With these times I was able to add back the commented out tests.

@zepumph can we review this tomorrow together since it came out of work in https://github.com/phetsims/joist/issues/752

jessegreenberg commented 2 years ago

Reviewed this with @zepumph today and all seems well with the changes. CLosing.