diff options
author | erg@chromium.org <erg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-21 01:08:16 +0000 |
---|---|---|
committer | erg@chromium.org <erg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-21 01:08:16 +0000 |
commit | 608eac27c1382ec982729d70509e80f4a15a4459 (patch) | |
tree | 6ff719ef4dcd0493eea8d68bf6fc1240124f7a38 | |
parent | fba962db1661e8529be0ac6bfc766ca8058331e8 (diff) | |
download | chromium_src-608eac27c1382ec982729d70509e80f4a15a4459.zip chromium_src-608eac27c1382ec982729d70509e80f4a15a4459.tar.gz chromium_src-608eac27c1382ec982729d70509e80f4a15a4459.tar.bz2 |
linux_aura: Don't inject black in WrenchMenu::GetForegroundColor().
Through a chain of calls, WrenchMenu::RecentTabsMenuModelDelegate hard
coded the color black into menus, even when we were using light on dark
system themes. This was done to bold specific menu labels. Separate out
a specific bold disabled menu item color, thread this through the
NativeTheme interface and add an accessor on WrenchMenu to decide
whether to use it.
Adds TODOs in the MenuDelegate interface to remove the
Get{Foreground,Background}Color methods. These interfaces can't be used
safely on the desktop. They can't be removed yet, though, because of a
few remaining calls in ash/ that I don't understand.
BUG=351307
Review URL: https://codereview.chromium.org/205153003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258485 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc | 1 | ||||
-rw-r--r-- | chrome/browser/ui/views/toolbar/wrench_menu.cc | 23 | ||||
-rw-r--r-- | chrome/browser/ui/views/toolbar/wrench_menu.h | 7 | ||||
-rw-r--r-- | ui/native_theme/common_theme.cc | 3 | ||||
-rw-r--r-- | ui/native_theme/fallback_theme.cc | 2 | ||||
-rw-r--r-- | ui/native_theme/native_theme.h | 1 | ||||
-rw-r--r-- | ui/native_theme/native_theme_win.cc | 2 | ||||
-rw-r--r-- | ui/views/controls/menu/menu_delegate.cc | 5 | ||||
-rw-r--r-- | ui/views/controls/menu/menu_delegate.h | 10 | ||||
-rw-r--r-- | ui/views/controls/menu/menu_item_view.cc | 10 |
10 files changed, 44 insertions, 20 deletions
diff --git a/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc b/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc index 346f253..4ba6d720 100644 --- a/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc +++ b/chrome/browser/ui/libgtk2ui/native_theme_gtk2.cc @@ -118,6 +118,7 @@ GdkColor NativeThemeGtk2::GetSystemGdkColor(ColorId color_id) const { // MenuItem case kColorId_EnabledMenuItemForegroundColor: + case kColorId_DisabledEmphasizedMenuItemForegroundColor: return GetMenuItemStyle()->text[GTK_STATE_NORMAL]; case kColorId_DisabledMenuItemForegroundColor: return GetMenuItemStyle()->text[GTK_STATE_INSENSITIVE]; diff --git a/chrome/browser/ui/views/toolbar/wrench_menu.cc b/chrome/browser/ui/views/toolbar/wrench_menu.cc index 8a0e57e..68a877c 100644 --- a/chrome/browser/ui/views/toolbar/wrench_menu.cc +++ b/chrome/browser/ui/views/toolbar/wrench_menu.cc @@ -848,15 +848,10 @@ class WrenchMenu::RecentTabsMenuModelDelegate : public ui::MenuModelDelegate { return model_->GetLabelFontListAt(index); } - bool GetForegroundColorAt(int index, - bool is_hovered, - SkColor* override_color) const { - // The items for which we get a font list, should be shown in black. - if (GetLabelFontListAt(index)) { - *override_color = SK_ColorBLACK; - return true; - } - return false; + bool GetShouldUseDisabledEmphasizedForegroundColor(int index) const { + // The items for which we get a font list, should be shown in the bolded + // color. + return GetLabelFontListAt(index) ? true : false; } // ui::MenuModelDelegate implementation: @@ -1003,12 +998,12 @@ const gfx::FontList* WrenchMenu::GetLabelFontList(int command_id) const { return NULL; } -bool WrenchMenu::GetForegroundColor(int command_id, - bool is_hovered, - SkColor* override_color) const { +bool WrenchMenu::GetShouldUseDisabledEmphasizedForegroundColor( + int command_id) const { if (IsRecentTabsCommand(command_id)) { - return recent_tabs_menu_model_delegate_->GetForegroundColorAt( - ModelIndexFromCommandId(command_id), is_hovered, override_color); + return recent_tabs_menu_model_delegate_-> + GetShouldUseDisabledEmphasizedForegroundColor( + ModelIndexFromCommandId(command_id)); } return false; } diff --git a/chrome/browser/ui/views/toolbar/wrench_menu.h b/chrome/browser/ui/views/toolbar/wrench_menu.h index 32da47b..9795156 100644 --- a/chrome/browser/ui/views/toolbar/wrench_menu.h +++ b/chrome/browser/ui/views/toolbar/wrench_menu.h @@ -60,11 +60,10 @@ class WrenchMenu : public views::MenuDelegate, // MenuDelegate overrides: virtual const gfx::FontList* GetLabelFontList(int command_id) const OVERRIDE; - virtual bool GetForegroundColor(int command_id, - bool is_hovered, - SkColor* override_color) const OVERRIDE; + virtual bool GetShouldUseDisabledEmphasizedForegroundColor( + int command_id) const OVERRIDE; virtual base::string16 GetTooltipText(int command_id, - const gfx::Point& p) const OVERRIDE; + const gfx::Point& p) const OVERRIDE; virtual bool IsTriggerableEvent(views::MenuItemView* menu, const ui::Event& e) OVERRIDE; virtual bool GetDropFormats( diff --git a/ui/native_theme/common_theme.cc b/ui/native_theme/common_theme.cc index 5442a07..0e6b76b 100644 --- a/ui/native_theme/common_theme.cc +++ b/ui/native_theme/common_theme.cc @@ -70,6 +70,9 @@ bool CommonThemeGetSystemColor(NativeTheme::ColorId color_id, SkColor* color) { case NativeTheme::kColorId_DisabledMenuItemForegroundColor: *color = kDisabledMenuItemForegroundColor; break; + case NativeTheme::kColorId_DisabledEmphasizedMenuItemForegroundColor: + *color = SK_ColorBLACK; + break; case NativeTheme::kColorId_SelectedMenuItemForegroundColor: *color = SK_ColorWHITE; break; diff --git a/ui/native_theme/fallback_theme.cc b/ui/native_theme/fallback_theme.cc index 6eb6864..11cac35 100644 --- a/ui/native_theme/fallback_theme.cc +++ b/ui/native_theme/fallback_theme.cc @@ -117,6 +117,8 @@ SkColor FallbackTheme::GetSystemColor(ColorId color_id) const { return kEnabledMenuItemForegroundColor; case kColorId_DisabledMenuItemForegroundColor: return kDisabledMenuItemForegroundColor; + case kColorId_DisabledEmphasizedMenuItemForegroundColor: + return SK_ColorBLACK; case kColorId_SelectedMenuItemForegroundColor: return kEnabledMenuItemForegroundColor; case kColorId_FocusedMenuItemBackgroundColor: diff --git a/ui/native_theme/native_theme.h b/ui/native_theme/native_theme.h index f8296ec..aed39ad 100644 --- a/ui/native_theme/native_theme.h +++ b/ui/native_theme/native_theme.h @@ -243,6 +243,7 @@ class NATIVE_THEME_EXPORT NativeTheme { // MenuItem kColorId_EnabledMenuItemForegroundColor, kColorId_DisabledMenuItemForegroundColor, + kColorId_DisabledEmphasizedMenuItemForegroundColor, kColorId_SelectedMenuItemForegroundColor, kColorId_FocusedMenuItemBackgroundColor, kColorId_HoverMenuItemBackgroundColor, diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc index 6eab39c..69edbb9 100644 --- a/ui/native_theme/native_theme_win.cc +++ b/ui/native_theme/native_theme_win.cc @@ -538,6 +538,8 @@ SkColor NativeThemeWin::GetSystemColor(ColorId color_id) const { return kEnabledMenuItemForegroundColor; case kColorId_DisabledMenuItemForegroundColor: return kDisabledMenuItemForegroundColor; + case kColorId_DisabledEmphasizedMenuItemForegroundColor: + return SK_ColorBLACK; case kColorId_FocusedMenuItemBackgroundColor: return kFocusedMenuItemBackgroundColor; case kColorId_MenuSeparatorColor: diff --git a/ui/views/controls/menu/menu_delegate.cc b/ui/views/controls/menu/menu_delegate.cc index 920fd53..699377f 100644 --- a/ui/views/controls/menu/menu_delegate.cc +++ b/ui/views/controls/menu/menu_delegate.cc @@ -21,6 +21,11 @@ const gfx::FontList* MenuDelegate::GetLabelFontList(int id) const { return NULL; } +bool MenuDelegate::GetShouldUseDisabledEmphasizedForegroundColor( + int command_id) const { + return false; +} + bool MenuDelegate::GetBackgroundColor(int command_id, bool is_hovered, SkColor* override_color) const { diff --git a/ui/views/controls/menu/menu_delegate.h b/ui/views/controls/menu/menu_delegate.h index e50ec8e..37c2db80 100644 --- a/ui/views/controls/menu/menu_delegate.h +++ b/ui/views/controls/menu/menu_delegate.h @@ -66,9 +66,16 @@ class VIEWS_EXPORT MenuDelegate { // The font for the menu item label. virtual const gfx::FontList* GetLabelFontList(int id) const; + // Whether this item should be displayed with a bolder color when disabled. + virtual bool GetShouldUseDisabledEmphasizedForegroundColor( + int command_id) const; + // Override the text color of a given menu item dependent on the // |command_id| and its |is_hovered| state. Returns true if it chooses to // override the color. + // + // TODO(erg): Remove this interface. Injecting raw colors into the menu + // circumvents the NativeTheme. virtual bool GetForegroundColor(int command_id, bool is_hovered, SkColor* override_color) const; @@ -76,6 +83,9 @@ class VIEWS_EXPORT MenuDelegate { // Override the background color of a given menu item dependent on the // |command_id| and its |is_hovered| state. Returns true if it chooses to // override the color. + // + // TODO(erg): Remove this interface. Injecting raw colors into the menu + // circumvents the NativeTheme. virtual bool GetBackgroundColor(int command_id, bool is_hovered, SkColor* override_color) const; diff --git a/ui/views/controls/menu/menu_item_view.cc b/ui/views/controls/menu/menu_item_view.cc index e619ee7..386a23e 100644 --- a/ui/views/controls/menu/menu_item_view.cc +++ b/ui/views/controls/menu/menu_item_view.cc @@ -792,12 +792,18 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) { } // Render the foreground. - ui::NativeTheme::ColorId color_id = - ui::NativeTheme::kColorId_DisabledMenuItemForegroundColor; + ui::NativeTheme::ColorId color_id; if (enabled()) { color_id = render_selection ? ui::NativeTheme::kColorId_SelectedMenuItemForegroundColor: ui::NativeTheme::kColorId_EnabledMenuItemForegroundColor; + } else { + bool emphasized = delegate && + delegate->GetShouldUseDisabledEmphasizedForegroundColor( + GetCommand()); + color_id = emphasized ? + ui::NativeTheme::kColorId_DisabledEmphasizedMenuItemForegroundColor : + ui::NativeTheme::kColorId_DisabledMenuItemForegroundColor; } SkColor fg_color = native_theme->GetSystemColor(color_id); SkColor override_foreground_color; |