phetsims / utterance-queue

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

Problem using the same Announcer and Utterance with two different UtteranceQueues #48

Closed jessegreenberg closed 2 years ago

jessegreenberg commented 2 years ago

Announcer.announcementCompleteEmitter introduces a problem with using the same Announcer with the same Utterance with two different UtteranceQueues.

Consider the following

const testUtterance = new phet.utteranceQueue.Utterance( {
    alert: 'This is a test utterance.',
    alertStableDelay: 0,
    announcerOptions: { cancelSelf: false }
} );
phet.scenery.voicingUtteranceQueue.addToBack( testUtterance )
phet.joist.joistVoicingUtteranceQueue.addToBack( testUtterance )

When the phet.scenery.voicingUtteranceQueue is finished announcing, the announcementCompleteEmitter` will fire. A listener in both UtteranceQueues will try to remove listeners from testUtterance.priorityProperty. Each UtteranceQueue in this case thinks that its Announcer is finished with the testUtterance, even though only the voicingUtteranceQueue finished speaking it.

This is a very rare case, and we will approach this with low priority until we encounter it.

jessegreenberg commented 2 years ago

I think we addressed this in the check added in #46

        // It is possible that this.announcer is also used by a different UtteranceQueue so when
        // announcementCompleteEmitter emits, it may not be for this UtteranceWrapper. this.announcingUtteranceWrapper
        // and its announcingUtterancePriorityListener could only have been set by this queue, so this check ensures
        // that we are removing the priorityProperty listener from the correct Utterance.
        if ( this.announcingUtteranceWrapper.utterance.priorityProperty.hasListener( announcingUtterancePriorityListener ) ) {
          this.announcingUtteranceWrapper.utterance.priorityProperty.unlink( announcingUtterancePriorityListener );
          // ......

I think this can be closed, @zepumph do you agree?

zepumph commented 2 years ago

Yes, I remember now. Thanks!