MenuItemView: use disabled text color for subtitles too

MenuItemView: use disabled text color for subtitles too

Previously, the subtitle text color was
* SelectedMenuItemForegroundColor, if the menuitem is selected
* MenuItemSubtitleColor, otherwise

This CL changes the rule to be
* SelectedMenuItemForegroundColor, if the menuitem is selected
* DisabledMenuItemForegroundColor, if the menuitem is disabled
* MenuItemSubtitleColor, otherwise

Linux Aura, CrOS, and Windows all have MenuItemSubtitleColor ==
DisabledMenuItemForegroundColor and so should be unaffected.

BUG=692745
R=sky@chromium.org,erg@chromium.org

Review-Url: https://codereview.chromium.org/2700473002
Cr-Commit-Position: refs/heads/master@{#451061}
Committed: https://chromium.googlesource.com/chromium/src/+/0e218d087a65e901c83276329540765ebd087c13

[Tom (Use chromium acct), 2017-02-15 21:32:08]

Description was changed from

==========
MenuItemView: use disabled text color for subtitles too

Previously, the subtitle text color was
* SelectedMenuItemForegroundColor, if the menuitem is selected
* MenuItemSubtitleColor, otherwise

This CL changes the rule to be
* SelectedMenuItemForegroundColor, if the menuitem is selected
* DisabledMenuItemForegroundColor, if the menuitem is disabled
* MenuItemSubtitleColor, otherwise

Linux Aura, CrOS, and Windows all have MenuItemSubtitleColor ==
DisabledMenuItemForegroundColor and so should be unaffected.

BUG=
R=sky@chromium.org
==========

to

==========
MenuItemView: use disabled text color for subtitles too

Previously, the subtitle text color was
* SelectedMenuItemForegroundColor, if the menuitem is selected
* MenuItemSubtitleColor, otherwise

This CL changes the rule to be
* SelectedMenuItemForegroundColor, if the menuitem is selected
* DisabledMenuItemForegroundColor, if the menuitem is disabled
* MenuItemSubtitleColor, otherwise

Linux Aura, CrOS, and Windows all have MenuItemSubtitleColor ==
DisabledMenuItemForegroundColor and so should be unaffected.

BUG=692745
R=sky@chromium.org,erg@chromium.org
==========

[Tom (Use chromium acct), 2017-02-15 21:32:08]

thomasanderson@google.com changed reviewers:
+ erg@chromium.org

[Tom (Use chromium acct), 2017-02-15 21:33:18]

reviewers ptal

erg:
chrome/browser/ui/libgtkui/native_theme_gtk3.cc

sky:
ui/views/controls/menu/menu_item_view.h
ui/views/controls/menu/menu_item_view.cc

https://codereview.chromium.org/2700473002/diff/20001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtkui/native_theme_gtk3.cc (right):

https://codereview.chromium.org/2700473002/diff/20001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtkui/native_theme_gtk3.cc:116: return
GetFgColor("GtkMenu#menu GtkMenuItem#menuitem #accelerator");
This was actually fine before, but this is more technically correct

[Tom (Use chromium acct), 2017-02-15 21:34:22]

Patchset #1 (id:1) has been deleted

[Elliot Glaysher, 2017-02-15 21:48:19]

native theme lgtm

https://codereview.chromium.org/2700473002/diff/20001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtkui/native_theme_gtk3.cc (right):

https://codereview.chromium.org/2700473002/diff/20001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtkui/native_theme_gtk3.cc:116: return
GetFgColor("GtkMenu#menu GtkMenuItem#menuitem #accelerator");
On 2017/02/15 21:33:18, Tom Anderson wrote:
> This was actually fine before, but this is more technically correct

The best kind of correct!

[sky, 2017-02-15 23:32:41]

LGTM

[Tom (Use chromium acct), 2017-02-15 23:50:37]

The CQ bit was checked by thomasanderson@google.com

[commit-bot: I haz the power, 2017-02-15 23:51:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 01:55:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 01:55:41]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[Tom (Use chromium acct), 2017-02-16 18:41:31]

The CQ bit was checked by thomasanderson@google.com

[commit-bot: I haz the power, 2017-02-16 18:42:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 19:48:13]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487270491255280,
"parent_rev": "b4f09b9a3acecf253d000ee1eca9c84f8556b284", "commit_rev":
"0e218d087a65e901c83276329540765ebd087c13"}

[commit-bot: I haz the power, 2017-02-16 19:48:45]

Description was changed from

==========
MenuItemView: use disabled text color for subtitles too

Previously, the subtitle text color was
* SelectedMenuItemForegroundColor, if the menuitem is selected
* MenuItemSubtitleColor, otherwise

This CL changes the rule to be
* SelectedMenuItemForegroundColor, if the menuitem is selected
* DisabledMenuItemForegroundColor, if the menuitem is disabled
* MenuItemSubtitleColor, otherwise

Linux Aura, CrOS, and Windows all have MenuItemSubtitleColor ==
DisabledMenuItemForegroundColor and so should be unaffected.

BUG=692745
R=sky@chromium.org,erg@chromium.org
==========

to

==========
MenuItemView: use disabled text color for subtitles too

Previously, the subtitle text color was
* SelectedMenuItemForegroundColor, if the menuitem is selected
* MenuItemSubtitleColor, otherwise

This CL changes the rule to be
* SelectedMenuItemForegroundColor, if the menuitem is selected
* DisabledMenuItemForegroundColor, if the menuitem is disabled
* MenuItemSubtitleColor, otherwise

Linux Aura, CrOS, and Windows all have MenuItemSubtitleColor ==
DisabledMenuItemForegroundColor and so should be unaffected.

BUG=692745
R=sky@chromium.org,erg@chromium.org

Review-Url: https://codereview.chromium.org/2700473002
Cr-Commit-Position: refs/heads/master@{#451061}
Committed:
https://chromium.googlesource.com/chromium/src/+/0e218d087a65e901c83276329540...
==========

[commit-bot: I haz the power, 2017-02-16 19:48:46]

Committed patchset #1 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0e218d087a65e901c83276329540...