New issue
Advanced search Search tips

Issue 813819 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

[MacViews] Menus don't respect system appearance setting

Project Member Reported by lgrey@chromium.org, Feb 20 2018

Issue description

To repro:
System Preferences > Appearance > Graphite
Open wrench menu in MacViews browser (polychrome build with -enable-features=ViewsBrowserWindows)
Observe blue highlight (contrast with Cocoa browser wrench menu, or system menus in MacViews)
 

Comment 1 by tapted@chromium.org, Feb 21 2018

Part of this is going to be

void NativeThemeMac::PaintMenuItemBackground(
    cc::PaintCanvas* canvas,
    State state,
    const gfx::Rect& rect,
    const MenuItemExtraParams& menu_item) const {
  cc::PaintFlags flags;
  switch (state) {
    case NativeTheme::kNormal:
    case NativeTheme::kDisabled:
      // Draw nothing over the regular background.
      break;
    case NativeTheme::kHovered:
      // TODO(tapted): Draw a gradient, and use [NSColor currentControlTint] to
      // pick colors. The System color "selectedMenuItemColor" is actually still
      // blue for Graphite. And while "keyboardFocusIndicatorColor" does change,
      // and is a good shade of gray, it's not blue enough for the Blue theme.
      flags.setColor(GetSystemColor(kColorId_FocusedMenuItemBackgroundColor));
      canvas->drawRect(gfx::RectToSkRect(rect), flags);


Probably... we should still use a native menu for the wrench menu, and find a nice way to delegate the insertion of the custom NSView*s 

Long-term we may want to consider ditching NSMenu for the wrench menu completely and just making it a popup/bubble window the way most* other browsers do (.e.g similar to the profile switcher). This would allow us to:
 - enable drag&drop in the bookmarks submenu, which we currently can not do no Mac (but can on other platforms)
 - fix a11y issues with zoom/cut/copy/paste and the extension icon container
 - allow opening menus on the extension icons
 - make it pretty / add branding / promos
Labels: Target-68
Owner: sdy@chromium.org
Status: Assigned (was: Available)
MacViews triage: assigning to sdy@ targeting M-68 as part of other menu work.
Owner: ellyjo...@chromium.org

Comment 4 by gov...@chromium.org, Mar 27 2018

Labels: M-68

Comment 5 by gov...@chromium.org, Mar 29 2018

** Bulk Edit **

FYI: Starting 04/13 M68 will be in canary, M68 Dev promotion will be on 04/26.

Comment 6 by lgrey@chromium.org, Mar 30 2018

Summary: [MacViews] Menus doesn't respect system appearance setting (was: [MacViews] Wrench menu doesn't respect system appearance setting)

Comment 7 by lgrey@chromium.org, Mar 30 2018

Labels: -Pri-3 Pri-2

Comment 8 by lgrey@chromium.org, Mar 30 2018

Summary: [MacViews] Menus don't respect system appearance setting (was: [MacViews] Menus doesn't respect system appearance setting)
Status: Fixed (was: Assigned)
This is Fixed as of <https://chromium-review.googlesource.com/c/chromium/src/+/998016>, which added logic for Graphite highlights.
Labels: Needs-Feedback
Tried checking the issue on latest canary 68.0.3419.0 using Mac 10.13.1 with the below mentioned steps.
1. System Preferences > Appearance 
2. Changed Appearance to Graphite (...from Blue)
3. In chrome://flags enabled "ViewsBrowserWindows"
4. Hovered mouse over the wrench menu.
We observed grey highlighting in latest canary and also in one of the older versions(68.0.3397.0-Assuming this version as without the fix). Attaching the screen shots of both.

@ellyjones: Could you please have a look at the screen shots and let us know if we have missed anything in the process. Please help us in verifying the fix.
813819-3397.0.png
587 KB View Download
813819-3419.0.png
583 KB View Download
#10: yup, this bug got superseded by https://bugs.chromium.org/p/chromium/issues/detail?id=829467#c9 so this bug, while it was fixed in the past, is now obsolete. No need to verify.

Sign in to add a comment