Do not allow multiline input when naming bookmark folders on Mac

- the respective nibs now use Single Line Mode for title entry
- BookmarkModel has been updated (with test cases) to strip extra whitespace for folder/bookmark titles.

BUG=100618
TEST=BookmarkModelTest.AddURLWithWhitespaceTitle, BookmarkModelTest.AddFolderWithWhitespaceTitle, BookmarkModelTest.SetTitleWithWhitespace

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

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

chrome/browser/bookmarks/bookmark_model.cc

@@ -11,6 +11,7 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_vector.h"
+#include "base/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/bookmarks/bookmark_expanded_state_tracker.h"
 #include "chrome/browser/bookmarks/bookmark_index.h"
@@ -305,11 +306,14 @@ const SkBitmap& BookmarkModel::GetFavicon(const BookmarkNode* node) {
 }
 
 void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) {
+  // Remove extra whitespace from folder/bookmark names.
+  string16 mutable_title = CollapseWhitespace(title, false);
+
   if (!node) {
     NOTREACHED();
     return;
   }
-  if (node->GetTitle() == title)
+  if (node->GetTitle() == mutable_title)
     return;
 
   if (is_permanent_node(node)) {
@@ -320,7 +324,7 @@ void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) {
   // The title index doesn't support changing the title, instead we remove then
   // add it back.
   index_->Remove(node);
-  AsMutable(node)->set_title(title);
+  AsMutable(node)->set_title(mutable_title);
   index_->Add(node);
 
   if (store_.get())
@@ -436,7 +440,8 @@ const BookmarkNode* BookmarkModel::AddFolder(const BookmarkNode* parent,
 
   BookmarkNode* new_node = new BookmarkNode(generate_next_node_id(), GURL());
   new_node->set_date_folder_modified(Time::Now());
-  new_node->set_title(title);
+  // Folders shouldn't have line breaks in their titles.
+  new_node->set_title(CollapseWhitespace(title, false));
   new_node->set_type(BookmarkNode::FOLDER);
 
   return AddNode(AsMutable(parent), index, new_node, false);
@@ -446,7 +451,9 @@ const BookmarkNode* BookmarkModel::AddURL(const BookmarkNode* parent,
                                           int index,
                                           const string16& title,
                                           const GURL& url) {
-  return AddURLWithCreationTime(parent, index, title, url, Time::Now());
+  return AddURLWithCreationTime(parent, index,
+                                CollapseWhitespace(title, false),
+                                url, Time::Now());
 }
 
 const BookmarkNode* BookmarkModel::AddURLWithCreationTime(

chrome/browser/bookmarks/bookmark_model_unittest.cc

@@ -41,6 +41,35 @@ using content::BrowserThread;
 
 namespace {
 
+// Test cases used to test the removal of extra whitespace when adding
+// a new folder/bookmark or updating a title of a folder/bookmark.
+static struct {
+  const std::string input_title;
+  const std::string expected_title;
+} whitespace_test_cases[] = {
+  {"foobar", "foobar"},
+  // Newlines.
+  {"foo\nbar", "foo bar"},
+  {"foo\n\nbar", "foo bar"},
+  {"foo\n\n\nbar", "foo bar"},
+  {"foo\r\nbar", "foo bar"},
+  {"foo\r\n\r\nbar", "foo bar"},
+  {"\nfoo\nbar\n", "foo bar"},
+  // Spaces.
+  {"foo  bar", "foo bar"},
+  {" foo bar ", "foo bar"},
+  {"  foo  bar  ", "foo bar"},
+  // Tabs.
+  {"\tfoo\tbar\t", "foo bar"},
+  {"\tfoo bar\t", "foo bar"},
+  // Mixed cases.
+  {"\tfoo\nbar\t", "foo bar"},
+  {"\tfoo\r\nbar\t", "foo bar"},
+  {"  foo\tbar\n", "foo bar"},
+  {"\t foo \t  bar  \t", "foo bar"},
+  {"\n foo\r\n\tbar\n \t", "foo bar"},
+};
+
 // Helper to get a mutable bookmark node.
 BookmarkNode* AsMutable(const BookmarkNode* node) {
   return const_cast<BookmarkNode*>(node);
@@ -212,6 +241,22 @@ TEST_F(BookmarkModelTest, AddURL) {
               new_node->id() != model_.synced_node()->id());
 }
 
+TEST_F(BookmarkModelTest, AddURLWithWhitespaceTitle) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(whitespace_test_cases); ++i) {
+    const BookmarkNode* root = model_.bookmark_bar_node();
+    const string16 title(ASCIIToUTF16(whitespace_test_cases[i].input_title));
+    const GURL url("http://foo.com");
+
+    const BookmarkNode* new_node = model_.AddURL(root, i, title, url);
+
+    int size = i + 1;
+    EXPECT_EQ(size, root->child_count());
+    EXPECT_EQ(ASCIIToUTF16(whitespace_test_cases[i].expected_title),
+              new_node->GetTitle());
+    EXPECT_EQ(BookmarkNode::URL, new_node->type());
+  }
+}
+
 TEST_F(BookmarkModelTest, AddURLToSyncedBookmarks) {
   const BookmarkNode* root = model_.synced_node();
   const string16 title(ASCIIToUTF16("foo"));
@@ -255,6 +300,21 @@ TEST_F(BookmarkModelTest, AddFolder) {
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 }
 
+TEST_F(BookmarkModelTest, AddFolderWithWhitespaceTitle) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(whitespace_test_cases); ++i) {
+    const BookmarkNode* root = model_.bookmark_bar_node();
+    const string16 title(ASCIIToUTF16(whitespace_test_cases[i].input_title));
+
+    const BookmarkNode* new_node = model_.AddFolder(root, i, title);
+
+    int size = i + 1;
+    EXPECT_EQ(size, root->child_count());
+    EXPECT_EQ(ASCIIToUTF16(whitespace_test_cases[i].expected_title),
+              new_node->GetTitle());
+    EXPECT_EQ(BookmarkNode::FOLDER, new_node->type());
+  }
+}
+
 TEST_F(BookmarkModelTest, RemoveURL) {
   const BookmarkNode* root = model_.bookmark_bar_node();
   const string16 title(ASCIIToUTF16("foo"));
@@ -309,6 +369,20 @@ TEST_F(BookmarkModelTest, SetTitle) {
   EXPECT_EQ(title, node->GetTitle());
 }
 
+TEST_F(BookmarkModelTest, SetTitleWithWhitespace) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(whitespace_test_cases); ++i) {
+    const BookmarkNode* root = model_.bookmark_bar_node();
+    string16 title(ASCIIToUTF16("dummy"));
+    const GURL url("http://foo.com");
+    const BookmarkNode* node = model_.AddURL(root, 0, title, url);
+
+    title = ASCIIToUTF16(whitespace_test_cases[i].input_title);
+    model_.SetTitle(node, title);
+    EXPECT_EQ(ASCIIToUTF16(whitespace_test_cases[i].expected_title),
+              node->GetTitle());
+  }
+}
+
 TEST_F(BookmarkModelTest, SetURL) {
   const BookmarkNode* root = model_.bookmark_bar_node();
   const string16 title(ASCIIToUTF16("foo"));

chrome/browser/ui/cocoa/bookmarks/bookmark_cell_single_line.h

+// 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.
+
+#ifndef CHROME_BROWSER_UI_COCOA_BOOKMARKS_BOOKMARK_CELL_SINGLE_LINE_H_
+#define CHROME_BROWSER_UI_COCOA_BOOKMARKS_BOOKMARK_CELL_SINGLE_LINE_H_
+#pragma once
+
+#import <Cocoa/Cocoa.h>
+
+// Provides a category for 10.5 compilation of a selector which is only
+// available on 10.6+. This purely enables compilation when the selector
+// is present and does not implement the method itself.
+@interface NSCell(multilinebookmarks)
+- (void)setUsesSingleLineMode:(BOOL)flag;
+@end
+
+#endif  // CHROME_BROWSER_UI_COCOA_BOOKMARKS_BOOKMARK_CELL_SINGLE_LINE_H_

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

@@ -13,6 +13,7 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/profiles/profile.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_all_tabs_controller.h"
+#import "chrome/browser/ui/cocoa/bookmarks/bookmark_cell_single_line.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_tree_browser_cell.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
@@ -564,6 +565,14 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver {
     [self setTableSelectionPath:selection];
     NSInteger row = [folderTreeView_ selectedRow];
     DCHECK(row >= 0);
+
+    // Put the cell into single-line mode before putting it into edit mode.
+    NSCell* folderCell = [folderTreeView_ preparedCellAtColumn:0 row:row];
+    if ([folderCell
+          respondsToSelector:@selector(setUsesSingleLineMode:)]) {
+      [folderCell setUsesSingleLineMode:YES];
+    }
+
     [folderTreeView_ editColumn:0 row:row withEvent:nil select:YES];
   }
 }

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

@@ -7,6 +7,7 @@
 #include "base/mac/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
+#import "chrome/browser/ui/cocoa/bookmarks/bookmark_cell_single_line.h"
 #include "chrome/browser/ui/cocoa/bookmarks/bookmark_model_observer_for_cocoa.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -68,6 +69,14 @@
 
 - (void)awakeFromNib {
   [nameField_ setStringValue:initialName_.get()];
+
+  // Check if NSTextFieldCell supports the method. This check is in place as
+  // only 10.6 and greater support the setUsesSingleLineMode method.
+  NSTextFieldCell* nameFieldCell_ = [nameField_ cell];
+  if ([nameFieldCell_
+          respondsToSelector:@selector(setUsesSingleLineMode:)]) {
+    [nameFieldCell_ setUsesSingleLineMode:YES];
+  }
 }
 
 - (void)runAsModalSheet {