[Webkit-unassigned] [Bug 42603] Add a mock and LayoutTestController bindings for speech input.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42603





--- Comment #5 from Jeremy Orlow <jorlow at chromium.org>  2010-07-27 07:49:48 PST ---
(From update of attachment 62009)
Leaving r=? because others may stil wish to comment and none of these issus are terribly bad.


LayoutTests/ChangeLog:33884
 +  \ No newline at end of file
Probably best to leave the end of the changelog as is.

WebKitTools/DumpRenderTree/mac/LayoutTestControllerMac.mm:345
 +  
only one new line

WebKitTools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:733
 +      // Implement this when the Gtk port gets speech input feature.
Prefix with "FIXME: "..and maybe add an ASSERT_NOT_IMPLEMENTED().WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1345
 +      result->setNull();
most methods seem to put this at the end

WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:1346
 +      if (arguments.size() != 1 || !arguments[0].isString())
Most methods seem to use > 0 instead of != 1 (which matches javascript semantics better) and seem to put the set inside the if argument...I'd say follow the patterns.

WebKit/chromium/src/WebViewImpl.h:303
 +      void setMockSpeechInputResult(const WebString& result);
nit: "result" as the param name might be redundant


Btw, SpeechInputClient should inherit form Noncopyable.

WebCore/page/SpeechInputClientMock.cpp:61
 +      } else {
no {}'s.

WebCore/page/SpeechInputClientMock.cpp:55
 +  void SpeechInputClientMock::timerFired(Timer<SpeechInputClientMock>*)
you might want to consider using 2 timers instead of one since they're really for fairly different purposes.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.