Only strip 'keyboard' from the input method name for OOBE screen.

Only strip 'keyboard' from the input method name for OOBE screen.

BUG=361855
TEST=Verified on linux_chromeos.

Committed: https://crrev.com/418c634e271b512873a8229a0ee1bbf9a973f025
Cr-Commit-Position: refs/heads/master@{#297378}

[Shu Chen, 2014-09-26 16:02:41]

shuchen@chromium.org changed reviewers:
+ nkostylev@chromium.org, nona@chromium.org

[Shu Chen, 2014-09-26 16:02:41]

Nikita/Nona, can you please review this cl? Thanks

[Nikita (slow), 2014-09-29 19:03:18]

nkostylev@chromium.org changed reviewers:
+ alemate@chromium.org

[Nikita (slow), 2014-09-29 19:03:19]

+Alexander who wrote this code initially.

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:437: LOG(ERROR) <<
"-----" << input_method.id();
nit: debug code

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:467: return
GetInputMethodLongNameInternal(input_method, true);
nit: true /* strip */

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:472: return
GetInputMethodLongNameInternal(input_method, true);
Should the strip parameter be false here?

[Alexander Alekseev, 2014-09-29 19:43:58]

On 2014/09/29 19:03:19, Nikita Kostylev wrote:
> +Alexander who wrote this code initially.

I haven't written any of this code ;-)

My implementation of "remove 'keyboard' from input method names for OOBE"
consisted of modification of strings only.

lgtm with Nikita's nits.

PS: It looks that we would need to support two different names for each keyboard
from now. That's sad.
Let's ping PMs to probably find a better way to display keyboards to users so
that we do not need two different names...


>
https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
> File chrome/browser/chromeos/input_method/input_method_util.cc (right):
> 
>
https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
> chrome/browser/chromeos/input_method/input_method_util.cc:437: LOG(ERROR) <<
> "-----" << input_method.id();
> nit: debug code
> 
>
https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
> chrome/browser/chromeos/input_method/input_method_util.cc:467: return
> GetInputMethodLongNameInternal(input_method, true);
> nit: true /* strip */
> 
>
https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
> chrome/browser/chromeos/input_method/input_method_util.cc:472: return
> GetInputMethodLongNameInternal(input_method, true);
> Should the strip parameter be false here?

[Shu Chen, 2014-09-30 01:11:15]

Thanks for your review!

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.cc (right):

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:437: LOG(ERROR) <<
"-----" << input_method.id();
On 2014/09/29 19:03:19, Nikita Kostylev wrote:
> nit: debug code

Done.

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:467: return
GetInputMethodLongNameInternal(input_method, true);
On 2014/09/29 19:03:18, Nikita Kostylev wrote:
> nit: true /* strip */

Done.

https://codereview.chromium.org/606063002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:472: return
GetInputMethodLongNameInternal(input_method, true);
On 2014/09/29 19:03:18, Nikita Kostylev wrote:
> Should the strip parameter be false here?

Done.

[Seigo Nonaka, 2014-09-30 02:42:08]

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.h:186: const
InputMethodDescriptor& input_method, bool strip) const;
What happens in the case of non US locale?
For example, "美式键盘" would be "美式" in the case of Chinese?

[Shu Chen, 2014-09-30 02:50:43]

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.h:186: const
InputMethodDescriptor& input_method, bool strip) const;
On 2014/09/30 02:42:08, Seigo Nonaka wrote:
> What happens in the case of non US locale?
> For example, "美式键盘" would be "美式" in the case of Chinese?

That depends on how the translator translates "US" in such context.
In OOBE case, the Chinese translator translates "US" to "美国英语".

[Seigo Nonaka, 2014-09-30 03:08:52]

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.h:186: const
InputMethodDescriptor& input_method, bool strip) const;
On 2014/09/30 02:50:43, Shu Chen wrote:
> On 2014/09/30 02:42:08, Seigo Nonaka wrote:
> > What happens in the case of non US locale?
> > For example, "美式键盘" would be "美式" in the case of Chinese?
> 
> That depends on how the translator translates "US" in such context.
> In OOBE case, the Chinese translator translates "US" to "美国英语".

Ah, I misunderstand. The real behavior is using translated input method id. I
misunderstand it drops the 'keyboard' word from input method name.
I think |strip| sounds like removing some strings, how about renaming the
argument as |for_oobe| or |for_shorter_space|?

[Shu Chen, 2014-09-30 03:18:56]

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/606063002/diff/40001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.h:186: const
InputMethodDescriptor& input_method, bool strip) const;
On 2014/09/30 03:08:52, Seigo Nonaka wrote:
> On 2014/09/30 02:50:43, Shu Chen wrote:
> > On 2014/09/30 02:42:08, Seigo Nonaka wrote:
> > > What happens in the case of non US locale?
> > > For example, "美式键盘" would be "美式" in the case of Chinese?
> > 
> > That depends on how the translator translates "US" in such context.
> > In OOBE case, the Chinese translator translates "US" to "美国英语".
> 
> Ah, I misunderstand. The real behavior is using translated input method id. I
> misunderstand it drops the 'keyboard' word from input method name.
> I think |strip| sounds like removing some strings, how about renaming the
> argument as |for_oobe| or |for_shorter_space|?

Done.

[Seigo Nonaka, 2014-09-30 05:28:47]

lgtm

[Nikita (slow), 2014-09-30 07:07:01]

lgtm

https://codereview.chromium.org/606063002/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/606063002/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.h:186: const
InputMethodDescriptor& input_method, bool for_oobe) const;
nit: I would suggest renaming for_oobe to short_name.

[Shu Chen, 2014-09-30 07:11:02]

https://codereview.chromium.org/606063002/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_util.h (right):

https://codereview.chromium.org/606063002/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.h:186: const
InputMethodDescriptor& input_method, bool for_oobe) const;
On 2014/09/30 07:07:01, Nikita Kostylev wrote:
> nit: I would suggest renaming for_oobe to short_name.

Done.

[Shu Chen, 2014-09-30 07:11:09]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2014-09-30 07:12:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/606063002/80001

[commit-bot: I haz the power, 2014-09-30 07:47:51]

Committed patchset #5 (id:80001) as 634cf22f842ba6ce7db52138d95ec4266ed206cd

[commit-bot: I haz the power, 2014-09-30 07:48:58]

Patchset 5 (id:??) landed as
https://crrev.com/418c634e271b512873a8229a0ee1bbf9a973f025
Cr-Commit-Position: refs/heads/master@{#297378}