Native themes: Add MenuItemSubtitleColor

Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

Committed: https://crrev.com/5826565d029a694c6e8ca9db19c067c9895ec77d
Cr-Commit-Position: refs/heads/master@{#439854}

[Tom (Use chromium acct), 2016-12-16 22:45:18]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[Tom (Use chromium acct), 2016-12-16 22:45:42]

The CQ bit was unchecked by thomasanderson@google.com

[Tom (Use chromium acct), 2016-12-16 22:47:42]

Patchset #1 (id:1) has been deleted

[Tom (Use chromium acct), 2016-12-16 22:48:41]

Description was changed from

==========
Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

to

==========
Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

[Tom (Use chromium acct), 2016-12-16 22:49:28]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-16 22:49:59]

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

[Tom (Use chromium acct), 2016-12-16 22:50:21]

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

[Tom (Use chromium acct), 2016-12-16 22:50:21]

erg@ please review native_theme_gtk[2,3].cc
sky@ please review all other files

[sky, 2016-12-16 23:30:39]

sky@chromium.org changed reviewers:
+ estade@chromium.org

[sky, 2016-12-16 23:30:39]

estade for ui/native_theme
ui/views LGTM

[Elliot Glaysher, 2016-12-16 23:33:41]

native theme lgtm (but i'm not an owner)

[Elliot Glaysher, 2016-12-16 23:34:37]

On 2016/12/16 23:33:41, Elliot Glaysher wrote:
> native theme lgtm (but i'm not an owner)

sorry, you were asking for a review of the c/b/u/libgtk native_theme. that
lgtm2.

[commit-bot: I haz the power, 2016-12-17 00:24:20]

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

[commit-bot: I haz the power, 2016-12-17 00:24:21]

Dry run: This issue passed the CQ dry run.

[Tom (Use chromium acct), 2016-12-20 00:33:29]

The CQ bit was checked by thomasanderson@google.com

[commit-bot: I haz the power, 2016-12-20 00:34:16]

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

[Evan Stade, 2016-12-20 00:39:22]

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/common_...
File ui/native_theme/common_theme.cc (right):

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/common_...
ui/native_theme/common_theme.cc:187: return kDisabledMenuItemForegroundColor;
if you make this

return
base_theme->GetSystemColor(NativeTheme::kDisabledMenuItemForegroundColor);

then you don't need to update mac

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/native_...
File ui/native_theme/native_theme_dark_aura.cc (right):

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/native_...
ui/native_theme/native_theme_dark_aura.cc:111: case
kColorId_MenuItemSubtitleColor:
nit: match order in enum declaration

[Tom (Use chromium acct), 2016-12-20 00:51:39]

The CQ bit was unchecked by thomasanderson@google.com

[Tom (Use chromium acct), 2016-12-20 01:03:19]

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/common_...
File ui/native_theme/common_theme.cc (right):

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/common_...
ui/native_theme/common_theme.cc:187: return kDisabledMenuItemForegroundColor;
On 2016/12/20 00:39:22, Evan Stade wrote:
> if you make this
> 
> return
> base_theme->GetSystemColor(NativeTheme::kDisabledMenuItemForegroundColor);
> 
> then you don't need to update mac

Done.

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/native_...
File ui/native_theme/native_theme_dark_aura.cc (right):

https://codereview.chromium.org/2575343005/diff/20001/ui/native_theme/native_...
ui/native_theme/native_theme_dark_aura.cc:111: case
kColorId_MenuItemSubtitleColor:
On 2016/12/20 00:39:22, Evan Stade wrote:
> nit: match order in enum declaration

Done.

[Tom (Use chromium acct), 2016-12-20 01:03:35]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 01:04:12]

Dry run: 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, 2016-12-20 02:05:23]

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

[commit-bot: I haz the power, 2016-12-20 02:05:24]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-12-20 18:31:11]

lgtm

[Tom (Use chromium acct), 2016-12-20 18:42:43]

The CQ bit was checked by thomasanderson@google.com

[Tom (Use chromium acct), 2016-12-20 18:42:43]

The patchset sent to the CQ was uploaded after l-g-t-m from erg@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2575343005/#ps40001
(title: "Address estade@'s comments")

[commit-bot: I haz the power, 2016-12-20 18:43:02]

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, 2016-12-20 18:49:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482259363152010,
"parent_rev": "c1a3ccb72af55af8b45f82fbb52598b90e3c3bbf", "commit_rev":
"e3a425fdd26a8e5d131e89c5698ea808d8dbcd1b"}

[commit-bot: I haz the power, 2016-12-20 18:49:47]

Description was changed from

==========
Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

to

==========
Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

Review-Url: https://codereview.chromium.org/2575343005
==========

[commit-bot: I haz the power, 2016-12-20 18:49:48]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-12-20 18:51:28]

Description was changed from

==========
Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

Review-Url: https://codereview.chromium.org/2575343005
==========

to

==========
Native themes: Add MenuItemSubtitleColor

MenuItemView was using ButtonDisabledColor for coloring menuitem
subtext (usually used for accelerator shortcuts).  However, Adwaita on
Gtk3.0 uses the same color for disabled button text as for menu
backgrounds, meaning menuitem subtext appeared invisible.  This CL
adds MenuItemSubtitleColor to fix this issue.

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

Committed: https://crrev.com/5826565d029a694c6e8ca9db19c067c9895ec77d
Cr-Commit-Position: refs/heads/master@{#439854}
==========

[commit-bot: I haz the power, 2016-12-20 18:51:29]

Patchset 2 (id:??) landed as
https://crrev.com/5826565d029a694c6e8ca9db19c067c9895ec77d
Cr-Commit-Position: refs/heads/master@{#439854}