Skip to content
This repository has been archived by the owner on Sep 11, 2022. It is now read-only.

Update speech-color-changer to be compatible with Safari #64

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

kennethkufluk
Copy link
Contributor

Safari does not currently implement webkitSpeechGrammarList and throws an error at line 2.
I have guarded against this, so the script now works in Safari on desktop and iPhone.

I've also added a comment around the grammar code because no browser currently supports them, as noted in this WICG discussion:
WICG/speech-api#57

Since we'll check usage stats to determine if grammars should be removed from the spec, and this script encourages grammar usage (I imagine this code will be copy/pasted assuming they have an effect), I added a comment to discourage their use.

I didn't want to remove grammars completely, as this script is used as a demonstration of the spec on MDN.

Safari does not currently implement webkitSpeechGrammarList and throws an error at line 2.
I have guarded against this, so the script now works in Safari on desktop and iPhone.

I've also added a comment around the grammar code because no browser currently supports them, as noted in this WICG discussion:
WICG/speech-api#57 

Since we'll check usage stats to determine if grammars should be removed from the spec, and this script encourages grammar usage (I imagine this code will be copy/pasted assuming they have an effect), I added a comment to discourage their use.

I didn't want to remove grammars completely, as this script is used as a demonstration of the spec on MDN.
@@ -1,14 +1,18 @@
var SpeechRecognition = SpeechRecognition || webkitSpeechRecognition
var SpeechGrammarList = SpeechGrammarList || webkitSpeechGrammarList
var SpeechGrammarList = SpeechGrammarList || window.webkitSpeechGrammarList
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a mistake to me. Did you meant to fix the line above?

var SpeechRecognition = SpeechRecognition || webkitSpeechRecognition || window.webkitSpeechRecognition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, but we could fix it there too. Safari is missing webkitSpeechGrammarList and so throws an error unless you check the property on window. It wasn't missing webkitSpeechRecognition in the version I tested so I didn't see an error there.

var speechRecognitionList = new SpeechGrammarList();
speechRecognitionList.addFromString(grammar, 1);
recognition.grammars = speechRecognitionList;
if (SpeechGrammarList) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this so it could run on Safari too.

Copy link
Contributor

@schalkneethling schalkneethling left a comment

Choose a reason for hiding this comment

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

This looks good to me, thank you @kennethkufluk 🎉

@schalkneethling schalkneethling merged commit 27de740 into mdn:master Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants