On bookmark edit, the OK button is now disabled if the entered URL is

invalid (e.g. is the empty string).  The Cancel button is never
disabled.

BUG=http://crbug.com/17006

TEST=Right click on a bookmark button to edit it.
Make sure OK is enabled.
Set URL to "" (the empty string).
Make sure OK is DISabled.
Set URL to "x".
Make sure OK is enabled.

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

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

3 files changed, 61 insertions, 12 deletions

diff --git a/chrome/browser/cocoa/bookmark_editor_controller.h b/chrome/browser/cocoa/bookmark_editor_controller.h
index e297ea3..742fbd1 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.h
+++ b/chrome/browser/cocoa/bookmark_editor_controller.h
@@ -18,6 +18,7 @@
   IBOutlet NSTextField* nameField_;
   IBOutlet NSTextField* urlField_;
   IBOutlet NSBrowser* browser_;
+  IBOutlet NSButton* okButton_;
   IBOutlet NSButton* newFolderButton_;
 
   NSWindow* parentWindow_;
@@ -47,6 +48,7 @@
 @interface BookmarkEditorController(TestingAPI)
 @property (assign) NSString* displayName;
 @property (assign) NSString* displayURL;
+@property (readonly) BOOL okButtonEnabled;
 @end
 
 
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.mm b/chrome/browser/cocoa/bookmark_editor_controller.mm
index 704c56e..4ba0ab39 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller.mm
@@ -72,6 +72,11 @@ void BookmarkEditor::Show(gfx::NativeView parent_hwnd,
   [nameField_ setStringValue:initialName_];
   [urlField_ setStringValue:initialUrl_];
 
+  // Get a ping when the URL text changes;
+  // trigger an initial ping to set things up.
+  [urlField_ setDelegate:self];
+  [self controlTextDidChange:nil];
+
   if (configuration_ == BookmarkEditor::SHOW_TREE) {
     // build the tree et al
     NOTIMPLEMENTED();
@@ -119,6 +124,30 @@ void BookmarkEditor::Show(gfx::NativeView parent_hwnd,
   [NSApp endSheet:[self window]];
 }
 
+// If possible, return a valid GURL from the URL text field.
+- (GURL)GURLFromUrlField {
+  NSString *url = [urlField_ stringValue];
+  GURL newURL = GURL([url UTF8String]);
+  if (!newURL.is_valid()) {
+    // Mimic observed friendliness from Windows
+    newURL = GURL([[NSString stringWithFormat:@"http://%@", url] UTF8String]);
+  }
+  return newURL;
+}
+
+// When the URL changes we may enable or disable the OK button.
+// We set ourselves as the delegate of urlField_ so this gets called.
+// (Yes, setting ourself as a delegate automatically registers us for
+// the notification.)
+- (void)controlTextDidChange:(NSNotification *)aNotification {
+  GURL newURL = [self GURLFromUrlField];
+  if (newURL.is_valid()) {
+    [okButton_ setEnabled:YES];
+  } else {
+    [okButton_ setEnabled:NO];
+  }
+}
+
 // TODO(jrg): Once the tree is available edits may be more extensive
 // than just name/url.
 - (IBAction)ok:(id)sender {
@@ -128,14 +157,11 @@ void BookmarkEditor::Show(gfx::NativeView parent_hwnd,
   if ((![name isEqual:initialName_]) ||
       (![url isEqual:initialUrl_])) {
     std::wstring newTitle = base::SysNSStringToWide(name);
-    GURL newURL = GURL([url UTF8String]);
-    if (!newURL.is_valid()) {
-      // Mimic observed friendliness from Windows
-      newURL = GURL([[NSString stringWithFormat:@"http://%@", url] UTF8String]);
-    }
+    GURL newURL = [self GURLFromUrlField];
     if (!newURL.is_valid()) {
-      // Silently ignoring a bad URL is unfriendly.
-      newURL = GURL();
+      // Shouldn't be reached -- OK button disabled if not valid!
+      NOTREACHED();
+      return;
     }
     int index = 0;
     BookmarkModel* model = profile_->GetBookmarkModel();
@@ -158,6 +184,11 @@ void BookmarkEditor::Show(gfx::NativeView parent_hwnd,
 - (void)didEndSheet:(NSWindow*)sheet
          returnCode:(int)returnCode
         contextInfo:(void*)contextInfo {
+  // This is probably unnecessary but it feels cleaner since the
+  // delegate of a text field can be automatically registered for
+  // notifications.
+  [urlField_ setDelegate:nil];
+
   [[self window] orderOut:self];
 
   // BookmarkEditor::Show() will create us then run away.  Unusually
@@ -180,6 +211,11 @@ void BookmarkEditor::Show(gfx::NativeView parent_hwnd,
 
 - (void)setDisplayURL:(NSString*)name {
   [urlField_ setStringValue:name];
+  [self controlTextDidChange:nil];
+}
+
+- (BOOL)okButtonEnabled {
+  return [okButton_ isEnabled];
 }
 
 @end  // BookmarkEditorController
diff --git a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
index 8400b9c..7303f34 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
@@ -90,14 +90,25 @@ TEST_F(BookmarkEditorControllerTest, UserEditsStuff) {
   EXPECT_EQ(child->GetURL(), GURL(url_name));
 
   // Change just the URL
+  EXPECT_TRUE([controller okButtonEnabled]);
   [controller setDisplayURL:@"http://yellow-sneakers.com/"];
+  EXPECT_TRUE([controller okButtonEnabled]);
   [controller ok:nil];
   child = parent->GetChild(0);
   EXPECT_EQ(child->GetTitle(), L"whamma jamma bamma");
   EXPECT_EQ(child->GetURL(), GURL("http://yellow-sneakers.com/"));
-}
-
-
-
-
 
+  // Give it a URL which needs fixen up to be valid
+  // (e.g. http:// prefix added)
+  [controller setDisplayURL:@"x"];
+  [controller ok:nil];
+  child = parent->GetChild(0);
+  EXPECT_TRUE(child->GetURL().is_valid());
+
+  // Confirm OK button enabled/disabled as appropriate.
+  EXPECT_TRUE([controller okButtonEnabled]);
+  [controller setDisplayURL:@""];
+  EXPECT_FALSE([controller okButtonEnabled]);
+  [controller setDisplayURL:@"http://www.cnn.com"];
+  EXPECT_TRUE([controller okButtonEnabled]);
+}