Skip to content

Conversation

@tuncayuk
Copy link
Contributor

@tuncayuk tuncayuk commented Nov 26, 2025

  1. Change resumeInfinity to reset the existing timeout instead of calling clear() when a timer is present

  2. Introduce resumeFromKeepAlive guard so keep-alive resume() calls don’t recursively trigger resumeInfinity

Key changes for consumers:

1) Added enableUtteranceKeepAlive parameter

Previously, the startKeepAlive() logic (resume/pause) was enabled only for non-Android user agents via:

const isAndroid = /android/i.test((window.navigator || {}).userAgent || '')

However, on Comcast XI devices, triggering SpeechSynthesisUtterance resume/pause events can cause the announce to get stuck (the Promise never resolves). Also, for Comcast XI we don’t need to run the startKeepAlive() (resume/pause) logic.

With the new enableUtteranceKeepAlive option, consumers can explicitly enable/disable the keep-alive behavior from the app side:

this.$announcer.configure({ enableUtteranceKeepAlive: false });
this.$announcer.enable();

Note: The default value is based on defaultUtteranceKeepAlive, so consumers can override it if needed. This is not a breaking change.
const defaultUtteranceKeepAlive = !isAndroid

2) Added cancelPrevious parameter

While working with Vishnu, I noticed they were cancelling the previous utterance during navigation by calling three APIs:

this.$announcer.stop();
this.$announcer.clear();
this.$announcer.speak(utterance);

To simplify this requirement, I added cancelPrevious, which cancels any in-progress/queued utterance before speaking the new one:

this.$announcer.speak(utterance, 'off', { cancelPrevious: true });

This reduces multiple calls into a single, safer API call.

3) Improved clearing of utterances

I improved how we clear utterances by removing them based on the utterance ID, ensuring references are cleaned up more reliably and avoiding stale entries in the map.

@github-actions
Copy link

Test Results: ✅ PASSED

Run at: 2025-11-26T16:01:42.800Z

Summary:
passed: 917 failed: 0 of 917 tests

Copy link
Collaborator

@michielvandergeest michielvandergeest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! A few minor syntax related comments

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

Test Results: ✅ PASSED

Run at: 2025-12-02T10:28:25.494Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Test Results: ✅ PASSED

Run at: 2025-12-03T12:17:48.082Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Test Results: ✅ PASSED

Run at: 2025-12-03T12:20:32.097Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Test Results: ✅ PASSED

Run at: 2025-12-03T14:38:24.613Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Test Results: ✅ PASSED

Run at: 2025-12-03T15:06:07.990Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Test Results: ✅ PASSED

Run at: 2025-12-04T09:07:59.870Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Test Results: ✅ PASSED

Run at: 2025-12-04T09:46:56.875Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

Test Results: ✅ PASSED

Run at: 2025-12-05T10:56:39.578Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

Test Results: ✅ PASSED

Run at: 2025-12-08T13:39:40.543Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

Test Results: ✅ PASSED

Run at: 2025-12-08T16:10:34.947Z

Summary:
passed: 917 failed: 0 of 917 tests

@github-actions
Copy link

Test Results: ✅ PASSED

Run at: 2025-12-23T20:37:14.088Z

Summary:
passed: 917 failed: 0 of 917 tests

})
debounce = null
}, 200)
}, 300)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a specific reason for increasing the timeout from 200 to 300?

Comment on lines +207 to +208
// Cancel any active speech synthesis
speechSynthesis.cancel()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does clearing the queue also mean that we want to cancel the current message being spoken out? I'm not sure that it should always be so ..

if (!state) {
return
}
if (state?.timer !== null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to use optional chaining (as it doesn't transpile to very optimal es5 code)

const { utterance } = state

// utterance check: utterance instance is invalid
if (!(utterance instanceof SpeechSynthesisUtterance)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this instanceof check? would there ever be a case where it's not an instance of SpeechSynthesisUtterance? instanceof is a relatively expensive operation

}

// Clear existing timer for this specific utterance
if (state?.timer !== null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the optional chaining syntax

const initialize = () => {
// syn api check: syn might not have getVoices method
if (!syn || typeof syn.getVoices !== 'function') {
initialized = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct? if syn is falsy, do we want to set initialized to true?

const isReady = !syn.speaking && !syn.pending

if (isReady) {
Log.warn(`SpeechSynthesis - ready after ${elapsed}ms`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should probably be Log.debug statements and not Log.warn?


const startTime = Date.now()

const intervalId = window.setInterval(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use window.setInterval instead of setInterval?


if (isReady) {
Log.warn(`SpeechSynthesis - ready after ${elapsed}ms`)
window.clearInterval(intervalId)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use window.clearInterval instead of clearInterval?


utterance.onerror = (e) => {
clear()
utterances.delete(id) // Cleanup
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not necessary anymore?

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

Successfully merging this pull request may close these issues.

2 participants