[Webkit-unassigned] [Bug 41518] Speech input plumbing in webcore and webkit
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jul 6 07:15:11 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=41518
Steve Block <steveblock at google.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #60359|review? |review-
Flag| |
--- Comment #4 from Steve Block <steveblock at google.com> 2010-07-06 07:15:11 PST ---
(From update of attachment 60359)
LayoutTests/fast/forms/input-text-speechbutton.html:22
+ layoutTestController.notifyDone();
if (window.layoutTestController)
LayoutTests/fast/forms/input-text-speechbutton.html:1
+ <html>
I think we should be using script-tests for these kind of tests.
WebCore/Android.mk:383
+ ifeq ($(ENABLE_INPUT_SPEECH), true)
No need to guard this. Contents of file should be guarded.
WebCore/html/HTMLInputElement.cpp:2777
+ return hasAttribute(speechAttr) && document()->page()->isSpeechInputSupported();
Do we need this extra check? Surely any platform that sets ENABLE(INPUT_SPEECH) will provide a speech client, and this entire method is guarded by ENABLE(INPUT_SPEECH).
WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:170
+ #if ENABLE(INPUT_SPEECH)
I think this shouldn't be guarded
WebKitTools/ChangeLog:13
+ * DumpRenderTree/chromium/LayoutTestController.h:
I think that all new LayoutTestController methods need to be implemented for Mac.
WebKit/chromium/ChangeLog:5
+ Speech input plumbing in webcore and webkit
Can we leave the WebKit/chromium changes for a separate patch to keep this one simpler?
WebCore/page/SpeechInput.h:45
+ class SpeechInputElementListener {
Shouldn't this be named SpeechInputListener, as it listens to SpeechInput?
WebCore/page/SpeechInputClient.h:39
+ class SpeechInputListener {
Shouldn't this be named SpeechInputClientListener, as it listens to SpeechInputClient?
WebCore/page/SpeechInputClient.h:42
+ virtual void recognitionResult(const String& result) = 0;
setRecognitionResult()?
WebCore/page/Page.h:155
+ void setSpeechInputClient(SpeechInputClient* client) { m_speechInputClient = client; }
Is there any reason for the client to be reset, other than for testing purposes? If not, I don't think we need this method. The embedder can configure their SpeechInputClient to be mocked out without having to call into WebCore.
WebCore/page/Page.cpp:133
+ , m_speechInputClient(0)
For other services (eg Geolocation, DeviceOrientation), the Page owns a FooController object, which holds a weak reference to the client, which is owned by the embedder. The controller takes care of multiplexing multiple requests from the page to the client. Here the Page holds a reference to the client directly. I see that you have a separate SpeechInputElement object for each input. Is there no need for a controller type object? Even if not, it might be best to add a SpeechInputController just to keep the pattern the same. It can have a single createSpeechElement(SpeechInputElementListener*) method.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list