diff options
20 files changed, 81 insertions, 51 deletions
diff --git a/chrome/app/chrome_dll.rc b/chrome/app/chrome_dll.rc index c32d2dd..5a26df5 100644 --- a/chrome/app/chrome_dll.rc +++ b/chrome/app/chrome_dll.rc @@ -34,6 +34,8 @@ IDR_MAINFRAME ACCELERATORS BEGIN VK_LEFT, IDC_BACK, VIRTKEY, ALT VK_BACK, IDC_BACK, VIRTKEY + "D", IDC_BOOKMARK_PAGE, VIRTKEY, CONTROL + "D", IDC_BOOKMARK_ALL_TABS, VIRTKEY, CONTROL, SHIFT VK_DELETE, IDC_CLEAR_BROWSING_DATA, VIRTKEY, CONTROL, SHIFT VK_F4, IDC_CLOSE_TAB, VIRTKEY, CONTROL "W", IDC_CLOSE_TAB, VIRTKEY, CONTROL @@ -122,7 +124,6 @@ BEGIN "H", IDC_SHOW_HISTORY, VIRTKEY, CONTROL "F", IDC_SHOW_APP_MENU, VIRTKEY, ALT "E", IDC_SHOW_PAGE_MENU, VIRTKEY, ALT - "D", IDC_STAR, VIRTKEY, CONTROL VK_ESCAPE, IDC_STOP, VIRTKEY VK_ESCAPE, IDC_TASK_MANAGER, VIRTKEY, SHIFT "U", IDC_VIEW_SOURCE, VIRTKEY, CONTROL diff --git a/chrome/app/chrome_dll_resource.h b/chrome/app/chrome_dll_resource.h index 91a802d..92e7487 100644 --- a/chrome/app/chrome_dll_resource.h +++ b/chrome/app/chrome_dll_resource.h @@ -84,12 +84,13 @@ #define IDC_MOVE_TAB_PREVIOUS 34033 // Page-related commands -#define IDC_STAR 35000 -#define IDC_VIEW_SOURCE 35001 -#define IDC_CLOSE_POPUPS 35002 -#define IDC_PRINT 35003 -#define IDC_SAVE_PAGE 35004 -#define IDC_ENCODING_MENU 35005 +#define IDC_BOOKMARK_PAGE 35000 +#define IDC_BOOKMARK_ALL_TABS 35001 +#define IDC_VIEW_SOURCE 35002 +#define IDC_CLOSE_POPUPS 35003 +#define IDC_PRINT 35004 +#define IDC_SAVE_PAGE 35005 +#define IDC_ENCODING_MENU 35006 // When adding a new encoding to this list, be sure to append it to the // EncodingMenuController::kValidEncodingIds array in diff --git a/chrome/app/nibs/MainMenu.xib b/chrome/app/nibs/MainMenu.xib index 3512963..3b777a8 100644 --- a/chrome/app/nibs/MainMenu.xib +++ b/chrome/app/nibs/MainMenu.xib @@ -320,7 +320,7 @@ <int key="NSMnemonicLoc">2147483647</int> <reference key="NSOnImage" ref="353210768"/> <reference key="NSMixedImage" ref="549394948"/> - <int key="NSTag">35004</int> + <int key="NSTag">35005</int> </object> <object class="NSMenuItem" id="553348595"> <reference key="NSMenu" ref="720053764"/> @@ -368,7 +368,7 @@ <int key="NSMnemonicLoc">2147483647</int> <reference key="NSOnImage" ref="353210768"/> <reference key="NSMixedImage" ref="549394948"/> - <int key="NSTag">35003</int> + <int key="NSTag">35004</int> </object> </object> </object> @@ -712,7 +712,7 @@ <reference key="NSOnImage" ref="353210768"/> <reference key="NSMixedImage" ref="549394948"/> <string key="NSAction">submenuAction:</string> - <int key="NSTag">35005</int> + <int key="NSTag">35006</int> <object class="NSMenu" key="NSSubmenu" id="466817936"> <string key="NSTitle">^IDS_TEXT_ENCODING_MAC</string> <object class="NSMutableArray" key="NSMenuItems"> @@ -740,7 +740,7 @@ <int key="NSMnemonicLoc">2147483647</int> <reference key="NSOnImage" ref="353210768"/> <reference key="NSMixedImage" ref="549394948"/> - <int key="NSTag">35001</int> + <int key="NSTag">35002</int> </object> <object class="NSMenuItem" id="703868281"> <reference key="NSMenu" ref="99609113"/> diff --git a/chrome/app/nibs/Toolbar.xib b/chrome/app/nibs/Toolbar.xib index 0548137..9ab07a1 100644 --- a/chrome/app/nibs/Toolbar.xib +++ b/chrome/app/nibs/Toolbar.xib @@ -384,7 +384,7 @@ <int key="NSMnemonicLoc">2147483647</int> <reference key="NSOnImage" ref="610745052"/> <reference key="NSMixedImage" ref="1070349142"/> - <int key="NSTag">35004</int> + <int key="NSTag">35005</int> </object> <object class="NSMenuItem" id="950677546"> <reference key="NSMenu" ref="1035953805"/> @@ -490,7 +490,7 @@ <int key="NSMnemonicLoc">2147483647</int> <reference key="NSOnImage" ref="610745052"/> <reference key="NSMixedImage" ref="1070349142"/> - <int key="NSTag">35001</int> + <int key="NSTag">35002</int> </object> <object class="NSMenuItem" id="830865062"> <reference key="NSMenu" ref="383305928"/> diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc index 95cfe64..edc3a46 100644 --- a/chrome/browser/bookmarks/bookmark_utils.cc +++ b/chrome/browser/bookmarks/bookmark_utils.cc @@ -614,20 +614,21 @@ void RegisterUserPrefs(PrefService* prefs) { bool GetURLAndTitleToBookmark(TabContents* tab_contents, GURL* url, std::wstring* title) { - if (!tab_contents->ShouldDisplayURL()) + if (!tab_contents || !tab_contents->ShouldDisplayURL()) return false; - *url = tab_contents->GetURL(); - if (url->is_empty() || !url->is_valid()) + GURL tab_url = tab_contents->GetURL(); + if (!tab_url.is_valid()) return false; - *title = UTF16ToWideHack(tab_contents->GetTitle()); + if (url != NULL) + *url = tab_url; + if (title != NULL) + *title = UTF16ToWideHack(tab_contents->GetTitle()); return true; } const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser) { BookmarkModel* model = browser->profile()->GetBookmarkModel(); - if (!model || !model->IsLoaded()) - return NULL; - + DCHECK(model && model->IsLoaded()); const BookmarkNode* parent = model->GetParentForNewNodes(); const BookmarkNode* folder = model->AddGroup( parent, parent->GetChildCount(), @@ -635,9 +636,8 @@ const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser) { for (int i = 0; i < browser->tab_count(); ++i) { GURL url; std::wstring title; - if (GetURLAndTitleToBookmark(browser->GetTabContentsAt(i), &url, &title)) { + if (GetURLAndTitleToBookmark(browser->GetTabContentsAt(i), &url, &title)) model->AddURL(folder, folder->GetChildCount(), title, url); - } } return folder; } diff --git a/chrome/browser/bookmarks/bookmark_utils.h b/chrome/browser/bookmarks/bookmark_utils.h index d45ecad..0b5718d 100644 --- a/chrome/browser/bookmarks/bookmark_utils.h +++ b/chrome/browser/bookmarks/bookmark_utils.h @@ -182,15 +182,14 @@ void RegisterPrefs(PrefService* prefs); // Register user prefs for BookmarkBar, BookmarkView, ... void RegisterUserPrefs(PrefService* prefs); -// Gets the url and title to use in creating a bookmark for the specified -// TabContents. Returns false if a bookmark shouldn't be created for the -// specified TabContents. +// Returns whether |tab_contents| can be bookmarked. If it can, |url| and +// |title| are filled in. Any of the parameters may be NULL. bool GetURLAndTitleToBookmark(TabContents* tab_contents, GURL* url, std::wstring* title); // Creates a new folder containing a bookmark for each of the tabs in -// |browser|. This returns null if the bookmark model isn't loaded. +// |browser|. const BookmarkNode* CreateBookmarkForAllTabs(Browser* browser); // Number of bookmarks we'll open before prompting the user to see if they diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 27ad096..eb63048 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -1366,7 +1366,8 @@ void Browser::ExecuteCommandWithDisposition( // Page-related commands case IDC_SAVE_PAGE: SavePage(); break; - case IDC_STAR: BookmarkCurrentPage(); break; + case IDC_BOOKMARK_PAGE: BookmarkCurrentPage(); break; + case IDC_BOOKMARK_ALL_TABS: BookmarkAllTabs(); break; case IDC_VIEW_SOURCE: ViewSource(); break; #if defined(OS_WIN) case IDC_CLOSE_POPUPS: ClosePopups(); break; @@ -1679,6 +1680,19 @@ bool Browser::CanCloseContentsAt(int index) { return CanCloseWithInProgressDownloads(); } +bool Browser::CanBookmarkAllTabs() const { + BookmarkModel* model = profile()->GetBookmarkModel(); + if (!model || !model->IsLoaded()) + return false; + + int bookmarkable_tab_contents = 0; + for (int i = 0; i < tab_count(); ++i) { + if (CanBookmarkTabContents(GetTabContentsAt(i))) + ++bookmarkable_tab_contents; + } + return (bookmarkable_tab_contents > 1); +} + void Browser::BookmarkAllTabs() { const BookmarkNode* node = bookmark_utils::CreateBookmarkForAllTabs(this); if (!node) @@ -2419,9 +2433,6 @@ void Browser::InitCommandState() { command_updater_.UpdateCommandEnabled(IDC_RESTORE_TAB, normal_window && !profile_->IsOffTheRecord()); - // Page-related commands - command_updater_.UpdateCommandEnabled(IDC_STAR, normal_window); - // Show various bits of UI command_updater_.UpdateCommandEnabled(IDC_CLEAR_BROWSING_DATA, normal_window); @@ -2453,6 +2464,11 @@ void Browser::UpdateCommandsForTabState() { // Page-related commands window_->SetStarredState(current_tab->is_starred()); + command_updater_.UpdateCommandEnabled(IDC_BOOKMARK_PAGE, + CanBookmarkTabContents(current_tab)); + command_updater_.UpdateCommandEnabled(IDC_BOOKMARK_ALL_TABS, + CanBookmarkAllTabs()); + // View-source should not be enabled if already in view-source mode or // the source is not viewable. command_updater_.UpdateCommandEnabled(IDC_VIEW_SOURCE, @@ -2482,6 +2498,10 @@ void Browser::UpdateStopGoState(bool is_loading, bool force) { command_updater_.UpdateCommandEnabled(IDC_STOP, is_loading); } +bool Browser::CanBookmarkTabContents(TabContents* tab_contents) const { + return (type() == TYPE_NORMAL) && + bookmark_utils::GetURLAndTitleToBookmark(tab_contents, NULL, NULL); +} /////////////////////////////////////////////////////////////////////////////// // Browser, UI update coalescing and handling (private): diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index d176b1d..715559a 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -474,6 +474,7 @@ class Browser : public TabStripModelDelegate, virtual void CreateHistoricalTab(TabContents* contents); virtual bool RunUnloadListenerBeforeClosing(TabContents* contents); virtual bool CanCloseContentsAt(int index); + virtual bool CanBookmarkAllTabs() const; virtual void BookmarkAllTabs(); // Overridden from TabStripModelObserver: @@ -563,6 +564,9 @@ class Browser : public TabStripModelDelegate, // loading. |force| is true if the button should change its icon immediately. void UpdateStopGoState(bool is_loading, bool force); + // Returns true if the specified tab contents can be bookmarked. + bool CanBookmarkTabContents(TabContents* tab_contents) const; + // UI update coalescing and handling //////////////////////////////////////// // Asks the toolbar (and as such the location bar) to update its state to diff --git a/chrome/browser/browser_commands_unittest.cc b/chrome/browser/browser_commands_unittest.cc index 768edd5..144263b 100644 --- a/chrome/browser/browser_commands_unittest.cc +++ b/chrome/browser/browser_commands_unittest.cc @@ -90,10 +90,10 @@ TEST_F(BrowserCommandsTest, BookmarkCurrentPage) { // TODO(beng): remove this once we can use TabContentses directly in testing // instead of the TestTabContents which causes this command not to // be enabled when the tab is added (and selected). - browser()->command_updater()->UpdateCommandEnabled(IDC_STAR, true); + browser()->command_updater()->UpdateCommandEnabled(IDC_BOOKMARK_PAGE, true); // Star it. - browser()->ExecuteCommand(IDC_STAR); + browser()->ExecuteCommand(IDC_BOOKMARK_PAGE); // It should now be bookmarked in the bookmark model. EXPECT_EQ(profile(), browser()->profile()); diff --git a/chrome/browser/cocoa/tab_strip_controller_unittest.mm b/chrome/browser/cocoa/tab_strip_controller_unittest.mm index bcc2cf8..2e83aa2 100644 --- a/chrome/browser/cocoa/tab_strip_controller_unittest.mm +++ b/chrome/browser/cocoa/tab_strip_controller_unittest.mm @@ -57,6 +57,8 @@ class TestTabStripDelegate : public TabStripModelDelegate { virtual bool CanCloseContentsAt(int index) { return true; } + virtual bool CanBookmarkAllTabs() const { return false; } + virtual void BookmarkAllTabs() {} }; diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm index ed726f8..5f591db 100644 --- a/chrome/browser/cocoa/toolbar_controller.mm +++ b/chrome/browser/cocoa/toolbar_controller.mm @@ -101,13 +101,13 @@ class PrefObserverBridge : public NotificationObserver { resizeDelegate_ = resizeDelegate; hasToolbar_ = YES; - // Register for notificaotions about state changes for the toolbar buttons + // Register for notifications about state changes for the toolbar buttons commandObserver_.reset(new CommandObserverBridge(self, commands)); commandObserver_->ObserveCommand(IDC_BACK); commandObserver_->ObserveCommand(IDC_FORWARD); commandObserver_->ObserveCommand(IDC_RELOAD); commandObserver_->ObserveCommand(IDC_HOME); - commandObserver_->ObserveCommand(IDC_STAR); + commandObserver_->ObserveCommand(IDC_BOOKMARK_PAGE); } return self; } @@ -239,7 +239,7 @@ class PrefObserverBridge : public NotificationObserver { case IDC_HOME: button = homeButton_; break; - case IDC_STAR: + case IDC_BOOKMARK_PAGE: button = starButton_; break; } @@ -254,7 +254,8 @@ class PrefObserverBridge : public NotificationObserver { setEnabled:commands->IsCommandEnabled(IDC_FORWARD) ? YES : NO]; [reloadButton_ setEnabled:commands->IsCommandEnabled(IDC_RELOAD) ? YES : NO]; [homeButton_ setEnabled:commands->IsCommandEnabled(IDC_HOME) ? YES : NO]; - [starButton_ setEnabled:commands->IsCommandEnabled(IDC_STAR) ? YES : NO]; + [starButton_ + setEnabled:commands->IsCommandEnabled(IDC_BOOKMARK_PAGE) ? YES : NO]; } - (void)updateToolbarWithContents:(TabContents*)tab diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm index b27eafd..4b96644 100644 --- a/chrome/browser/cocoa/toolbar_controller_unittest.mm +++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm @@ -77,7 +77,7 @@ class ToolbarControllerTest : public PlatformTest { [[views objectAtIndex:kReloadIndex] isEnabled] ? true : false); EXPECT_EQ(updater->IsCommandEnabled(IDC_HOME), [[views objectAtIndex:kHomeIndex] isEnabled] ? true : false); - EXPECT_EQ(updater->IsCommandEnabled(IDC_STAR), + EXPECT_EQ(updater->IsCommandEnabled(IDC_BOOKMARK_PAGE), [[views objectAtIndex:kStarIndex] isEnabled] ? true : false); } diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc index f2bba91..c598070 100644 --- a/chrome/browser/gtk/browser_toolbar_gtk.cc +++ b/chrome/browser/gtk/browser_toolbar_gtk.cc @@ -84,7 +84,7 @@ BrowserToolbarGtk::BrowserToolbarGtk(Browser* browser, BrowserWindowGtk* window) browser_->command_updater()->AddCommandObserver(IDC_FORWARD, this); browser_->command_updater()->AddCommandObserver(IDC_RELOAD, this); browser_->command_updater()->AddCommandObserver(IDC_HOME, this); - browser_->command_updater()->AddCommandObserver(IDC_STAR, this); + browser_->command_updater()->AddCommandObserver(IDC_BOOKMARK_PAGE, this); registrar_.Add(this, NotificationType::BROWSER_THEME_CHANGED, @@ -298,7 +298,7 @@ void BrowserToolbarGtk::EnabledStateChangedForCommand(int id, bool enabled) { if (home_.get()) widget = home_->widget(); break; - case IDC_STAR: + case IDC_BOOKMARK_PAGE: widget = star_->widget(); break; } @@ -600,7 +600,7 @@ void BrowserToolbarGtk::OnButtonClick(GtkWidget* button, } else if (toolbar->home_.get() && button == toolbar->home_->widget()) { tag = IDC_HOME; } else if (button == toolbar->star_->widget()) { - tag = IDC_STAR; + tag = IDC_BOOKMARK_PAGE; } DCHECK_NE(tag, -1) << "Unexpected button click callback"; diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index d7d7bfb..b1f5983 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -286,8 +286,8 @@ const struct AcceleratorMapping { { XF86XK_Refresh, IDC_RELOAD, GdkModifierType(0) }, // Miscellany. - { GDK_d, IDC_STAR, GDK_CONTROL_MASK }, - { XF86XK_AddFavorite, IDC_STAR, GdkModifierType(0) }, + { GDK_d, IDC_BOOKMARK_PAGE, GDK_CONTROL_MASK }, + { XF86XK_AddFavorite, IDC_BOOKMARK_PAGE, GdkModifierType(0) }, { XF86XK_Favorites, IDC_SHOW_BOOKMARK_BAR, GdkModifierType(0) }, { XF86XK_History, IDC_SHOW_HISTORY, GdkModifierType(0) }, { GDK_o, IDC_OPEN_FILE, GDK_CONTROL_MASK }, diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc index 9ceb6d8..a0802f9 100644 --- a/chrome/browser/tabs/tab_strip_model.cc +++ b/chrome/browser/tabs/tab_strip_model.cc @@ -488,10 +488,7 @@ bool TabStripModel::IsContextMenuCommandEnabled( case CommandTogglePinned: return true; case CommandBookmarkAllTabs: { - if (count() <= 1) - return false; - BookmarkModel* model = profile_->GetBookmarkModel(); - return model && model->IsLoaded(); + return delegate_->CanBookmarkAllTabs(); } default: NOTREACHED(); diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h index 9e7659e..0c70378 100644 --- a/chrome/browser/tabs/tab_strip_model.h +++ b/chrome/browser/tabs/tab_strip_model.h @@ -200,6 +200,10 @@ class TabStripModelDelegate { // Returns whether some contents can be closed. virtual bool CanCloseContentsAt(int index) = 0; + // Returns true if we should allow "bookmark all tabs" in this window; this is + // true when there is more than one bookmarkable tab open. + virtual bool CanBookmarkAllTabs() const = 0; + // Creates a bookmark folder containing a bookmark for all open tabs. virtual void BookmarkAllTabs() = 0; }; diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index 5beb59f..949d29e 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -65,6 +65,7 @@ class TabStripDummyDelegate : public TabStripModelDelegate { virtual bool CanRestoreTab() { return false; } virtual void RestoreTab() {} virtual bool CanCloseContentsAt(int index) { return can_close_ ; } + virtual bool CanBookmarkAllTabs() const { return false; } virtual void BookmarkAllTabs() {} private: diff --git a/chrome/browser/views/accelerator_table_gtk.cc b/chrome/browser/views/accelerator_table_gtk.cc index 4a24493..25fbac7 100644 --- a/chrome/browser/views/accelerator_table_gtk.cc +++ b/chrome/browser/views/accelerator_table_gtk.cc @@ -98,7 +98,7 @@ const AcceleratorMapping kAcceleratorMap[] = { { base::VKEY_F5, true, false, false, IDC_RELOAD }, // Miscellany. - { base::VKEY_D, false, true, false, IDC_STAR }, + { base::VKEY_D, false, true, false, IDC_BOOKMARK_PAGE }, { base::VKEY_B, false, true, false, IDC_SHOW_BOOKMARK_BAR }, { base::VKEY_H, false, true, false, IDC_SHOW_HISTORY }, { base::VKEY_J, false, true, false, IDC_SHOW_DOWNLOADS }, diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc index aedb65b..ef460c4 100644 --- a/chrome/browser/views/toolbar_view.cc +++ b/chrome/browser/views/toolbar_view.cc @@ -167,7 +167,7 @@ ToolbarView::ToolbarView(Browser* browser) browser_->command_updater()->AddCommandObserver(IDC_FORWARD, this); browser_->command_updater()->AddCommandObserver(IDC_RELOAD, this); browser_->command_updater()->AddCommandObserver(IDC_HOME, this); - browser_->command_updater()->AddCommandObserver(IDC_STAR, this); + browser_->command_updater()->AddCommandObserver(IDC_BOOKMARK_PAGE, this); if (browser->type() == Browser::TYPE_NORMAL) display_mode_ = DISPLAYMODE_NORMAL; else @@ -329,7 +329,7 @@ void ToolbarView::EnabledStateChangedForCommand(int id, bool enabled) { case IDC_HOME: button = home_; break; - case IDC_STAR: + case IDC_BOOKMARK_PAGE: button = star_; break; } @@ -831,7 +831,7 @@ void ToolbarView::CreateLeftSideControls() { void ToolbarView::CreateCenterStack(Profile *profile) { star_ = new ToolbarStarToggle(this, this); - star_->set_tag(IDC_STAR); + star_->set_tag(IDC_BOOKMARK_PAGE); star_->SetDragController(this); star_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_STAR)); star_->SetToggledTooltipText(l10n_util::GetString(IDS_TOOLTIP_STARRED)); diff --git a/chrome/test/automated_ui_tests/automated_ui_tests.cc b/chrome/test/automated_ui_tests/automated_ui_tests.cc index fd22151..5c5ce9b 100644 --- a/chrome/test/automated_ui_tests/automated_ui_tests.cc +++ b/chrome/test/automated_ui_tests/automated_ui_tests.cc @@ -519,7 +519,7 @@ bool AutomatedUITest::PressUpArrow() { } bool AutomatedUITest::StarPage() { - return RunCommandAsync(IDC_STAR); + return RunCommandAsync(IDC_BOOKMARK_PAGE); } bool AutomatedUITest::ViewSource() { |