[Mac] Bookmark button cell updates

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/metrics/user_metrics.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_constants.h"
@@ skipping 17 matching lines @@
 
 // Padding on the left side of the arrow icon.
 const int kHierarchyButtonLeftPadding = 11;
 
 const int kIconTextSpacer = 4;
 const int kTextRightPadding = 4;
 const int kIconLeftPadding = 4;
 
 const int kDefaultFontSize = 12;
 
+// Kerning value for the title text.
+const CGFloat kKernAmount = 0.2;

[Elly Fong-Jones, 2017/04/04 15:04:36]

it seems weird that this has to be specified manually - is there a systemwide
default we should use?

[lgrey, 2017/04/04 15:12:27]

I wondered what was going on here too. My understanding is:
|titleTextAttributes| (which is an instance method that previously had this as a
magic number) is used
to create an attributed string, which is what ends up getting drawn. To put it
another way, this value
determines the kerning, vs. just being used for size lookup.

Re: a systemwide default, my assumption is that bookmark buttons having a
specific value is
intentional. Is it worth checking in with UX?

[Elly Fong-Jones, 2017/04/05 11:26:37]

Well, what's the systemwide default, and how do they look with the systemwide
default vs the current value? We can check with UX, but they will ask us for
side-by-side screenshots.

[lgrey, 2017/04/05 15:21:56]

The docs say the default it "kerning is disabled", by which I'm guessing they
mean "we use the font's default kern tables without further adjustment"

Kern = 0.2 (current behavior) https://screenshot.googleplex.com/WXXF3iC6SoT
Kerning unspecified https://screenshot.googleplex.com/4qBEEmG8mn0

+
 };  // namespace
 
 // TODO(lgrey): Bake setting the chevron image into this
 // class instead of setting it externally.
 @interface OffTheSideButtonCell : BookmarkButtonCell
 
 - (NSString*)accessibilityTitle;
 
 @end
 @implementation OffTheSideButtonCell
@@ skipping 10 matching lines @@
   NSRect imageRect = [super imageRectForBounds:theRect];
   // Make sure the chevron icon stays centered. Normally a bookmark bar item
   // with no label has its icon placed at a fixed x-position.
   CGFloat totalWidth = NSMaxX(theRect);
   imageRect.origin.x = (totalWidth - [self image].size.width) / 2;
   return imageRect;
 }
 
 @end
 
-@interface BookmarkButtonCell(Private)
+@interface BookmarkButtonCell (Private)
 // Returns YES if the cell is the offTheSide button cell.
 - (BOOL)isOffTheSideButtonCell;
 - (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;
@@ skipping 26 matching lines @@
                                           image:image
                                  menuController:menuController]
        autorelease];
   return buttonCell;
 }
 
 + (id)offTheSideButtonCell {
   return [[[OffTheSideButtonCell alloc] init] autorelease];
 }
 
