Closed Bug 1160844 Opened 9 years ago Closed 9 years ago

test_speech_queue.html is always failure when the backend adds en-US voice synth

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: m_kato, Assigned: eeejay)

References

Details

(Whiteboard: [webspeechapi])

Attachments

(1 file, 1 obsolete file)

As long as I look nsSynthVoiceRegistry::FindBestMatch, the voice is selected by the following rules.

1. if lang by WebSpeech API or document lang by HTML is set, it is used for selecting the voice.
2. Use UI lang's voice
3. Use en-US lang's voice
4. Use default voice

So, although this test calls synthSetDefault, if backend adds en-US, this test always failure due to 1. and 3.

Also, this test is disabled on B2G, so this isn't tested when adding any backend.
Blocks: 1003457
Whiteboard: [webspeechapi]
Summary: test_speech_queue.html is always failure when the backed adds en-US voice synth → test_speech_queue.html is always failure when the backend adds en-US voice synth
Assignee: nobody → m_kato
Comment on attachment 8600757 [details] [diff] [review]
test_speech_queue cannot use mock voice when backend exists

Actually, selecting order of voice by web speech synth is the following.

1. if lang by WebSpeech API or document lang by HTML is set, it is used.
2. Use UI lang's voice
3. Use en-US lang's voice
4. Use default voice by nsISynthVoiceRegistry::setDefaultVoice

So if backend such as Pico or upcoming SAPI exists, it usually adds en-US voice at least.  So web speech synth cannot select default voice when backend exists.

I think we should change order of this rules by the following for mock mochitest.

1. lang by WebSpeech API or document lang's voice
2. Use default voice
3. Use UI lang's voice
4. Use en-US lang's voice
Attachment #8600757 - Flags: review?(bugs)
Comment on attachment 8600757 [details] [diff] [review]
test_speech_queue cannot use mock voice when backend exists

s/becomces/becomes the/
Attachment #8600757 - Flags: review?(eitan)
Attachment #8600757 - Flags: review?(bugs)
Attachment #8600757 - Flags: review+
(In reply to Makoto Kato (:m_kato) from comment #2)
> I think we should change order of this rules by the following for mock mochitest.
> 
> 1. lang by WebSpeech API or document lang's voice
> 2. Use default voice
> 3. Use UI lang's voice
> 4. Use en-US lang's voice

Maybe a longer term question, but the Web Speech API makes no allowances for fallback voices.

In other words, if, using a URI in the Web Speech API, you select a voice that is not there,
you obtain no voice.

Thus, as far as I can see, the old and proposed nsSynthVoiceRegistry::FindBestMatch() seems to
not adhere to the Web Speech API specification.

Is this something we want to do?
I'm working to fix this too! I didn't open a specific bug for this, but basically I want to add a pref:
media.webspeech.synth.test

When it is true (and it will be true in mochitest profiles), built in backends will be disabled so they don't interfere with tests.
(In reply to kdavis from comment #4)
> (In reply to Makoto Kato (:m_kato) from comment #2)
> > I think we should change order of this rules by the following for mock mochitest.
> > 
> > 1. lang by WebSpeech API or document lang's voice
> > 2. Use default voice
> > 3. Use UI lang's voice
> > 4. Use en-US lang's voice
> 
> Maybe a longer term question, but the Web Speech API makes no allowances for
> fallback voices.
> 
> In other words, if, using a URI in the Web Speech API, you select a voice
> that is not there,
> you obtain no voice.
> 
> Thus, as far as I can see, the old and proposed
> nsSynthVoiceRegistry::FindBestMatch() seems to
> not adhere to the Web Speech API specification.
> 
> Is this something we want to do?

AFAIK, you can't specify an explicit URI (in a previous spec version you could). You need to set SpeechSynthesisUtterance.voice to an actual SpeechSynthesisVoice object. So this issue shouldn't come up.
Also, I am working on a patch to remove the JS tts backends, and add fake test ones. This will allow us to enable e10s tests and remove the horribly hackish ipc tests.
(In reply to Eitan Isaacson [:eeejay] from comment #6)
> (In reply to kdavis from comment #4)
> > 
> > Maybe a longer term question, but the Web Speech API makes no allowances for
> > fallback voices.
> > 
> > In other words, if, using a URI in the Web Speech API, you select a voice
> > that is not there, you obtain no voice.
> > 
> > Thus, as far as I can see, the old and proposed nsSynthVoiceRegistry::FindBestMatch()
> > seems to not adhere to the Web Speech API specification.
> > 
> > Is this something we want to do?
> 
> AFAIK, you can't specify an explicit URI (in a previous spec version you
> could). You need to set SpeechSynthesisUtterance.voice to an actual
> SpeechSynthesisVoice object. So this issue shouldn't come up.

You are correct. I was thinking of the old spec.

In particular, the 19 October 2012 Web Speech API Specification had the attribute
SpeechSynthesisUtterance::voiceURI while the 6 June 2014 Web Speech API Specification
has the attribute SpeechSynthesisUtterance::voice.
Here is a try run with all the changes I am proposing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c19313dc58f

There are four separate patches there:
1. Don't enable nsPicoService if synth.test is true.
2. Add SpeechSynthesisUtterance.chosenVoiceURI for testing purposes.
3. Make speech synth tests e10s friendly by adding a built in backend instead of using a JS one via SpecialPowers.
4. Remove speech synth ipc tests, since e10s is providing the same coverage.

With all these changes, the synth tests could be enabled on e10s and all platforms (well, we'll see how well the try run goes :)
Eitan, If you add prefs for test, it is good for Pico and bug 1003457.

We cannot add new backend now since current mochitests (test_speech_queue.html and test_indirect_service_events.html) are always failed with any OS backend.
Comment on attachment 8600757 [details] [diff] [review]
test_speech_queue cannot use mock voice when backend exists

Review of attachment 8600757 [details] [diff] [review]:
-----------------------------------------------------------------

I'm going to r- this. I think the pref approach will be better in the longer term since it will allow all the tests to run with a predictable voice set.
Attachment #8600757 - Flags: review?(eitan) → review-
Grabbing this bug from Makoto with his consent :)

This disables the built-in platform speech service. In this case, the B2G pico service. I didn't reenable any mochitests yet because in B2G there is another issue that the tests run in content processes, so this won't work anyway.
Attachment #8600757 - Attachment is obsolete: true
Attachment #8602937 - Flags: review?(bugs)
Assignee: m_kato → eitan
Attachment #8602937 - Flags: review?(bugs) → review+
Blocks: 1162699
Gip doesn't throw error when using try again.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5370ae616010

I think the folowing means infra issue.

00:20:38 ERROR - TEST-UNEXPECTED-ERROR | test_a11y_color_filters.py TestColorFiltersAccessibility.test_a11y_color_filters | DMError: unable to execute ADB ([Errno 2] No such file or directory): ensure Android SDK is installed and adb is in your $PATH
This was caused by a bad patch in bug 1162692, relanding.
Flags: needinfo?(eitan)
https://hg.mozilla.org/mozilla-central/rev/763bd3743f94
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Blocks: 1187155
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.