Closed Bug 1167541 Opened 5 years ago Closed 4 years ago

SpeechSynthesisEvent::utterance isn't implemented

Categories

(Core :: Web Speech, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: kdavis, Assigned: kdavis)

References

Details

(Whiteboard: [webspeechapi][systemsfe])

Attachments

(1 file, 1 obsolete file)

SpeechSynthesisEvent::utterance isn't implemented. See WebSpeech API https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#speechsynthesisevent
Whiteboard: [webspeechapi]
Component: DOM → Web Speech
Assignee: nobody → kdavis
Whiteboard: [webspeechapi] → [webspeechapi][systemsfe]
Part 1 of 1: Implemented  SpeechSynthesisEvent::utterance

This is the part 1 of 1 for this bug.

This patch implements the member utterance of SpeechSynthesisEvent,
which for whatever reason was not implemented.

This adheres to the WebSpeech API which states that SpeechSynthesisEvent
has an utterance member of type SpeechSynthesisUtterance

https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#dfn-callbackutterance


The try for this patch is running here https://treeherder.mozilla.org/#/jobs?repo=try&revision=df56687cd2d8
Attachment #8627648 - Flags: review?(bugs)
(Did I forget to file a spec bug to add Constructor for the event.)
(In reply to Olli Pettay [:smaug] from comment #2)
> Doesn't
> http://mxr.mozilla.org/mozilla-central/source/dom/events/test/
> test_all_synthetic_events.html?force=1#439 fail with this?

at least in case media.webspeech.synth.enabled is set to true.
(In reply to Olli Pettay [:smaug] from comment #5)
> (In reply to Olli Pettay [:smaug] from comment #2)
> > Doesn't
> > http://mxr.mozilla.org/mozilla-central/source/dom/events/test/
> > test_all_synthetic_events.html?force=1#439 fail with this?
> 
> at least in case media.webspeech.synth.enabled is set to true.

Completely right! M(7) Nice catch!

The question then is what to do for the SpeechSynthesisEvent test
in test_all_synthetic_events.html.

Do you have an idea?
So the options are to make the property nullable, and then
in dictionary also nullable and default to = null;

Or, when creating the event pass an utterance object in the dictionary.
And since SpeechSynthesisUtterance has a Constructor, 
perhaps something like
aProps.utterance = new SpeechSynthesisUtterance();
return new SpeechSynthesisEvent(aName, aProps);
in the test.

I think the latter would make sense at least for now since the spec has non-nullable utterance.
Attachment #8627648 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #7)
> So the options are to make the property nullable, and then
> in dictionary also nullable and default to = null;
> 
> Or, when creating the event pass an utterance object in the dictionary.
> And since SpeechSynthesisUtterance has a Constructor, 
> perhaps something like
> aProps.utterance = new SpeechSynthesisUtterance();
> return new SpeechSynthesisEvent(aName, aProps);
> in the test.
> 
> I think the latter would make sense at least for now since the spec has
> non-nullable utterance.

Ok I'll do the latter.

Thanks!
Actually, since SpeechSynthesisErrorEvent extend SpeechSynthesisEvent, and looks like we don't 
always have a sane utterance, would making utterance nullable make more sense after all?
(In reply to Olli Pettay [:smaug] from comment #9)
> Actually, since SpeechSynthesisErrorEvent extend SpeechSynthesisEvent, and
> looks like we don't 
> always have a sane utterance, would making utterance nullable make more
> sense after all?

I think we always have a sane utterance.

Looking at

https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#utterance-events

it seems that all the the EventHandler's of SpeechSynthesisUtterance except onerror emit 
SpeechSynthesisEvent's while the onerror EventHandler emits a SpeechSynthesisErrorEvent.

So I think your solution is fine.
But what is the utterance in those error events?
(In reply to Olli Pettay [:smaug] from comment #11)
> But what is the utterance in those error events?

I think is should be the SpeechSynthesisUtterance the onerror
EventHandler is registered with.
(In reply to Olli Pettay [:smaug] from comment #11)
> But what is the utterance in those error events?

One thing I just noticed, however, is the EventHandler onvoiceschanged
on SpeechSynthesis 

https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#dfn-ttsonvoiceschanged

can not emit a SpeechSynthesisEvent as there is no associated utterance.

So, it must emit just a normal Event which seems wrong to me as a generic
Event, or for that matter a SpeechSynthesisEvent, can not say what voices
have been added or removed as it doesn't have the proper members to do so.

This seems like a WebSpeech API bug to me.
(In reply to kdavis from comment #12)
> (In reply to Olli Pettay [:smaug] from comment #11)
> I think is should be the SpeechSynthesisUtterance the onerror
> EventHandler is registered with.
Ah, right.

And yes voiceschanged should be probably just an Event, not SpeechSynthesisEvent.
Part 1 of 1: Implemented  SpeechSynthesisEvent::utterance

This is the part 1 of 1 for this bug.

This patch implements the member utterance of SpeechSynthesisEvent,
which for whatever reason was not implemented.

This adheres to the WebSpeech API which states that SpeechSynthesisEvent
has an utterance member of type SpeechSynthesisUtterance

https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#dfn-callbackutterance

The interdiff between this patch and the one it obsoletes is

====================================================
--- a/dom/events/test/test_all_synthetic_events.html
+++ b/dom/events/test/test_all_synthetic_events.html
@@ -436,6 +436,7 @@ const kEventConstructors = {
                                                        },
                                              },
   SpeechSynthesisEvent:                      { create: function (aName, aProps) {
+                                                         aProps.utterance = new SpeechSynthesisUtterance("Hello World");
                                                          return new SpeechSynthesisEvent(aName, aProps);
                                                        },
                                              },
====================================================

a changed discussed in Comment 7

The try for this patch is running here https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bb37f4ccb8b

Note in particular M(7) of the B2G emulator tests is OK.
Attachment #8627648 - Attachment is obsolete: true
Attachment #8628214 - Flags: review?(bugs)
Attachment #8628214 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Blocks: 1167542
https://hg.mozilla.org/mozilla-central/rev/75951c11a9bc
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.