Bookmark bar menu/folder button text is left-aligned, not centered.

Bookmark bar menu/folder maximum window width is now 1000, not 150.

High cosmetic impact.

BUG=36487, 17608

TEST=\
1) Add bookmarks on the bar with small (e.g. 'x') and big titles.
Make sure all looks OK (e.g. small ones are small.)
2) Add bookmarks in a folder.
Make sure text is left aligned on the menus.
3) Add bookmarks in a folder with long names (e.g. 100 characters).
Make sure folder/menus are now much wider.
4) Add bookmarks in a folder with mega long names (e.g. 4000 characters).
Make sure menus have a maximum size of ~1000 pixes (no need to be exact but 'no limit' is fail).
5) In a folder, add a subfolder named 'x' and a bookmark with a REAL long name.
Make sure the "button" (menu item) is the full width of the menu.

Review URL: http://codereview.chromium.org/842005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41387 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 96 insertions, 24 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index 8d2c117..f5ad45b 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -39,7 +39,14 @@ namespace bookmarks {
 // Magic numbers from Cole
 // TODO(jrg): create an objc-friendly version of bookmark_bar_constants.h?
 
+// Used as a maximum width for buttons on the bar.
 const CGFloat kDefaultBookmarkWidth = 150.0;
+
+// TODO(jrg): http://crbug.com/36276 to get final sizes.
+// Used as a min/max width for buttons on menus (not on the bar).
+const CGFloat kBookmarkMenuButtonMinimumWidth = kDefaultBookmarkWidth;
+const CGFloat kBookmarkMenuButtonMaximumWidth = 1000.0;
+
 const CGFloat kBookmarkVerticalPadding = 2.0;
 const CGFloat kBookmarkHorizontalPadding = 1.0;
 
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
index 2dca753..88a5f21 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
@@ -90,6 +90,7 @@
 // If |node| is NULL this is an "(empty)" button.
 // Does NOT add this button to our button list.
 // Returns an autoreleased button.
+// Adjusts the input frame width as appropriate.
 //
 // TODO(jrg): combine with addNodesToButtonList: code from
 // bookmark_bar_controller.mm, and generalize that to use both x and y
@@ -97,10 +98,20 @@
 // http://crbug.com/35966
 - (BookmarkButton*)makeButtonForNode:(const BookmarkNode*)node
                                frame:(NSRect)frame {
+  NSCell* cell = [self cellForBookmarkNode:node];
+  DCHECK(cell);
+
+  // The "+2" is needed because, sometimes, Cocoa is off by a tad when
+  // returning the value it thinks it needs.
+  CGFloat desired = [cell cellSize].width + 2;
+  frame.size.width = std::min(
+    std::max(bookmarks::kBookmarkMenuButtonMinimumWidth, desired),
+    bookmarks::kBookmarkMenuButtonMaximumWidth);
+
   BookmarkButton* button = [[[BookmarkButton alloc] initWithFrame:frame]
                                autorelease];
   DCHECK(button);
-  NSCell* cell = [self cellForBookmarkNode:node];
+
   [button setCell:cell];
   [button setDelegate:self];
   if (node) {
@@ -182,25 +193,30 @@
   int buttons = node->GetChildCount();
   if (buttons == 0)
     buttons = 1;  // the "empty" button
+
   int height = buttons * bookmarks::kBookmarkButtonHeight;
-  // TODO(jrg): use full width for buttons, like menus?
-  // http://crbug.com/36487
-  int width = (bookmarks::kDefaultBookmarkWidth +
-               2 * bookmarks::kBookmarkVerticalPadding);
-  [[self window] setFrame:NSMakeRect(newWindowTopLeft.x,
-                                     newWindowTopLeft.y - height,
-                                     width,
-                                     height)
-                  display:YES];
+
+  // Note: this will be replaced once we make buttons; for now, use a
+  // reasonable value.  Button creation needs a valid (x,y,h) in a
+  // frame to position them properly.
+  int windowWidth = (bookmarks::kBookmarkMenuButtonMinimumWidth +
+                     2 * bookmarks::kBookmarkVerticalPadding);
+
+  NSRect windowFrame = NSMakeRect(newWindowTopLeft.x,
+                                  newWindowTopLeft.y - height,
+                                  windowWidth,
+                                  height);
+  [[self window] setFrame:windowFrame display:YES];
 
   // TODO(jrg): combine with frame code in bookmark_bar_controller.mm
   // http://crbug.com/35966
-  NSRect frame = NSMakeRect(bookmarks::kBookmarkHorizontalPadding,
-                            height - (bookmarks::kBookmarkBarHeight -
-                                      bookmarks::kBookmarkHorizontalPadding),
-                            bookmarks::kDefaultBookmarkWidth,
-                            (bookmarks::kBookmarkBarHeight -
-                             2 * bookmarks::kBookmarkVerticalPadding));
+  NSRect buttonsOuterFrame = NSMakeRect(
+    bookmarks::kBookmarkHorizontalPadding,
+    height - (bookmarks::kBookmarkBarHeight -
+              bookmarks::kBookmarkHorizontalPadding),
+    bookmarks::kDefaultBookmarkWidth,
+    (bookmarks::kBookmarkBarHeight -
+     2 * bookmarks::kBookmarkVerticalPadding));
 
   // TODO(jrg): combine with addNodesToButtonList: code from
   // bookmark_bar_controller.mm (but use y offset)
@@ -208,21 +224,42 @@
   if (!node->GetChildCount()) {
     // If no children we are the empty button.
     BookmarkButton* button = [self makeButtonForNode:nil
-                                               frame:frame];
+                                               frame:buttonsOuterFrame];
     [buttons_ addObject:button];
     [mainView_ addSubview:button];
   } else {
     for (int i = 0; i < node->GetChildCount(); i++) {
       const BookmarkNode* child = node->GetChild(i);
       BookmarkButton* button = [self makeButtonForNode:child
-                                                 frame:frame];
+                                                 frame:buttonsOuterFrame];
       [buttons_ addObject:button];
       [mainView_ addSubview:button];
-      frame.origin.y -= bookmarks::kBookmarkBarHeight;
+      buttonsOuterFrame.origin.y -= bookmarks::kBookmarkBarHeight;
     }
   }
