Fix label color in TextButtonBase

Fix label color in TextButtonBase

TextButtonBase derived classes like radiobutton or checbox
have the wrong color (blue) for their labels, the problem
is the hardcoded values in native_theme_win.cc

In this change I also consolidated to a single place where
the colors are set for TextButtonBase. 


BUG=165615
TEST=see bug
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=172926

[cpu_(ooo_6.6-7.5), 2012-12-12 03:42:12]

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
File ui/views/controls/button/text_button.cc (left):

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:544: color_highlight_ =
theme->GetSystemColor(
highlight_ is only used on windows and only in PB_FOR_DRAG cases which we don't
seem to be hitting. So I left this alone.

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:549:
ui::NativeTheme::kColorId_TextButtonHoverColor);
this seems correct, on hover the text becomes blue as the button itself. So I
left it alone.

[sky, 2012-12-12 04:28:36]

You sure this doesn't change colors in bad ways on chromeos or windows?

[sky, 2012-12-12 15:43:48]

Adding Mike as I know he has been looking at picking up system colors recently.

[msw, 2012-12-12 19:38:16]

I'm in favor of buttons using COLOR_BTNTEXT and COLOR_GRAYTEXT by default
instead of custom values. Can you do the same for LabelButton and cleanup
NativeTheme[Win|Aura]? I'm happy to help out if you want.

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
File ui/views/controls/button/text_button.cc (right):

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:210: color_(0),
nit: Skip initializing these values to 0.

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:533:
ui::NativeTheme::kColorId_LabelEnabledColor);
Repurpose kColorId_TextButtonEnabledColor to get COLOR_BTNTEXT on Win (instead
of this, which yields COLOR_WINDOWTEXT).

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:537:
ui::NativeTheme::kColorId_LabelDisabledColor);
Using kColorId_LabelDisabledColor (COLOR_GRAYTEXT) seems fine, but then I'd make
the same change in LabelButton and trash the old ID.
Otherwise, you could make kColorId_TextButtonDisabledColor use COLOR_GRAYTEXT
(and possibly tweak Aura's corresponding value?).

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:792:
set_color_enabled(skia::COLORREFToSkColor(GetSysColor(COLOR_BTNTEXT)));
Use the theme here to get COLOR_BTNTEXT and COLOR_GRAYTEXT indirectly.

[cpu_(ooo_6.6-7.5), 2012-12-12 22:53:25]

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
File ui/views/controls/button/text_button.cc (right):

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:210: color_(0),
On 2012/12/12 19:38:16, msw wrote:
> nit: Skip initializing these values to 0.

Done.

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:533:
ui::NativeTheme::kColorId_LabelEnabledColor);
On 2012/12/12 19:38:16, msw wrote:
> Repurpose kColorId_TextButtonEnabledColor to get COLOR_BTNTEXT on Win (instead
> of this, which yields COLOR_WINDOWTEXT).

Done.

https://codereview.chromium.org/11558006/diff/1/ui/views/controls/button/text...
ui/views/controls/button/text_button.cc:537:
ui::NativeTheme::kColorId_LabelDisabledColor);
On 2012/12/12 19:38:16, msw wrote:
> Using kColorId_LabelDisabledColor (COLOR_GRAYTEXT) seems fine, but then I'd
make
> the same change in LabelButton and trash the old ID.
> Otherwise, you could make kColorId_TextButtonDisabledColor use COLOR_GRAYTEXT
> (and possibly tweak Aura's corresponding value?).

Done.

[cpu_(ooo_6.6-7.5), 2012-12-12 22:54:39]

ok, new approach ready, please take a look.

[msw, 2012-12-12 23:45:45]

LGTM with nits.

https://codereview.chromium.org/11558006/diff/11003/ui/native_theme/native_th...
File ui/native_theme/native_theme_win.cc (right):

https://codereview.chromium.org/11558006/diff/11003/ui/native_theme/native_th...
ui/native_theme/native_theme_win.cc:62: COLOR_BTNTEXT,
nit: alphabetize.

https://codereview.chromium.org/11558006/diff/11003/ui/views/controls/button/...
File ui/views/controls/button/text_button.cc (right):

https://codereview.chromium.org/11558006/diff/11003/ui/views/controls/button/...
ui/views/controls/button/text_button.cc:215: // OnNativeThemeChanged sets the
'color' member variables.
nit: nix quotes around color.

https://codereview.chromium.org/11558006/diff/11003/ui/views/controls/button/...
ui/views/controls/button/text_button.cc:216:
OnNativeThemeChanged(ui::NativeTheme::instance());
nit: use GetNativeTheme() instead of ui::NativeTheme::instance().

[cpu_(ooo_6.6-7.5), 2012-12-12 23:55:05]

changes done. sky rubber-stamp ?

[sky, 2012-12-13 00:54:26]

https://codereview.chromium.org/11558006/diff/12003/ui/views/controls/button/...
File ui/views/controls/button/text_button.cc (right):

https://codereview.chromium.org/11558006/diff/12003/ui/views/controls/button/...
ui/views/controls/button/text_button.cc:216:
OnNativeThemeChanged(GetNativeTheme());
Change this so that you aren't invoking a virtual method from the constructor.

[cpu_(ooo_6.6-7.5), 2012-12-13 01:30:36]

change done. please take a look again.

[sky, 2012-12-13 17:25:55]

LGTM