Bug 80513 – Speech JavaScript API: SpeechRecognitionEvent

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Hans Wennborg]

Speech JavaScript API: SpeechRecognitionEvent

[Hans Wennborg]

Created attachment 130625 [details]
Patch

[Hans Wennborg]

Created attachment 130818 [details]
Patch

Rebase, const-qualify readonly attribute getters, return raw pointers.

[Adam Barth]

Comment on attachment 130818 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130818&action=review

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:29
> +    interface [
> +        Conditional=SCRIPTED_SPEECH
> +    ] SpeechRecognitionEvent : Event {

Please use the ConstructorTemplate=Event and InitializedByEventConstructor:

https://trac.webkit.org/wiki/WebKitIDL#ConstructorTemplate

This will generate DOM4-style event constructors.

[Adam Barth]

+haraken because of ConstructorTemplate=Event

[Adam Barth]

Comment on attachment 130818 [details]
Patch

Once you have a DOM4-style event constructor, you can write tests that exercise this class.  Note: you should also be able to exercise it with document.createEvent.

[Kentaro Hara]

Comment on attachment 130818 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130818&action=review

>> Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:29
>> +    ] SpeechRecognitionEvent : Event {
> 
> Please use the ConstructorTemplate=Event and InitializedByEventConstructor:
> 
> https://trac.webkit.org/wiki/WebKitIDL#ConstructorTemplate
> 
> This will generate DOM4-style event constructors.

And please add fast/events/constructors/speech-recognition-event-constructors.html for tests.

[Hans Wennborg]

(In reply to comment #6)
> (From update of attachment 130818 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130818&action=review
> 

Thanks very much for the input!

> >> Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:29
> >> +    ] SpeechRecognitionEvent : Event {
> > 
> > Please use the ConstructorTemplate=Event and InitializedByEventConstructor:
> > 
> > https://trac.webkit.org/wiki/WebKitIDL#ConstructorTemplate
> > 
> > This will generate DOM4-style event constructors.
Done.

> 
> And please add fast/events/constructors/speech-recognition-event-constructors.html for tests.
Done.

New patch coming up.

[Hans Wennborg]

Created attachment 131057 [details]
Patch

[Adam Barth]

Comment on attachment 131057 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131057&action=review

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.cpp:32
> +

You've got an extra blank line here.

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.cpp:70
> +void SpeechRecognitionEvent::initSpeechRecognitionEvent(const AtomicString& type, bool canBubble, bool cancelable, SpeechRecognitionResult* result, SpeechRecognitionError* error, short resultIndex, SpeechRecognitionResultList* resultHistory)

Oh, you can skip these init functions.  DOM4-style events don't need them because you can just construct the event directly.  We've been removing them from the platform.

> Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:42
> +        void initSpeechRecognitionEvent(in [Optional=DefaultIsUndefined] DOMString typeArg,
> +                                        in [Optional=DefaultIsUndefined] boolean canBubbleArg,
> +                                        in [Optional=DefaultIsUndefined] boolean cancelableArg,
> +                                        in [Optional=DefaultIsUndefined] SpeechRecognitionResult resultArg,
> +                                        in [Optional=DefaultIsUndefined] SpeechRecognitionError errorArg,
> +                                        in [Optional=DefaultIsUndefined] short resultIndexArg,
> +                                        in [Optional=DefaultIsUndefined] SpeechRecognitionResultList resultHistoryArg);

We should skip this.  Sorry I didn't notice it in your previous patch.

> LayoutTests/fast/events/constructors/speech-recognition-event-constructor.html:30
> +// Test passing resultIndex in the initializer.
> +shouldBe("new webkitSpeechRecognitionEvent('eventType', { resultIndex: 42 }).resultIndex", "42");

I mean, you can also test the other properties, but maybe that's not worth while.

[Hans Wennborg]

(In reply to comment #9)
> (From update of attachment 131057 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131057&action=review

Thanks very much for the review!

> > Source/WebCore/Modules/speech/SpeechRecognitionEvent.cpp:32
> > +
> 
> You've got an extra blank line here.
Done.

> > Source/WebCore/Modules/speech/SpeechRecognitionEvent.cpp:70
> > +void SpeechRecognitionEvent::initSpeechRecognitionEvent(const AtomicString& type, bool canBubble, bool cancelable, SpeechRecognitionResult* result, SpeechRecognitionError* error, short resultIndex, SpeechRecognitionResultList* resultHistory)
> 
> Oh, you can skip these init functions.  DOM4-style events don't need them because you can just construct the event directly.  We've been removing them from the platform.
Done.

> > Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:42
> > +        void initSpeechRecognitionEvent(in [Optional=DefaultIsUndefined] DOMString typeArg,
> > +                                        in [Optional=DefaultIsUndefined] boolean canBubbleArg,
> > +                                        in [Optional=DefaultIsUndefined] boolean cancelableArg,
> > +                                        in [Optional=DefaultIsUndefined] SpeechRecognitionResult resultArg,
> > +                                        in [Optional=DefaultIsUndefined] SpeechRecognitionError errorArg,
> > +                                        in [Optional=DefaultIsUndefined] short resultIndexArg,
> > +                                        in [Optional=DefaultIsUndefined] SpeechRecognitionResultList resultHistoryArg);
> 
> We should skip this.  Sorry I didn't notice it in your previous patch.
Done.

It wasn't in the first patch. I copied it from StorageEvent.idl when adding ConstructorTemplate=Event, thinking maybe it was supposed to be there too. Thanks for clarifying.

> > LayoutTests/fast/events/constructors/speech-recognition-event-constructor.html:30
> > +// Test passing resultIndex in the initializer.
> > +shouldBe("new webkitSpeechRecognitionEvent('eventType', { resultIndex: 42 }).resultIndex", "42");
> 
> I mean, you can also test the other properties, but maybe that's not worth while.

The SpeechRecognitionResult, Error and ResultList don't have constructors, so there's no straightforward way of testing those properties :/ Maybe I should just add constructors for them.

Also realized I forgot to add this layout test to the Skipped files for the other ports. Doing that now.

[Hans Wennborg]

Committed r110420: <http://trac.webkit.org/changeset/110420>