phetsims / number-play

"Number Play" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
3 stars 1 forks source link

SpeechSynthesisAnnouncer cannot be used before initialized #204

Closed jessegreenberg closed 1 year ago

jessegreenberg commented 1 year ago

@marlitas I'm not sure what is happening, but with mac + safari when I try to set the second locale, the sim freezes with this error:

Screenshot 2023-01-17 at 9 02 17 AM

When using mac + chrome, I can add the second locale and didn't notice any speech synthesis bugs.

Originally posted by @Nancy-Salpepi in https://github.com/phetsims/number-play/issues/152#issuecomment-1385481586

marlitas commented 1 year ago

I see this with Windows + firefox as well. Was working on https://github.com/phetsims/number-play/issues/160 and got blocked by this.

jessegreenberg commented 1 year ago

This lazyLink is supposed to be preventing this exact issue from happening:

    // Voices may not be available on load or the list of voices may change - update if we get an indication that
    // the list of available voices has changed.
    this.voicesProperty.lazyLink( this.updateVoiceListener );

The voicesProperty changes in response to the onvoiceschanged event and on during initialization. I am guessing that we are hitting this while the SpeechSynthesisAnnouncer is being initialized. This may have been introduced in https://github.com/phetsims/utterance-queue/issues/96 when voices became a Property.

jessegreenberg commented 1 year ago

OK, proposing the above commit to fix this. Instead of checking for populated voices we check to wait until the SpeechSynthesisAnnouncer has been initialized.

I am not sure when updateVoice needs to be called but if it has to be called on startup, you could also link it to the SpeechSynthesisAnnouncer.isInitializedProperty as well.

EDIT:

if it has to be called on startup, you could also link it to the

That seemed right so I added it. @marlitas do you agree, anything else for this issue?

pixelzoom commented 1 year ago

Over in https://github.com/phetsims/number-suite-common/issues/33, I reported that both number-play and number-compare are currently failing to start in Chrome when run from phetmarks. This is blocking further development and top priority. @chrisklus seems to think this issue is the cause.

jessegreenberg commented 1 year ago

Sorry about that! I guess I did not test https://github.com/phetsims/number-suite-common/commit/3e1652f1f9f388d1253888c4fc5612c60f2b06e9. Fixed with the above commit.

marlitas commented 1 year ago

@jessegreenberg this looks good, thanks for the work here! @chrisklus over to you for final review and closure.

chrisklus commented 1 year ago

Reviewed and discussed with @marlitas on Slack, changes look great and I tested on Safari and Chrome. Thanks @marlitas and @jessegreenberg! Closing.