keep the same collection when showing bookmark manager. make cmd-b be

a toggle with "show/hide" menu items instead of just "manage" menu text. make
cmd-y not toggle and always show history. fix updating of bookmark menus
when there are no windows open. remove some outdated cruft from the main
controller and nib. (bug 226904, and then some)

camino/resources/localized/English.lproj/MainMenu.nib/classes.nib

@@ -32,7 +32,7 @@
                 goHome = id; 
                 importBookmarks = id; 
                 infoLink = id; 
-                manageSidebar = id; 
+                manageBookmarks = id; 
                 newTab = id; 
                 newWindow = id; 
                 nextTab = id; 
@@ -45,11 +45,10 @@
                 reloadWithCharset = id; 
                 savePage = id; 
                 sendURL = id; 
-                showAboutBox = id; 
+                showHistory = id; 
                 smallerTextSize = id; 
                 toggleBookmarksToolbar = id; 
                 toggleOfflineMode = id; 
-                toggleSidebar = id; 
                 viewSource = id; 
             }; 
             CLASS = MainController; 

camino/src/application/MainController.h

@@ -117,14 +117,14 @@
 -(IBAction) connectToServer:(id)aSender;
 
 // View menu actions.
--(IBAction) toggleSidebar:(id)sender;
 -(IBAction) toggleBookmarksToolbar:(id)aSender;
 -(IBAction) doReload:(id)aSender;
 -(IBAction) doStop:(id)aSender;
 -(IBAction) biggerTextSize:(id)aSender;
 -(IBAction) smallerTextSize:(id)aSender;
 -(IBAction) viewSource:(id)aSender;
--(IBAction) manageSidebar: (id)aSender;
+-(IBAction) manageBookmarks: (id)aSender;
+-(IBAction) showHistory:(id)aSender;
 -(IBAction) reloadWithCharset:(id)aSender;
 
 // Bookmarks menu actions.

camino/src/application/MainController.mm

@@ -617,11 +617,29 @@ const int kReuseWindowOnAE = 2;
 	[[ProgressDlgController sharedDownloadController] showWindow:aSender];
 }
 
-- (void)adjustBookmarksMenuItemsEnabling:(BOOL)inBrowserWindowFrontmost;
+//
+// -adjustBookmarksMenuItemsEnabling:
+//
+// We've turned off auto-enabling for the bookmarks menu because of the unknown
+// number of bookmarks in the list so we have to manage it manually. This routine
+// should be called whenever a window goes away, becomes main, or is no longer main.
+//
+- (void)adjustBookmarksMenuItemsEnabling:(BOOL)inBrowserWindowFrontmost
 {
   [mAddBookmarkMenuItem setEnabled:inBrowserWindowFrontmost];
   [mCreateBookmarksFolderMenuItem setEnabled:inBrowserWindowFrontmost];
   [mCreateBookmarksSeparatorMenuItem setEnabled:YES];
+
+  [mToggleSidebarMenuItem setEnabled:YES];     // always enabled. 
+  BrowserWindowController* browserController = (BrowserWindowController*)[[self getFrontmostBrowserWindow] windowController];
+  if (browserController) {
+    if ([browserController bookmarksAreVisible:NO])
+      [mToggleSidebarMenuItem setTitle:NSLocalizedString(@"Hide All Bookmarks", @"")];
+    else
+      [mToggleSidebarMenuItem setTitle:NSLocalizedString(@"Show All Bookmarks", @"")];
+  }
+  else
+    [mToggleSidebarMenuItem setTitle:NSLocalizedString(@"Show All Bookmarks", @"")];
 }
 
 - (NSView*)getSavePanelView
@@ -815,12 +833,12 @@ const int kReuseWindowOnAE = 2;
 }
 
 //
-// manageSizebar:
+// -showHistory:
 //
-// opens the appropriate sidebar panel (creating a new window if needed)
-// depending on the tag of the menu item
+// show the history in the bookmark manager. Creates a new window if
+// one isn't already there. history isn't a toggle, hence the name.
 //
