Bug 80019 – Speech JavaScript API: compile- and runtime flags

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

[Hans Wennborg]

Implement Speech JavaScript API

[Hans Wennborg]

Created attachment 129679 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Hans Wennborg]

Created attachment 129689 [details]
Patch

Let's add the compile- and runtime flags first to make this more reviewable.

[WebKit Review Bot]

Attachment 129689 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitDefines.h"
Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:71:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 1 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 129689 [details]
Patch

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

> Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:157
> +#if ENABLE(SCRIPTED_SPEECH)
> +    static void setScriptedSpeechEnabled(bool isEnabled) { isScriptedSpeechEnabled = isEnabled; }
> +    static bool scriptedSpeechEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechRecognitionEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechRecognitionErrorEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechGrammarEnabled() { return isScriptedSpeechEnabled; }
> +    static bool webkitSpeechGrammarListEnabled() { return isScriptedSpeechEnabled; }
> +#endif

This change is slightly ahead of what you need in this patch.

[Satish Sampath]

Comment on attachment 129689 [details]
Patch

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

Tools/Scripts/build-webkit: line 98
> $scriptedSpeechSupport

I see the rest of this list is alphabetically sorted and I understand $scriptedSpeechSupport is placed next to $inputSpeechSupport. But since in the long term $inputSpeechSupport should be going away so it may make sense to insert $scriptedSpeechSupport in the right alphabetical order instead of the current location.

Same for ENABLE_SCRIPTED_SPEECH in lines 232-234

>> Source/WebCore/bindings/generic/RuntimeEnabledFeatures.h:157
>> +#endif
> 
> This change is slightly ahead of what you need in this patch.

I think Hans's idea was to have this patch start both the compile time and runtime flags for the feature. In that sense may be the first 2 in the above list make sense but the rest could be added in subsequent patches where they are required.

[Adam Barth]

It's not a big deal either way.

[WebKit Review Bot]

Comment on attachment 129689 [details]
Patch

Clearing flags on attachment: 129689

Committed r109667: <http://trac.webkit.org/changeset/109667>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.