fix "IME changed" notification uses a wrong field of IME

fix "IME changed" notification uses a wrong field of IME

Added a medium length name to IMEInfo. It is used for the bubble with "Your input method has changed to ...". In most cases the medium length text is equal to the short text except where this issue is concerned.

BUG=137629
TEST=Enable English and Japanese keyboards; restart; Control + Space: observe popup bubble "Your input method has changed to..."

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158641

[sschmitz, 2012-09-24 19:55:10]

Reviewers:
mukai: overall
jennyz: for owner of ash/system
zork: for owner of chrome/browser/chromeos/input_method
derat: for owner of chrome/browser/chromeos/system

[Daniel Erat, 2012-09-24 20:48:00]

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
chrome/browser/chromeos/input_method/input_method_util.cc:71: }
kMappintFromIdToMediumLengthName[] = {
s/Mappint/Mapping/

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
chrome/browser/chromeos/input_method/input_method_util.h:78: string16
GetInputMethodLongName(
did i miss something, or is this not defined?

[sschmitz, 2012-09-24 21:28:16]

Fix for Dan's comments.

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
chrome/browser/chromeos/input_method/input_method_util.cc:71: }
kMappintFromIdToMediumLengthName[] = {
On 2012/09/24 20:48:00, Daniel Erat wrote:
> s/Mappint/Mapping/
OMG (wrong in several locations). Great eyes. 
Done.

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/10960060/diff/1/chrome/browser/chromeos/input...
chrome/browser/chromeos/input_method/input_method_util.h:78: string16
GetInputMethodLongName(
On 2012/09/24 20:48:00, Daniel Erat wrote:
> did i miss something, or is this not defined?
It is defined. It is not new. It is unchanged. I only renamed the argument to
match the two functions above and put it on a new line because of length.
No change.

[Daniel Erat, 2012-09-24 21:31:29]

LGTM for ash

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:404: string16
InputMethodUtil::GetInputMethodMediumName(
nit: make the order here match that in the header (i.e. move medium between
short and long)

[jennyz, 2012-09-24 21:38:20]

lgtm for ash/system.

[Jun Mukai, 2012-09-24 21:48:32]

adding yusukes.  He would have more insights on input_methods.

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:75: {
"zinnia-japanese", "\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
It would be fine for Japanese, but what about other languages?  Whom should we
ask for it?

[Daniel Erat, 2012-09-24 21:50:27]

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:75: {
"zinnia-japanese", "\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
On 2012/09/24 21:48:32, Jun Mukai wrote:
> It would be fine for Japanese, but what about other languages?  Whom should we
> ask for it?

Is this a suggestion to make all of these instead be translatable strings (along
with the short names)?  If so, that sounds reasonable to me -- seeing these in a
C++ file was a bit unexpected.

[Yusuke Sato, 2012-09-24 22:00:50]

http://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/inp...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

http://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/inp...
chrome/browser/chromeos/input_method/input_method_util.cc:49: {
"zinnia-japanese", "\xe6\x89\x8b" },  // U+624B, "hand"
not your fault at all, but please remove this line. zinnia has been removed from
Chrome OS.

http://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/inp...
chrome/browser/chromeos/input_method/input_method_util.cc:72: { "mozc",
"\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
Two requests:

1. Please use chrome/app/generated_resources.grd database rather than hard-coded
UTF-8 Japanese for this kind of long string. You can use "Japanese IME" or
something similar as a default English string in .grd.

2. Please also handle Chinese and Korean IMEs here since "Your input method has
changed to 拼" is as ugly as "Your input method has changed to あ" (which is
crbug.com/137629)

pinyin, pinyin-dv --> "Simplified Chinese IME"
mozc-chewing, m17n:zh:cangjie, m17n:zh:quick --> "Traditional Chinese IME"
mozc-hangul --> "Korean IME"

http://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/inp...
chrome/browser/chromeos/input_method/input_method_util.cc:75: {
"zinnia-japanese", "\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
ditto. please remove zinnia.

[Yusuke Sato, 2012-09-24 22:03:44]

http://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/inp...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

http://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/inp...
chrome/browser/chromeos/input_method/input_method_util.cc:75: {
"zinnia-japanese", "\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
On 2012/09/24 21:50:27, Daniel Erat wrote:
> On 2012/09/24 21:48:32, Jun Mukai wrote:
> > It would be fine for Japanese, but what about other languages?  Whom should
we
> > ask for it?
> 
> Is this a suggestion to make all of these instead be translatable strings
(along
> with the short names)?  If so, that sounds reasonable to me -- seeing these in
a
> C++ file was a bit unexpected.

+1 for translatable string for kMappingFromIdToMediumLengthName[]. Not sure if
we should do that for the short names though (sicne we can only use up to 4
ASCII characters or 2 CJK characters there).

[sschmitz, 2012-09-24 22:48:13]

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:72: { "mozc",
"\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
On 2012/09/24 22:00:50, Yusuke Sato wrote:
> Two requests:
> 
> 1. Please use chrome/app/generated_resources.grd database rather than
hard-coded
> UTF-8 Japanese for this kind of long string. You can use "Japanese IME" or
> something similar as a default English string in .grd.
> 
> 2. Please also handle Chinese and Korean IMEs here since "Your input method
has
> changed to 拼" is as ugly as "Your input method has changed to あ" (which is
> crbug.com/137629)
> 
> pinyin, pinyin-dv --> "Simplified Chinese IME"
> mozc-chewing, m17n:zh:cangjie, m17n:zh:quick --> "Traditional Chinese IME"
> mozc-hangul --> "Korean IME"
I can do that. Who will translate? And, for each of those we really only want
one language translation besides the placeholder for English. Correct? Do we
indicate that in the  *.grd file somehow?

[Yusuke Sato, 2012-09-24 23:15:24]

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:72: { "mozc",
"\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
On 2012/09/24 22:48:14, sschmitz wrote:
> On 2012/09/24 22:00:50, Yusuke Sato wrote:
> > Two requests:
> > 
> > 1. Please use chrome/app/generated_resources.grd database rather than
> hard-coded
> > UTF-8 Japanese for this kind of long string. You can use "Japanese IME" or
> > something similar as a default English string in .grd.
> > 
> > 2. Please also handle Chinese and Korean IMEs here since "Your input method
> has
> > changed to 拼" is as ugly as "Your input method has changed to あ" (which is
> > crbug.com/137629)
> > 
> > pinyin, pinyin-dv --> "Simplified Chinese IME"
> > mozc-chewing, m17n:zh:cangjie, m17n:zh:quick --> "Traditional Chinese IME"
> > mozc-hangul --> "Korean IME"
> I can do that. Who will translate? 

L10N folks will take care of that for you:
http://dev.chromium.org/developers/design-documents/ui-localization

> And, for each of those we really only want
> one language translation besides the placeholder for English. 

hmm I don't think so. For example, when the UI language is set to French, we can
(or should) show "IME japonais", but it should be "IME japonés" when it's
Spanish.

kMappingFromIdToIndicatorText[] uses hard-coded UTF-8 strings because the length
of the string is restricted (up to 4 ASCII chars) and therefore it's hard to
translate them correctly. But for kMappingFromIdToMediumLengthName[], this
doesn't seem to be the case.

[sschmitz, 2012-09-25 01:41:43]

Moved to resource ids for medium length names; more cases; removed one. Per
suggestions.

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:49: {
"zinnia-japanese", "\xe6\x89\x8b" },  // U+624B, "hand"
On 2012/09/24 22:00:50, Yusuke Sato wrote:
> not your fault at all, but please remove this line. zinnia has been removed
from
> Chrome OS.

Done.

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:72: { "mozc",
"\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
On 2012/09/24 22:00:50, Yusuke Sato wrote:
> Two requests:
> 
> 1. Please use chrome/app/generated_resources.grd database rather than
hard-coded
> UTF-8 Japanese for this kind of long string. You can use "Japanese IME" or
> something similar as a default English string in .grd.
> 
> 2. Please also handle Chinese and Korean IMEs here since "Your input method
has
> changed to 拼" is as ugly as "Your input method has changed to あ" (which is
> crbug.com/137629)
> 
> pinyin, pinyin-dv --> "Simplified Chinese IME"
> mozc-chewing, m17n:zh:cangjie, m17n:zh:quick --> "Traditional Chinese IME"
> mozc-hangul --> "Korean IME"

Done.

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:75: {
"zinnia-japanese", "\xe6\x97\xa5\xe6\x9c\xac\xe8\xaa\x9e" },
On 2012/09/24 22:03:44, Yusuke Sato wrote:
> On 2012/09/24 21:50:27, Daniel Erat wrote:
> > On 2012/09/24 21:48:32, Jun Mukai wrote:
> > > It would be fine for Japanese, but what about other languages?  Whom
should
> we
> > > ask for it?
> > 
> > Is this a suggestion to make all of these instead be translatable strings
> (along
> > with the short names)?  If so, that sounds reasonable to me -- seeing these
in
> a
> > C++ file was a bit unexpected.
> 
> +1 for translatable string for kMappingFromIdToMediumLengthName[]. Not sure if
> we should do that for the short names though (sicne we can only use up to 4
> ASCII characters or 2 CJK characters there).
I followed Yusuke's advice and left the way for short names, but use resource
IDs for medium length names. 
Done.

https://codereview.chromium.org/10960060/diff/3002/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/input_method_util.cc:404: string16
InputMethodUtil::GetInputMethodMediumName(
On 2012/09/24 21:31:29, Daniel Erat wrote:
> nit: make the order here match that in the header (i.e. move medium between
> short and long)

Done.

[Zachary Kuznia, 2012-09-25 04:19:01]

lgtm

[Yusuke Sato, 2012-09-25 04:32:47]

Thanks. LGTM.

[commit-bot: I haz the power, 2012-09-25 15:39:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sschmitz@chromium.org/10960060/5007

[commit-bot: I haz the power, 2012-09-25 20:26:57]

Change committed as 158641