Summary: | Speech JavaScript API: compile- and runtime flags | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hans Wennborg <hans> | ||||||
Component: | New Bugs | Assignee: | Hans Wennborg <hans> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, arun.patole, cc-bugs, cgarcia, donggwan.kim, fishd, gustavo, jamesr, japhet, levin+threading, macpherson, menard, ojan, peter, rakuco, satish, tkent, vestbo, webkit.review.bot, zan, zoltan | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 80261 | ||||||||
Attachments: |
|
Description
Hans Wennborg
2012-03-01 04:03:56 PST
Created attachment 129679 [details]
Patch
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 Created attachment 129689 [details]
Patch
Let's add the compile- and runtime flags first to make this more reviewable.
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 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 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. It's not a big deal either way. Comment on attachment 129689 [details] Patch Clearing flags on attachment: 129689 Committed r109667: <http://trac.webkit.org/changeset/109667> All reviewed patches have been landed. Closing bug. |