++ (CGFloat)cellWidthForNode:(const bookmarks::BookmarkNode*)node
+                      image:(NSImage*)image {
+  NSString* title =
+      [self cleanTitle:base::SysUTF16ToNSString(node->GetTitle())];
+  CGFloat width = kIconLeftPadding + [image size].width;
+  if ([title length] > 0) {
+    CGSize titleSize = [title sizeWithAttributes:@{
+      NSParagraphStyleAttributeName : [self paragraphStyleForBookmarkBarCell],
+      NSKernAttributeName : @(kKernAmount),
+      NSFontAttributeName : [self fontForBookmarkBarCell],
+    }];
+    width += kIconTextSpacer + std::ceil(titleSize.width) + kTextRightPadding;
+  } else {
+    width += kIconLeftPadding;
+  }
+  return width;
+}
+
 - (id)initForNode:(const BookmarkNode*)node
              text:(NSString*)text
             image:(NSImage*)image
    menuController:(BookmarkContextMenuCocoaController*)menuController {
   if ((self = [super initTextCell:text])) {
     menuController_ = menuController;
     [self configureBookmarkButtonCell];
     [self setTextColor:[NSColor blackColor]];
-    [self setBookmarkNode:node];
+    [self setBookmarkNode:node image:image];
     // When opening a bookmark folder, the default behavior is that the
     // favicon is greyed when menu item is hovered with the mouse cursor.
     // When using NSNoCellMask, the favicon won't be greyed when menu item
     // is hovered.
     // In the bookmark bar, the favicon is not greyed when the bookmark is
     // hovered with the mouse cursor.
     // It makes the behavior of the bookmark folder consistent with hovering
     // on the bookmark bar.
     [self setHighlightsBy:NSNoCellMask];
-
-    if (node) {
-      NSString* title = base::SysUTF16ToNSString(node->GetTitle());
-      [self setBookmarkCellText:title image:image];
-    } else {
-      [self setEmpty:YES];
-      [self setBookmarkCellText:l10n_util::GetNSString(IDS_MENU_EMPTY_SUBMENU)
-                          image:nil];
-    }
   }
 
   return self;
 }
 
 - (id)initWithText:(NSString*)text
              image:(NSImage*)image
     menuController:(BookmarkContextMenuCocoaController*)menuController {
   if ((self = [super initTextCell:text])) {
     menuController_ = menuController;
@@ skipping 26 matching lines @@
 - (BOOL)isOffTheSideButtonCell {
   return NO;
 }
 
 // Perform all normal init routines specific to the BookmarkButtonCell.
 - (void)configureBookmarkButtonCell {
   [self setButtonType:NSMomentaryPushInButton];
   [self setShowsBorderOnlyWhileMouseInside:YES];
   [self setControlSize:NSSmallControlSize];
   [self setAlignment:NSLeftTextAlignment];
-  [self setFont:[NSFont systemFontOfSize:kDefaultFontSize]];
+  [self setFont:[[self class] fontForBookmarkBarCell]];
   [self setBordered:NO];
   [self setBezeled:NO];
   [self setWraps:NO];
   // NSLineBreakByTruncatingMiddle seems more common on OSX but let's
   // try to match Windows for a bit to see what happens.
   [self setLineBreakMode:NSLineBreakByTruncatingTail];
 
   // The overflow button chevron bitmap is not 16 units high, so it'd be scaled
   // at paint time without this.
   [self setImageScaling:NSImageScaleNone];
@@ skipping 14 matching lines @@
 - (NSSize)cellSizeForBounds:(NSRect)aRect {
   // There's no bezel or border so return cellSize.
   NSSize size = [self cellSize];
   size.width = std::min(aRect.size.width, size.width);
   size.height = std::min(aRect.size.height, size.height);
   return size;
 }
 
 - (void)setBookmarkCellText:(NSString*)title
                       image:(NSImage*)image {
-  title = [title stringByReplacingOccurrencesOfString:@"\n"
-                                           withString:@" "];
-  title = [title stringByReplacingOccurrencesOfString:@"\r"
-                                           withString:@" "];
-
-  if ([title length]) {
+  title = [[self class] cleanTitle:title];
+  if ([title length] && ![self isOffTheSideButtonCell]) {
     [self setImagePosition:NSImageLeft];
     [self setTitle:title];
   } else if ([self isFolderButtonCell]) {
     // Left-align icons for bookmarks within folders, regardless of whether
     // there is a title.
     [self setImagePosition:NSImageLeft];
   } else {
     // For bookmarks without a title that aren't visible directly in the
     // bookmarks bar, squeeze things tighter by displaying only the image.
     // By default, Cocoa leaves extra space in an attempt to display an
     // empty title.
     [self setImagePosition:NSImageOnly];
   }
 
   if (image)
     [self setImage:image];
 }
 
 - (void)setBookmarkNode:(const BookmarkNode*)node {
+  [self setBookmarkNode:node image:nil];
+}
+
+- (void)setBookmarkNode:(const BookmarkNode*)node image:(NSImage*)image {
   [self setRepresentedObject:[NSValue valueWithPointer:node]];
+  if (node) {
+    NSString* title = base::SysUTF16ToNSString(node->GetTitle());
+    [self setBookmarkCellText:title image:image];
+  } else {
+    [self setEmpty:YES];
+    [self setBookmarkCellText:l10n_util::GetNSString(IDS_MENU_EMPTY_SUBMENU)
+                        image:nil];
+  }
 }
 
 - (const BookmarkNode*)bookmarkNode {
   return static_cast<const BookmarkNode*>([[self representedObject]
                                             pointerValue]);
 }
 
 - (NSMenu*)menu {
   // If node is NULL, this is a custom button, the menu does not represent
   // anything.
@@ skipping 49 matching lines @@
 - (void)setDrawFolderArrow:(BOOL)draw {
   drawFolderArrow_ = draw;
   if (draw && !arrowImage_) {
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
     arrowImage_.reset(
         [rb.GetNativeImageNamed(IDR_MENU_HIERARCHY_ARROW).ToNSImage() retain]);
   }
 }
 
 - (NSDictionary*)titleTextAttributes {
-  base::scoped_nsobject<NSMutableParagraphStyle> style(
-      [NSMutableParagraphStyle new]);
-  [style setAlignment:NSNaturalTextAlignment];
-  [style setLineBreakMode:NSLineBreakByTruncatingTail];
+  NSParagraphStyle* style = [[self class] paragraphStyleForBookmarkBarCell];
   NSColor* textColor = textColor_.get();
   if (!textColor) {
     textColor = [NSColor blackColor];
   }
   if (![self isEnabled]) {
     textColor = [textColor colorWithAlphaComponent:0.5];
   }
   NSFont* theFont = [self font];
   if (!theFont) {
-    theFont = [NSFont systemFontOfSize:kDefaultFontSize];
+    theFont = [[self class] fontForBookmarkBarCell];
   }
 
   return @{
-     NSFontAttributeName : theFont,
-     NSForegroundColorAttributeName : textColor,
-     NSParagraphStyleAttributeName : style.get(),
-     NSKernAttributeName : [NSNumber numberWithFloat:0.2]
+    NSFontAttributeName : theFont,
+    NSForegroundColorAttributeName : textColor,
+    NSParagraphStyleAttributeName : style,
+    NSKernAttributeName : @(kKernAmount),
   };
 }
 
 - (NSString*)visibleTitle {
   return [self imagePosition] != NSImageOnly ? [self title] : @"";
 }
 
 // Add extra size for the arrow so it doesn't overlap the text.
 // Does not sanity check to be sure this is actually a folder node.
 - (NSSize)cellSize {
@@ skipping 85 matching lines @@
   // In Material Design on Retina, and not in a folder menu, nudge the hover
   // background by 1px.
   const CGFloat kLineWidth = [controlView cr_lineWidth];
   if ([self isMaterialDesignButtonType] && ![self isFolderButtonCell] &&
       kLineWidth < 1) {
     return -kLineWidth;
   }
   return 0.0;
 }
 
++ (NSFont*)fontForBookmarkBarCell {
+  return [NSFont systemFontOfSize:kDefaultFontSize];
+}
+
++ (NSParagraphStyle*)paragraphStyleForBookmarkBarCell {
+  NSMutableParagraphStyle* style = [[NSMutableParagraphStyle alloc] init];
+  [style setAlignment:NSNaturalTextAlignment];
+  [style setLineBreakMode:NSLineBreakByTruncatingTail];
+  return [style autorelease];
+}
+
+// Returns |title| with newlines and line feeds replaced with
+// spaces.
++ (NSString*)cleanTitle:(NSString*)title {
+  title = [title stringByReplacingOccurrencesOfString:@"\n" withString:@" "];
+  title = [title stringByReplacingOccurrencesOfString:@"\r" withString:@" "];
+  return title;
+}
 
 @end