[Webkit-unassigned] [Bug 47089] Language tag in speech for HTML input elements

[bugzilla-daemon at webkit.org]

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





--- Comment #21 from Leandro Graciá Gil <leandrogracia at chromium.org>  2010-10-08 08:44:48 PST ---
(From update of attachment 70248)
View in context: https://bugs.webkit.org/attachment.cgi?id=70248&action=review

>> LayoutTests/fast/speech/input-text-language-tag.html:16
>> +    shouldBeEqualToString('document.getElementById("speechInputNone").value', 'error: no result found for language \'\'');
> 
> Instead of treating this as an error case, can you set a mock result for the empty language tag and check that here?

Discussed and rejected. We need a result in all cases to start the change event, and this is as good as any other.

>> WebCore/platform/mock/SpeechInputClientMock.cpp:33
>> +#include <wtf/text/CString.h>
> 
> Do you need this include in the latest code?

Sorry, my mistake during rebase merges.
Done.

>> WebCore/platform/mock/SpeechInputClientMock.cpp:88
>> +        m_recognitionResult.set("_", result);
> 
> Define "_" as a constant at the top of the file and use it both here and in timerFired(). Also add a comment with this constant declaration explaining what it is used for.

Done.

>> WebCore/platform/mock/SpeechInputClientMock.cpp:103
>> +            String error("error: no result found for language '");
> 
> Since this is test code and from the test result we know exactly which portion of the code called this, perhaps we don't need this human-readable error string. Instead you can just skip calling setRecognitionResult(). That also matches what the real recognizer would do, if it was not able to recognize the user voice in the given language.

Wee need some output even for the empty language case. Otherwise no change event will be generated and the test will timeout without any checking.

>> WebCore/platform/mock/SpeechInputClientMock.h:55
>> +    bool startRecognition(int, const WTF::String&, const IntRect&);
> 
> Remove the WTF:: specifier

Done.

>> WebKit/chromium/src/SpeechInputClientImpl.h:39
>> +#include <wtf/text/WTFString.h>
> 
> Remove this since you have a forward declaration for String below.

Done.

>> WebKit/chromium/src/WebSpeechInputControllerMockImpl.cpp:60
>> +    m_webcoreMock->setRecognitionResult(result, WebString::fromUTF8("en"));
> 
> Should you pass an empty string here instead of 'en' ?

Done. Now that mock results for empty language strings are again supported it makes sense.

>> WebKit/chromium/src/WebSpeechInputControllerMockImpl.h:39
>> +#include <wtf/text/WTFString.h>
> 
> Remove this since you have a forward declaration for String below.

Done.

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