--(IBAction)manageSidebar: (id)aSender
+-(IBAction) showHistory:(id)aSender
 {
   NSWindow* browserWindow = [self getFrontmostBrowserWindow];
   if (!browserWindow) {
@@ -828,10 +846,23 @@ const int kReuseWindowOnAE = 2;
     browserWindow = [mApplication mainWindow];
   }
 
-  if ( [aSender tag] == 0 )
-    [[browserWindow windowController] manageBookmarks: aSender];
-  else
-    [[browserWindow windowController] manageHistory: aSender];
+  [[browserWindow windowController] manageHistory: aSender];
+}
+
+//
+// manageBookmarks:
+//
+// toggle the bookmark manager (creating a new window if needed)
+//
+-(IBAction)manageBookmarks: (id)aSender
+{
+  NSWindow* browserWindow = [self getFrontmostBrowserWindow];
+  if (!browserWindow) {
+    [self newWindow:self];
+    browserWindow = [mApplication mainWindow];
+  }
+
+  [[browserWindow windowController] manageBookmarks: aSender];
 }
 
 - (MVPreferencesController *)preferencesController
@@ -967,7 +998,7 @@ const int kReuseWindowOnAE = 2;
   // disable items that aren't relevant if there's no main browser window open
   SEL action = [aMenuItem action];
 
-  //NSLog(@"MainController validateMenuItem for %@ (%s)", [aMenuItem title], action);
+  // NSLog(@"MainController validateMenuItem for %@ (%s)", [aMenuItem title], action);
   
   if (action == @selector(printPage:) ||
         /* ... many more items go here ... */
@@ -981,7 +1012,7 @@ const int kReuseWindowOnAE = 2;
   {
     return (browserController != nil);
   }
-
+  
   if (action == @selector(newTab:))
     return (browserController && [browserController newTabsAllowed]);
   
@@ -1011,13 +1042,6 @@ const int kReuseWindowOnAE = 2;
       return NO;
   }
 
-  if (action == @selector(toggleSidebar:)) {
-    if (browserController)
-      return YES;
-    else
-      return NO;
-  }
-  
   if ( action == @selector(getInfo:) )
     return (browserController && [browserController canGetInfo]);
 
@@ -1069,14 +1093,6 @@ const int kReuseWindowOnAE = 2;
   return YES;
 }
 
--(IBAction) toggleSidebar:(id)sender
-{
-    BrowserWindowController *browserController = [self getMainWindowBrowserController];
-    if (!browserController) return;
-
-    [browserController toggleSidebar:sender];
-}
-
 -(IBAction) toggleBookmarksToolbar:(id)aSender
 {
   BrowserWindowController* browserController = [self getMainWindowBrowserController];

camino/src/browser/BrowserWindowController.mm

@@ -388,6 +388,10 @@ static NSArray* sToolbarDefaults = nil;
   // ensure that the browser is visible when we close.
   [self ensureBrowserVisible:self];
   
+  // we have to manually enable/disable the bookmarks menu items, because we
+  // turn autoenabling off for that menu
+  [[NSApp delegate] adjustBookmarksMenuItemsEnabling:NO];
+
   [[NSNotificationCenter defaultCenter] removeObserver:self];
 
   // autorelease just in case we're here because of a window closing
@@ -1125,14 +1129,34 @@ static NSArray* sToolbarDefaults = nil;
   mCachedBMVC = aVC;
 }
 
+//
+// - manageBookmarks:
+//
+// Toggle the bookmark manager in all cases. This is what users
+// expect to happen. When the manager is displayed, retain the 
+// last selected collection, regardless of what it is.
+//
 -(IBAction)manageBookmarks: (id)aSender
 {
-  if ( ![mContentView isBookmarkManagerVisible] )
-    [self toggleBookmarkManager: self];
-
-  [mBookmarkViewController selectContainer:kBookmarkMenuContainerIndex];
+  [self toggleBookmarkManager: self];
 }
 
+//
+// -manageHistory:
+//
+// History is a slightly different beast from bookmarks. Unlike 
+// bookmarks, which acts as a toggle, history ensures the manager
+// is visible and selects the history collection. If the manager
+// is already visible, selects the history collection.
+//
+// An alternate solution would be to have it select history when
+// it wasn't the selected container, and hide when history was
+// the selected collection (toggling in that one case). This makes
+// me feel dirty as the command does two different things depending
+// on the (possibly undiscoverable to the user) context in which it is
+// invoked. For that reason, I've chosen to not have history be a 
+// toggle and see the fallout.
+//
 -(IBAction)manageHistory: (id)aSender
 {
   if ( ![mContentView isBookmarkManagerVisible] )
@@ -2604,6 +2628,10 @@ static NSArray* sToolbarDefaults = nil;
     if (browserView)
       [browserView setActive:YES];
   }
+
+  // we have to manually update the bookmarks menu items, because we
+  // turn autoenabling off for that menu
+  [[NSApp delegate] adjustBookmarksMenuItemsEnabling:[[self window] isMainWindow]];
 }
 
 //