Allow non-radio button properties in uber tray. Simplified chinese will use it.

Allow non-radio button properties in uber tray. Simplified chinese will use it.


BUG=122318
TEST=manually


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

[Yusuke Sato, 2012-04-06 09:31:42]

http://codereview.chromium.org/10008043/diff/1/chrome/browser/chromeos/system...
File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):

http://codereview.chromium.org/10008043/diff/1/chrome/browser/chromeos/system...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:392: key, true);
I think the old status area implementation sends false for "command" items to
follow the IBus protocol. Did this CL work with ibus-pinyin?

[Jun Mukai, 2012-04-09 01:56:23]

http://codereview.chromium.org/10008043/diff/1/chrome/browser/chromeos/system...
File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):

http://codereview.chromium.org/10008043/diff/1/chrome/browser/chromeos/system...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:392: key, true);
On 2012/04/06 09:31:42, Yusuke Sato wrote:
> I think the old status area implementation sends false for "command" items to
> follow the IBus protocol. Did this CL work with ibus-pinyin?
> 

You're right.  Fixed the code around here.

[Yusuke Sato, 2012-04-09 02:04:56]

lgtm

On 2012/04/09 01:56:23, Jun Mukai wrote:
>
http://codereview.chromium.org/10008043/diff/1/chrome/browser/chromeos/system...
> File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):
> 
>
http://codereview.chromium.org/10008043/diff/1/chrome/browser/chromeos/system...
> chrome/browser/chromeos/system/ash_system_tray_delegate.cc:392: key, true);
> On 2012/04/06 09:31:42, Yusuke Sato wrote:
> > I think the old status area implementation sends false for "command" items
to
> > follow the IBus protocol. Did this CL work with ibus-pinyin?
> > 
> 
> You're right.  Fixed the code around here.

[Jun Mukai, 2012-04-09 02:07:03]

Add sky@ because now it contains ash/shell.cc

sky, could you take a look at this CL?

[sky, 2012-04-09 15:18:56]

http://codereview.chromium.org/10008043/diff/3001/ash/shell.cc
File ash/shell.cc (right):

http://codereview.chromium.org/10008043/diff/3001/ash/shell.cc#newcode381
ash/shell.cc:381: const std::string& key, bool is_selection) OVERRIDE {
nit: when you wrap, each param on its own line.

http://codereview.chromium.org/10008043/diff/3001/ash/system/ime/tray_ime.cc
File ash/system/ime/tray_ime.cc (right):

http://codereview.chromium.org/10008043/diff/3001/ash/system/ime/tray_ime.cc#...
ash/system/ime/tray_ime.cc:172: std::map<views::View*, std::pair<std::string,
bool> > property_map_;
Document what the pair is.

http://codereview.chromium.org/10008043/diff/3001/ash/system/tray/system_tray.cc
File ash/system/tray/system_tray.cc (right):

http://codereview.chromium.org/10008043/diff/3001/ash/system/tray/system_tray...
ash/system/tray/system_tray.cc:419: : selected(false), is_selection(false) {
wrap is_selection to the next line.

[Jun Mukai, 2012-04-10 02:13:46]

http://codereview.chromium.org/10008043/diff/3001/ash/shell.cc
File ash/shell.cc (right):

http://codereview.chromium.org/10008043/diff/3001/ash/shell.cc#newcode381
ash/shell.cc:381: const std::string& key, bool is_selection) OVERRIDE {
On 2012/04/09 15:18:56, sky wrote:
> nit: when you wrap, each param on its own line.

Done.

http://codereview.chromium.org/10008043/diff/3001/ash/system/ime/tray_ime.cc
File ash/system/ime/tray_ime.cc (right):

http://codereview.chromium.org/10008043/diff/3001/ash/system/ime/tray_ime.cc#...
ash/system/ime/tray_ime.cc:172: std::map<views::View*, std::pair<std::string,
bool> > property_map_;
On 2012/04/09 15:18:56, sky wrote:
> Document what the pair is.

Use IMEPropertyInfo struct instead.

http://codereview.chromium.org/10008043/diff/3001/ash/system/tray/system_tray.cc
File ash/system/tray/system_tray.cc (right):

http://codereview.chromium.org/10008043/diff/3001/ash/system/tray/system_tray...
ash/system/tray/system_tray.cc:419: : selected(false), is_selection(false) {
On 2012/04/09 15:18:56, sky wrote:
> wrap is_selection to the next line.

Done.

[sky, 2012-04-10 16:19:43]

LGTM, but wait for Sadrul to review too.

[sadrul, 2012-04-10 16:31:12]

LGTM.

I think the doc for SetImePropertyActivated is rather confusing, because looking
at the code, and the doc of the function, it appears that a property that is not
|is_selection_item| cannot be activated. But apparently that is not the case.
Perhaps the doc for SetImePropertyActivated needs to be updated/fixed (feel free
to do that in a separate CL)?

[commit-bot: I haz the power, 2012-04-10 16:33:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/10008043/10001

[Yusuke Sato, 2012-04-10 16:41:37]

sadrul: I'll do that.

[commit-bot: I haz the power, 2012-04-10 18:01:16]

Change committed as 131583