-
   [self updateTheme:[self themeProvider]];
+
+  // Now that we have all our buttons we can determine the real size
+  // of our window.
+  CGFloat width = 0.0;
+  for (BookmarkButton* button in buttons_.get()) {
+    width = std::max(width, NSWidth([button bounds]));
+  }
+  width = std::min(width, bookmarks::kBookmarkMenuButtonMaximumWidth);
+
+  // Things look and feel more menu-like if all the buttons are the
+  // full width of the window, especially if there are submenus.
+  for (BookmarkButton* button in buttons_.get()) {
+    NSRect buttonFrame = [button frame];
+    buttonFrame.size.width = width;
+    [button setFrame:buttonFrame];
+  }
+
+  // Finally, set our window size.
+  width += (2 * bookmarks::kBookmarkVerticalPadding);
+  windowFrame.size.width = width;
+  [[self window] setFrame:windowFrame display:YES];
+
   [[parentController_ parentWindow] addChildWindow:[self window]
                                            ordered:NSWindowAbove];
 }
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm
index 59d7465..7d00a23 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm
@@ -38,6 +38,7 @@ class BookmarkBarFolderControllerTest : public CocoaTest {
  public:
   BrowserTestHelper helper_;
   scoped_nsobject<BookmarkBarController> parentBarController_;
+  const BookmarkNode* longTitleNode_;  // owned by model
 
   BookmarkBarFolderControllerTest() {
     BookmarkModel* model = helper_.profile()->GetBookmarkModel();
@@ -52,9 +53,10 @@ class BookmarkBarFolderControllerTest : public CocoaTest {
                                                   L"subgroup");
     model->AddURL(folderA, folderA->GetChildCount(), L"title a",
                   GURL("http://www.google.com/a"));
-    model->AddURL(folderA, folderA->GetChildCount(),
-                  L"title super duper long long whoa momma title you betcha",
-                  GURL("http://www.google.com/b"));
+    longTitleNode_ = model->AddURL(
+      folderA, folderA->GetChildCount(),
+      L"title super duper long long whoa momma title you betcha",
+      GURL("http://www.google.com/b"));
     model->AddURL(folderB, folderB->GetChildCount(), L"t",
                   GURL("http://www.google.com/c"));
 
@@ -67,6 +69,13 @@ class BookmarkBarFolderControllerTest : public CocoaTest {
     [parentBarController_ loaded:model];
   }
 
+  // Remove the bookmark with the long title.
+  void RemoveLongTitleNode() {
+    BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+    model->Remove(longTitleNode_->GetParent(),
+                  longTitleNode_->GetParent()->IndexOfChild(longTitleNode_));
+  }
+
   // Return a simple BookmarkBarFolderController.
   BookmarkBarFolderController* SimpleBookmarkBarFolderController() {
     BookmarkButton* parentButton = [[parentBarController_ buttons]
@@ -231,6 +240,25 @@ TEST_F(BookmarkBarFolderControllerTest, ChildFolderCallbacks) {
   EXPECT_TRUE([bbfc childFolderWillClose]);
 }
 
+// Make sure bookmark folders have variable widths.
+TEST_F(BookmarkBarFolderControllerTest, ChildFolderWidth) {
+  scoped_nsobject<BookmarkBarFolderController> bbfc;
+
+  bbfc.reset(SimpleBookmarkBarFolderController());
+  EXPECT_TRUE(bbfc.get());
+  CGFloat wideWidth = NSWidth([[bbfc window] frame]);
+
+  RemoveLongTitleNode();
+  bbfc.reset(SimpleBookmarkBarFolderController());
+  EXPECT_TRUE(bbfc.get());
+  CGFloat thinWidth = NSWidth([[bbfc window] frame]);
+
+  // Make sure window size changed as expected.
+  EXPECT_GT(wideWidth, thinWidth);
+}
+
+
+
 // TODO(jrg): draggingEntered: and draggingExited: trigger timers so
 // they are hard to test.  Factor out "fire timers" into routines
 // which can be overridden to fire immediately to make behavior
diff --git a/chrome/browser/cocoa/bookmark_button_cell.mm b/chrome/browser/cocoa/bookmark_button_cell.mm
index 0f790e3..6ee2e8c 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.mm
+++ b/chrome/browser/cocoa/bookmark_button_cell.mm
@@ -98,7 +98,7 @@
   DCHECK([self controlView]);
 
   scoped_nsobject<NSMutableParagraphStyle> style([NSMutableParagraphStyle new]);
-  [style setAlignment:NSCenterTextAlignment];
+  [style setAlignment:NSLeftTextAlignment];
   NSDictionary* dict = [NSDictionary
                          dictionaryWithObjectsAndKeys:color,
                          NSForegroundColorAttributeName,