Bug 80019 - Speech JavaScript API: compile- and runtime flags
Summary: Speech JavaScript API: compile- and runtime flags
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks: 80261
  Show dependency treegraph
 
Reported: 2012-03-01 04:03 PST by Hans Wennborg
Modified: 2012-03-05 04:01 PST (History)
21 users (show)

See Also:


Attachments
Patch (136.49 KB, patch)
2012-03-01 04:11 PST, Hans Wennborg
no flags Details | Formatted Diff | Diff
Patch (36.97 KB, patch)
2012-03-01 05:53 PST, Hans Wennborg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Wennborg 2012-03-01 04:03:56 PST
Implement Speech JavaScript API
Comment 1 Hans Wennborg 2012-03-01 04:11:51 PST
Created attachment 129679 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-01 04:15:33 PST
Please wait for approval from fishd@chromium.org 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
Comment 3 Hans Wennborg 2012-03-01 05:53:57 PST
Created attachment 129689 [details]
Patch

Let's add the compile- and runtime flags first to make this more reviewable.
Comment 4 WebKit Review Bot 2012-03-01 05:57:44 PST
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.
Comment 5 Adam Barth 2012-03-03 15:34:12 PST
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.
Comment 6 Satish Sampath 2012-03-03 16:27:10 PST
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.
Comment 7 Adam Barth 2012-03-03 17:00:34 PST
It's not a big deal either way.
Comment 8 WebKit Review Bot 2012-03-03 17:06:25 PST
Comment on attachment 129689 [details]
Patch

Clearing flags on attachment: 129689

Committed r109667: <http://trac.webkit.org/changeset/109667>
Comment 9 WebKit Review Bot 2012-03-03 17:06:32 PST
All reviewed patches have been landed.  Closing bug.