[Mac][Material Design] Fix bookmark chevron button focus ring location.

chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.h"
 
 #include "base/logging.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_constants.h"
+#import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.h"
+#import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_view_cocoa.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_button.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_context_menu_cocoa_controller.h"
 #include "chrome/grit/generated_resources.h"
 #import "components/bookmarks/browser/bookmark_model.h"
 #include "content/public/browser/user_metrics.h"
 #import "ui/base/cocoa/nsview_additions.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/resources/grit/ui_resources.h"
@@ skipping 21 matching lines @@
 
 @interface BookmarkButtonCell(Private)
 - (void)configureBookmarkButtonCell;
 - (void)applyTextColor;
 // Returns the title the button cell displays. Note that a button cell can
 // have a title string assigned but it won't be visible if its image position
 // is NSImageOnly.
 - (NSString*)visibleTitle;
 // Returns the dictionary of attributes to associate with the button title.
 - (NSDictionary*)titleTextAttributes;
+// Returns the BookmarkBarController for the BookmarkButtonCell's controlView.
+- (BookmarkBarController*)
+    bookmarkBarViewControllerFromControlView:(NSView*)controlView;
 @end
 
 
 @implementation BookmarkButtonCell
 
 @synthesize startingChildIndex = startingChildIndex_;
 @synthesize drawFolderArrow = drawFolderArrow_;
 
 + (id)buttonCellForNode:(const BookmarkNode*)node
                    text:(NSString*)text
@@ skipping 303 matching lines @@
   return imageRect;
 }
 
 - (CGFloat)textStartXOffset {
   if (!ui::MaterialDesignController::IsModeMaterial()) {
     return [super textStartXOffset];
   }
   return kIconLeftPadding + [[self image] size].width + kIconTextSpacer;
 }
 
+- (BookmarkBarController*)
+    bookmarkBarViewControllerFromControlView:(NSView*)controlView {

[tapted, 2016/06/16 05:53:19]

having the button cell know much about its parents, and using rtti to find it,
probably breaks some kind of rule. I think the nice way to do this is to add a
property like `isOffTheSideButton()`, and have it return YES -- either by
setting a data member in bookmark_button_cell.mm's

// Used by the off-the-side menu, the only case where a
// BookmarkButtonCell is loaded from a nib.
- (void)awakeFromNib {
  [self configureBookmarkButtonCell];
}

(somewhat hacky)

OR having -[BookmarkBarController
configureOffTheSideButtonContentsAndVisibility] set it

OR creating a new subclass, just in bookmark_button_cell.mm like:

@interface OffTheSideButtonCell : BookmarkButtonCell
@end
@implementation
- (BOOL)isOffTheSideButton {
  return YES;
}
@end

then open BookmarkBar.xib in a text editor and change
customClass="BookmarkButtonCell" on line 25 to OffTheSideButtonCell

That's essentially the approach currently used by isFolderButtonCell - the only
downside is editing the .xib by hand, but this should be safe.

[shrike, 2016/06/16 15:19:09]

Great! My original preference was to change the cell class but I didn't think of
a way to do that outside of IB.

I am also unsure why we need a 2pt shift. My understanding of how the focus ring
drawing works is that the Appkit builds some kind of path based on the graphics
commands you execute during a certain window of time. That means the focus ring
should follow the bitmap.

+  NSView* nextView = [controlView superview];
+  while (nextView) {
+    if ([nextView isKindOfClass:[BookmarkBarView class]]) {
+      BookmarkBarView* bookmarkBarView = (BookmarkBarView*)nextView;
+      return [bookmarkBarView controller];
+    }
+    nextView = [nextView superview];
+  }
+  return nil;
+}
+
 - (void)drawFocusRingMaskWithFrame:(NSRect)cellFrame
                             inView:(NSView*)controlView {
-  // In Material Design we have to move the focus ring over by 2 pts to get it
-  // to line up with the image.
-  if (ui::MaterialDesignController::IsModeMaterial() &&
-      [self visibleTitle].length > 0) {
-    cellFrame.origin.x += 2;
+  if (ui::MaterialDesignController::IsModeMaterial()) {
+    // In Material Design we have to move the focus ring over by 2 pts to get it
+    // to line up with the image.
+    if ([self visibleTitle].length > 0) {
+      cellFrame.origin.x += 2;
+    }
+
+    // We also have to nudge the chevron button's focus ring up 2pts.
+    BookmarkBarController* bookmarkBarController =
+        [self bookmarkBarViewControllerFromControlView:controlView];
+    if ([bookmarkBarController offTheSideButton] == controlView) {
+      cellFrame.origin.y -= 2;
+    }
   }
   [super drawFocusRingMaskWithFrame:cellFrame inView:controlView];
 }
 
 // Override cell drawing to add a submenu arrow like a real menu.
 - (void)drawInteriorWithFrame:(NSRect)cellFrame inView:(NSView*)controlView {
   // First draw "everything else".
   [super drawInteriorWithFrame:cellFrame inView:controlView];
 
   // If asked to do so, and if a folder, draw the arrow.
@@ skipping 32 matching lines @@
   const CGFloat kLineWidth = [controlView cr_lineWidth];
   if ([self tag] == kMaterialStandardButtonTypeWithLimitedClickFeedback &&
       ![self isFolderButtonCell] && kLineWidth < 1) {
     return -kLineWidth;
   }
   return 0.0;
 }
 
 
 @end