Enable the OK button when editing a bookmark even when there is no text, and keep alignment correct in both cases. Also strip out newlines from bookmark titles.

[Patch by feldstein.]

BUG=26353
TEST=Add and remove titles from bookmarks in the bookmark bar and verify that they look correct, and icons are centered when there is no text.

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

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

5 files changed, 31 insertions, 12 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index a9d9ba4..8909574 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -594,14 +594,13 @@
 // TODO(jrg): move much of the cell config into the BookmarkButtonCell class.
 - (NSCell*)cellForBookmarkNode:(const BookmarkNode*)node {
   NSString* title = base::SysWideToNSString(node->GetTitle());
-  NSButtonCell *cell = [[[BookmarkButtonCell alloc] initTextCell:nil]
-                         autorelease];
+  BookmarkButtonCell* cell =
+      [[[BookmarkButtonCell alloc] initTextCell:nil] autorelease];
   DCHECK(cell);
   [cell setRepresentedObject:[NSValue valueWithPointer:node]];
 
-  [cell setImage:[self getFavIconForNode:node]];
-  [cell setImagePosition:NSImageLeft];
-  [cell setTitle:title];
+  NSImage* image = [self getFavIconForNode:node];
+  [cell setBookmarkCellText:title image:image];
   [cell setMenu:buttonContextMenu_];
   return cell;
 }
@@ -862,12 +861,12 @@
 - (void)nodeFavIconLoaded:(BookmarkModel*)model
                      node:(const BookmarkNode*)node {
   for (NSButton* button in buttons_.get()) {
-    NSButtonCell* cell = [button cell];
+    BookmarkButtonCell* cell = [button cell];
     void* pointer = [[cell representedObject] pointerValue];
     const BookmarkNode* cellnode = static_cast<const BookmarkNode*>(pointer);
     if (cellnode == node) {
-      [cell setImage:[self getFavIconForNode:node]];
-      [cell setImagePosition:NSImageLeft];
+      [cell setBookmarkCellText:[cell title]
+                          image:[self getFavIconForNode:node]];
       // Adding an image means we might need more room for the
       // bookmark.  Test for it by growing the button (if needed)
       // and shifting everything else over.
diff --git a/chrome/browser/cocoa/bookmark_button_cell.h b/chrome/browser/cocoa/bookmark_button_cell.h
index 252ed08..f8ca25d 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.h
+++ b/chrome/browser/cocoa/bookmark_button_cell.h
@@ -13,6 +13,11 @@
 
 @interface BookmarkButtonCell : GradientButtonCell<NSMenuDelegate> {
 }
+// |-setBookmarkCellText:image:| is used to set the text and image
+// of a BookmarkButtonCell, and align the image to the left (NSImageLeft)
+// if there is text in the title, and centered (NSImageCenter) if there is
+- (void)setBookmarkCellText:(NSString*)title
+                      image:(NSImage*)image;
 @end
 
 #endif  // CHROME_BROWSER_COCOA_BOOKMARK_BUTTON_CELL_H_
diff --git a/chrome/browser/cocoa/bookmark_button_cell.mm b/chrome/browser/cocoa/bookmark_button_cell.mm
index 2a5a3eb..1e2190b 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.mm
+++ b/chrome/browser/cocoa/bookmark_button_cell.mm
@@ -34,6 +34,23 @@
   return size;
 }
 
+- (void)setBookmarkCellText:(NSString*)title
+                      image:(NSImage*)image {
+  title = [title stringByReplacingOccurrencesOfString:@"\n"
+                                           withString:@" "];
+  title = [title stringByReplacingOccurrencesOfString:@"\r"
+                                           withString:@" "];
+  if (image) {
+    [self setImage:image];
+    if ([title length] < 1) {
+      [self setImagePosition:NSImageOnly];
+    } else {
+      [self setImagePosition:NSImageLeft];
+    }
+  }
+  [self setTitle:title];
+}
+
 // We share the context menu among all bookmark buttons.  To allow us
 // to disambiguate when needed (e.g. "open bookmark"), we set the
 // menu's delegate to be us.  We (the cell) have the bookmark encoded
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.mm b/chrome/browser/cocoa/bookmark_editor_controller.mm
index f675fc3..2915094 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller.mm
@@ -328,10 +328,8 @@ int IndexOfFolderChild(const BookmarkNode* child_node) {
 // (Yes, setting ourself as a delegate automatically registers us for
 // the notification.)
 - (void)controlTextDidChange:(NSNotification*)aNotification {
-  // Name must not be empty, but it can be whitespace.
-  NSString* name = [nameField_ stringValue];
   GURL newURL = [self GURLFromUrlField];
-  [okButton_ setEnabled:([name length] != 0 && newURL.is_valid()) ? YES : NO];
+  [okButton_ setEnabled:(newURL.is_valid()) ? YES : NO];
 }
 
 // The ok: action is connected to the OK button in the Edit Bookmark window
diff --git a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
index 9b25f0f..e8d06d5 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
@@ -138,7 +138,7 @@ TEST_F(BookmarkEditorControllerTest, EditAndConfirmOKButton) {
   EXPECT_TRUE([default_controller_ okButtonEnabled]);
   // Then test the name.
   [default_controller_ setDisplayName:@""];
-  EXPECT_FALSE([default_controller_ okButtonEnabled]);
+  EXPECT_TRUE([default_controller_ okButtonEnabled]);
   [default_controller_ setDisplayName:@"                   "];
   EXPECT_TRUE([default_controller_ okButtonEnabled]);
   // Then little mix of both.