[webkit-reviews] review denied: [Bug 80260] Speech JavaScript API: bindings and client interface : [Attachment 130090] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Hans Wennborg <hans at chromium.org>'s
request for review:
Bug 80260: Speech JavaScript API: bindings and client interface
https://bugs.webkit.org/show_bug.cgi?id=80260

Attachment 130090: Patch
https://bugs.webkit.org/attachment.cgi?id=130090&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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)?