Bug 80260 – Speech JavaScript API: bindings and client interface

• Component: New Bugs
• Status: RESOLVED FIXED

[Hans Wennborg]

Speech JavaScript API: bindings and client interface

[Hans Wennborg]

Created attachment 130090 [details]
Patch

[Hans Wennborg]

I only updated Chromium's build files (.gyp/.gypi) for now, as I was unsure about which other systems I should update.

Looking at the xcode project, the only Modules I can see are geolocation and websockets. Looking at mediastream, I see it only in the .gypi files and GNUmakefile.list.am.

Some input into which build systems that need to be updated would be great.

[Adam Barth]

Comment on attachment 130090 [details]
Patch

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

The contents of the patch look pretty reasonable on a quick read through.  However, the patch itself is much, much too large.  Would it be possible to send patches for each interface separately?  That would allow us to review each patch carefully.  Also, you'll like need to add more tests throughout this process.

> Source/WebCore/ChangeLog:13
> +        * Modules/scripted_speech/DOMWindowSpeechRecognition.idl: Added.

scripted_speech <- WebKit doesn't use _ in its directory names.  A name like "speech" is probably more appropriate.

DOMWindowSpeechRecognition.idl <- The name of this file should follow the name of the module, so something like DOMWindowSpeech.idl

> Source/WebCore/Modules/scripted_speech/SpeechGrammarList.cpp:55
> +    // FIXME: Is this what we're supposed to do?
> +    m_grammars.append(SpeechGrammar::create(String("data:") + string, weight));

Is the first argument to SpeechGrammar::create intended to be a data URL (or is "data:" used here in another context)?

[Adam Barth]

I believe the upload tool should have warned you when you uploaded this patch that it was too large.  As I recall, it recommends patches be 20KB or less, which is 1/5th the size of this patch.

[Adam Barth]

> Some input into which build systems that need to be updated would be great.

If this feature is only enabled in Chromium, then we can start out only adding it to the Chromium build system.  When other folks want to enable the feature, we can add it to more build systems.

[Hans Wennborg]

(In reply to comment #4)
> I believe the upload tool should have warned you when you uploaded this patch that it was too large.  As I recall, it recommends patches be 20KB or less, which is 1/5th the size of this patch.

Hmm, I don't remember seeing any such warning, but I may be wrong. (I used "webkit-patch upload" from a git repository.)

Thanks for the input. I'll start breaking up the patch into smaller pieces and addressing the other issues tomorrow.