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]);
+}