Move all white-listed input method indicator text from InputMethodUtil to manifests of IME extensio…

Move all white-listed input method indicator text from InputMethodUtil to manifests of IME extensions.

BUG=232260
TEST=Verified on linux_chromeos.

Committed: https://crrev.com/bc91aaa2ed163990f30a36b5999e722ed548977c
Cr-Commit-Position: refs/heads/master@{#297357}

[Shu Chen, 2014-09-29 02:28:00]

shuchen@chromium.org changed reviewers:
+ benwells@chromium.org, nona@chromium.org, plundblad@chromium.org

[Shu Chen, 2014-09-29 02:28:01]

Can you please review this cl?

nona@ for general IMF.
benwells@ for extensions/common/...
plundblad@ for chrome/browser/resources/chromeos/braille_ime/...

Thanks!

[Peter Lundblad, 2014-09-29 12:20:47]

lgtm for braille_ime with small suggestions.

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

https://codereview.chromium.org/608183002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:283: // TODO(shuchen):
remove this method.
Does 'this method' refer to the one being called on the next line or the calee?
Might want to add a bit more explanation to the TODO for the uninitiated reader.

https://codereview.chromium.org/608183002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/braille_ime/manifest.json (right):

https://codereview.chromium.org/608183002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/braille_ime/manifest.json:21: "indicator":
"\u2803\u2817\u2807",
Please port over the comment describing what this means (sans the actual
character values which are now obvious from the string itself).

[Shu Chen, 2014-09-29 14:48:08]

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

https://codereview.chromium.org/608183002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_util.cc:283: // TODO(shuchen):
remove this method.
On 2014/09/29 12:20:47, Peter Lundblad wrote:
> Does 'this method' refer to the one being called on the next line or the
calee?
> Might want to add a bit more explanation to the TODO for the uninitiated
reader.

Done.

https://codereview.chromium.org/608183002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/braille_ime/manifest.json (right):

https://codereview.chromium.org/608183002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/braille_ime/manifest.json:21: "indicator":
"\u2803\u2817\u2807",
On 2014/09/29 12:20:47, Peter Lundblad wrote:
> Please port over the comment describing what this means (sans the actual
> character values which are now obvious from the string itself).

Done.

[Shu Chen, 2014-09-29 14:52:06]

shuchen@chromium.org changed reviewers:
+ scheib@chromium.org
- benwells@chromium.org

[Shu Chen, 2014-09-29 14:52:06]

+scheib@ as benwells is OOO.

[scheib, 2014-09-29 16:00:15]

extensions/common/... lgtm

[Seigo Nonaka, 2014-09-30 02:49:01]

lgtm with one comment.

Thank you for your cleaning up!

https://codereview.chromium.org/608183002/diff/40001/chromeos/ime/input_metho...
File chromeos/ime/input_method_descriptor.cc (right):

https://codereview.chromium.org/608183002/diff/40001/chromeos/ime/input_metho...
chromeos/ime/input_method_descriptor.cc:52: return indicator_;
This |indicator_| can be set by third party, right?

If so, I think the UI is not good if the indicator is too long or contains line
breaking characters.
I think no need to do in this CL but probably it is good to restrict to several
length (e.g. 4) and good to remove some control characters.

[Shu Chen, 2014-09-30 02:52:56]

https://codereview.chromium.org/608183002/diff/40001/chromeos/ime/input_metho...
File chromeos/ime/input_method_descriptor.cc (right):

https://codereview.chromium.org/608183002/diff/40001/chromeos/ime/input_metho...
chromeos/ime/input_method_descriptor.cc:52: return indicator_;
On 2014/09/30 02:49:01, Seigo Nonaka wrote:
> This |indicator_| can be set by third party, right?
> 
> If so, I think the UI is not good if the indicator is too long or contains
line
> breaking characters.
> I think no need to do in this CL but probably it is good to restrict to
several
> length (e.g. 4) and good to remove some control characters.

This cl only enables the indicator for component IME extensions.
I will follow up on how it can be supported for 3rd party IME extensions in the
future.
Thanks!

[Shu Chen, 2014-09-30 03:01:27]

The CQ bit was checked by shuchen@chromium.org

[commit-bot: I haz the power, 2014-09-30 03:05:16]

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

[commit-bot: I haz the power, 2014-09-30 03:50:10]

Committed patchset #3 (id:40001) as 7af9972127cb53fec306850be6fa1d1ff99e6847

[commit-bot: I haz the power, 2014-09-30 03:51:08]

Patchset 3 (id:??) landed as
https://crrev.com/bc91aaa2ed163990f30a36b5999e722ed548977c
Cr-Commit-Position: refs/heads/master@{#297357}