From d1ba5b511c94fb02b79ceae67a172d6bdb716e98 Mon Sep 17 00:00:00 2001 From: "sky@chromium.org" Date: Thu, 28 Jan 2010 19:36:46 +0000 Subject: Makes it so deleting a bookmark from the context menu doesn't close the bookmark menu. BUG=2469 TEST=click on a bookmark folder on the bookmark bar, right click on an item and chose delete. Make sure the folder stays up and still works correctly (and the item you deleted isn't there). Review URL: http://codereview.chromium.org/551178 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@37424 0039d316-1c4b-4281-b951-d872f2087c98 --- .../bookmarks/bookmark_context_menu_controller.cc | 20 ++++-- .../bookmarks/bookmark_context_menu_controller.h | 7 ++ chrome/browser/views/bookmark_bar_view_test.cc | 78 ++++++++++++++++++++- chrome/browser/views/bookmark_context_menu.cc | 18 ++++- chrome/browser/views/bookmark_context_menu.h | 21 ++++++ .../views/bookmark_menu_controller_views.cc | 81 +++++++++++++++++++++- .../browser/views/bookmark_menu_controller_views.h | 23 +++++- views/controls/menu/menu_controller.cc | 70 +++++++++++++------ views/controls/menu/menu_controller.h | 24 ++++++- views/controls/menu/menu_delegate.h | 9 +++ views/controls/menu/menu_item_view.cc | 14 ++++ views/controls/menu/menu_item_view.h | 8 ++- views/controls/menu/submenu_view.cc | 4 ++ views/controls/menu/submenu_view.h | 3 + 14 files changed, 342 insertions(+), 38 deletions(-) diff --git a/chrome/browser/bookmarks/bookmark_context_menu_controller.cc b/chrome/browser/bookmarks/bookmark_context_menu_controller.cc index a46110b..0ce5386 100644 --- a/chrome/browser/bookmarks/bookmark_context_menu_controller.cc +++ b/chrome/browser/bookmarks/bookmark_context_menu_controller.cc @@ -278,6 +278,8 @@ void BookmarkContextMenuController::BuildMenu() { } void BookmarkContextMenuController::ExecuteCommand(int id) { + BookmarkModel* model = RemoveModelObserver(); + switch (id) { case IDS_BOOMARK_BAR_OPEN_ALL: case IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO: @@ -327,12 +329,13 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { case IDS_BOOKMARK_BAR_REMOVE: { UserMetrics::RecordAction("BookmarkBar_ContextMenu_Remove", profile_); - BookmarkModel* model = RemoveModelObserver(); + delegate_->WillRemoveBookmarks(selection_); for (size_t i = 0; i < selection_.size(); ++i) { model->Remove(selection_[i]->GetParent(), selection_[i]->GetParent()->IndexOfChild(selection_[i])); } + delegate_->DidRemoveBookmarks(); selection_.clear(); break; } @@ -387,13 +390,17 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { case IDS_BOOKMARK_MANAGER_SORT: UserMetrics::RecordAction("BookmarkManager_Sort", profile_); - model_->SortChildren(parent_); + model->SortChildren(parent_); break; - case IDS_COPY: case IDS_CUT: - bookmark_utils::CopyToClipboard(profile_->GetBookmarkModel(), - selection_, id == IDS_CUT); + delegate_->WillRemoveBookmarks(selection_); + bookmark_utils::CopyToClipboard(model, selection_, false); + delegate_->DidRemoveBookmarks(); + break; + + case IDS_COPY: + bookmark_utils::CopyToClipboard(model, selection_, false); break; case IDS_PASTE: { @@ -405,8 +412,7 @@ void BookmarkContextMenuController::ExecuteCommand(int id) { parent_->IndexOfChild(selection_[0]) : -1; if (index != -1) index++; - bookmark_utils::PasteFromClipboard(profile_->GetBookmarkModel(), - parent_, index); + bookmark_utils::PasteFromClipboard(model, parent_, index); break; } diff --git a/chrome/browser/bookmarks/bookmark_context_menu_controller.h b/chrome/browser/bookmarks/bookmark_context_menu_controller.h index 0934b70..3812e28 100644 --- a/chrome/browser/bookmarks/bookmark_context_menu_controller.h +++ b/chrome/browser/bookmarks/bookmark_context_menu_controller.h @@ -29,6 +29,13 @@ class BookmarkContextMenuControllerDelegate { virtual void AddItemWithStringId(int command_id, int string_id) = 0; virtual void AddSeparator() = 0; virtual void AddCheckboxItem(int command_id) = 0; + + // Sent before bookmarks are removed. + virtual void WillRemoveBookmarks( + const std::vector& bookmarks) {} + + // Sent after bookmarks have been removed. + virtual void DidRemoveBookmarks() {} }; // BookmarkContextMenuController creates and manages state for the context menu diff --git a/chrome/browser/views/bookmark_bar_view_test.cc b/chrome/browser/views/bookmark_bar_view_test.cc index 8e24366..3d84ff8 100644 --- a/chrome/browser/views/bookmark_bar_view_test.cc +++ b/chrome/browser/views/bookmark_bar_view_test.cc @@ -14,6 +14,7 @@ #include "chrome/common/pref_service.h" #include "chrome/test/testing_profile.h" #include "chrome/test/interactive_ui/view_event_test_base.h" +#include "grit/generated_resources.h" #include "views/controls/button/menu_button.h" #include "views/controls/button/text_button.h" #include "views/controls/menu/menu_controller.h" @@ -38,8 +39,8 @@ class TestingPageNavigator : public PageNavigator { } // namespace // Base class for event generating bookmark view tests. These test are intended -// to exercise ChromeMenus, but that's easier done with BookmarkBarView rather -// than ChromeMenu itself. +// to exercise View's menus, but that's easier done with BookmarkBarView rather +// than View's menu itself. // // SetUp creates a bookmark model with the following structure. // All folders are in upper case, all URLs in lower case. @@ -1101,3 +1102,76 @@ class BookmarkBarViewTest14 : public BookmarkBarViewEventTestBase { }; VIEW_TEST(BookmarkBarViewTest14, ContextMenus2) + +// Makes sure deleting from the context menu keeps the bookmark menu showing. +class BookmarkBarViewTest15 : public BookmarkBarViewEventTestBase { + public: + BookmarkBarViewTest15() : deleted_menu_id_(0) {} + + protected: + virtual void DoTestOnMessageLoop() { + // Show the other bookmarks. + views::TextButton* button = bb_view_->other_bookmarked_button(); + ui_controls::MoveMouseToCenterAndPress(button, ui_controls::LEFT, + ui_controls::DOWN | ui_controls::UP, + CreateEventTask(this, &BookmarkBarViewTest15::Step2)); + } + + private: + void Step2() { + // Menu should be showing. + views::MenuItemView* menu = bb_view_->GetMenu(); + ASSERT_TRUE(menu != NULL); + ASSERT_TRUE(menu->GetSubmenu()->IsShowing()); + + views::MenuItemView* child_menu = + menu->GetSubmenu()->GetMenuItemAt(1); + ASSERT_TRUE(child_menu != NULL); + + deleted_menu_id_ = child_menu->GetCommand(); + + // Right click on the second child to get its context menu. + ui_controls::MoveMouseToCenterAndPress(child_menu, ui_controls::RIGHT, + ui_controls::DOWN | ui_controls::UP, + CreateEventTask(this, &BookmarkBarViewTest15::Step3)); + } + + void Step3() { + // Make sure the context menu is showing. + views::MenuItemView* menu = bb_view_->GetContextMenu(); + ASSERT_TRUE(menu != NULL); + ASSERT_TRUE(menu->GetSubmenu()); + ASSERT_TRUE(menu->GetSubmenu()->IsShowing()); + + views::MenuItemView* delete_menu = + menu->GetMenuItemByID(IDS_BOOKMARK_BAR_REMOVE); + ASSERT_TRUE(delete_menu); + + // Click on the delete button. + ui_controls::MoveMouseToCenterAndPress(delete_menu, + ui_controls::LEFT, ui_controls::DOWN | ui_controls::UP, + CreateEventTask(this, &BookmarkBarViewTest15::Step4)); + } + + void Step4() { + // The context menu should not be showing. + views::MenuItemView* context_menu = bb_view_->GetContextMenu(); + ASSERT_TRUE(context_menu == NULL); + + // But the menu should be showing. + views::MenuItemView* menu = bb_view_->GetMenu(); + ASSERT_TRUE(menu != NULL); + ASSERT_TRUE(menu->GetSubmenu()->IsShowing()); + + // And the deleted_menu_id_ should have been removed. + ASSERT_TRUE(menu->GetMenuItemByID(deleted_menu_id_) == NULL); + + bb_view_->GetMenu()->GetMenuController()->Cancel(true); + + Done(); + } + + int deleted_menu_id_; +}; + +VIEW_TEST(BookmarkBarViewTest15, MenuStaysVisibleAfterDelete) diff --git a/chrome/browser/views/bookmark_context_menu.cc b/chrome/browser/views/bookmark_context_menu.cc index a3f3a15..21eb200 100644 --- a/chrome/browser/views/bookmark_context_menu.cc +++ b/chrome/browser/views/bookmark_context_menu.cc @@ -25,7 +25,8 @@ BookmarkContextMenu::BookmarkContextMenu( parent, selection, configuration))), parent_window_(parent_window), - ALLOW_THIS_IN_INITIALIZER_LIST(menu_(new views::MenuItemView(this))) { + ALLOW_THIS_IN_INITIALIZER_LIST(menu_(new views::MenuItemView(this))), + observer_(NULL) { controller_->BuildMenu(); } @@ -56,6 +57,10 @@ bool BookmarkContextMenu::IsCommandEnabled(int command_id) const { return controller_->IsCommandEnabled(command_id); } +bool BookmarkContextMenu::ShouldCloseAllMenusOnExecute(int id) { + return id != IDS_BOOKMARK_BAR_REMOVE; +} + //////////////////////////////////////////////////////////////////////////////// // BookmarkContextMenu, BookmarkContextMenuControllerDelegate implementation: @@ -79,3 +84,14 @@ void BookmarkContextMenu::AddCheckboxItem(int command_id) { menu_->AppendMenuItem(command_id, l10n_util::GetString(command_id), views::MenuItemView::CHECKBOX); } + +void BookmarkContextMenu::WillRemoveBookmarks( + const std::vector& bookmarks) { + if (observer_) + observer_->WillRemoveBookmarks(bookmarks); +} + +void BookmarkContextMenu::DidRemoveBookmarks() { + if (observer_) + observer_->DidRemoveBookmarks(); +} diff --git a/chrome/browser/views/bookmark_context_menu.h b/chrome/browser/views/bookmark_context_menu.h index 94c2d97..47b8ade 100644 --- a/chrome/browser/views/bookmark_context_menu.h +++ b/chrome/browser/views/bookmark_context_menu.h @@ -8,6 +8,17 @@ #include "chrome/browser/bookmarks/bookmark_context_menu_controller.h" #include "views/controls/menu/menu_delegate.h" +// Observer for the BookmarkContextMenu. +class BookmarkContextMenuObserver { + public: + // Invoked before the specified items are removed from the bookmark model. + virtual void WillRemoveBookmarks( + const std::vector& bookmarks) = 0; + + // Invoked after the items have been removed from the model. + virtual void DidRemoveBookmarks() = 0; +}; + class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, public views::MenuDelegate { public: @@ -25,10 +36,15 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, views::MenuItemView* menu() const { return menu_.get(); } + void set_observer(BookmarkContextMenuObserver* observer) { + observer_ = observer; + } + // Overridden from views::MenuDelegate: virtual void ExecuteCommand(int command_id); virtual bool IsItemChecked(int command_id) const; virtual bool IsCommandEnabled(int command_id) const; + virtual bool ShouldCloseAllMenusOnExecute(int id); // Overridden from BookmarkContextMenuControllerDelegate: virtual void CloseMenu(); @@ -36,6 +52,9 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, virtual void AddItemWithStringId(int command_id, int string_id); virtual void AddSeparator(); virtual void AddCheckboxItem(int command_id); + virtual void WillRemoveBookmarks( + const std::vector& bookmarks); + virtual void DidRemoveBookmarks(); private: scoped_ptr controller_; @@ -46,6 +65,8 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, // The menu itself. scoped_ptr menu_; + BookmarkContextMenuObserver* observer_; + DISALLOW_COPY_AND_ASSIGN(BookmarkContextMenu); }; diff --git a/chrome/browser/views/bookmark_menu_controller_views.cc b/chrome/browser/views/bookmark_menu_controller_views.cc index 5a7256b..cb334ed 100644 --- a/chrome/browser/views/bookmark_menu_controller_views.cc +++ b/chrome/browser/views/bookmark_menu_controller_views.cc @@ -206,6 +206,7 @@ bool BookmarkMenuController::ShowContextMenu(MenuItemView* source, nodes[0]->GetParent(), nodes, BookmarkContextMenuController::BOOKMARK_BAR)); + context_menu_->set_observer(this); context_menu_->RunMenuAt(gfx::Point(x, y)); context_menu_.reset(NULL); return true; @@ -271,8 +272,7 @@ void BookmarkMenuController::BookmarkModelChanged() { void BookmarkMenuController::BookmarkNodeFavIconLoaded( BookmarkModel* model, const BookmarkNode* node) { - std::map::iterator menu_pair = - node_to_menu_id_map_.find(node); + NodeToMenuIDMap::iterator menu_pair = node_to_menu_id_map_.find(node); if (menu_pair == node_to_menu_id_map_.end()) return; // We're not showing a menu item for the node. @@ -287,6 +287,19 @@ void BookmarkMenuController::BookmarkNodeFavIconLoaded( } } +void BookmarkMenuController::WillRemoveBookmarks( + const std::vector& bookmarks) { + std::set removed_menus; + + WillRemoveBookmarksImpl(bookmarks, &removed_menus); + + STLDeleteElements(&removed_menus); +} + +void BookmarkMenuController::DidRemoveBookmarks() { + profile_->GetBookmarkModel()->AddObserver(this); +} + MenuItemView* BookmarkMenuController::CreateMenu(const BookmarkNode* parent, int start_child_index) { MenuItemView* menu = new MenuItemView(this); @@ -337,6 +350,7 @@ void BookmarkMenuController::BuildMenu(const BookmarkNode* parent, GetBitmapNamed(IDR_BOOKMARK_BAR_FOLDER); MenuItemView* submenu = menu->AppendSubMenuWithIcon(id, node->GetTitle(), *folder_icon); + node_to_menu_id_map_[node] = id; BuildMenu(node, 0, submenu, next_menu_id); } else { NOTREACHED(); @@ -351,3 +365,66 @@ BookmarkMenuController::~BookmarkMenuController() { observer_->BookmarkMenuDeleted(this); STLDeleteValues(&node_to_menu_map_); } + +MenuItemView* BookmarkMenuController::GetMenuByID(int id) { + for (NodeToMenuMap::const_iterator i = node_to_menu_map_.begin(); + i != node_to_menu_map_.end(); ++i) { + MenuItemView* menu = i->second->GetMenuItemByID(id); + if (menu) + return menu; + } + return NULL; +} + +void BookmarkMenuController::WillRemoveBookmarksImpl( + const std::vector& bookmarks, + std::set* removed_menus) { + // Remove the observer so that when the remove happens we don't prematurely + // cancel the menu. + profile_->GetBookmarkModel()->RemoveObserver(this); + + // Remove the menu items. + std::set changed_parent_menus; + for (std::vector::const_iterator i = bookmarks.begin(); + i != bookmarks.end(); ++i) { + NodeToMenuIDMap::iterator node_to_menu = node_to_menu_id_map_.find(*i); + if (node_to_menu != node_to_menu_id_map_.end()) { + MenuItemView* menu = GetMenuByID(node_to_menu->second); + DCHECK(menu); // If there an entry in node_to_menu_id_map_, there should + // be a menu. + removed_menus->insert(menu); + changed_parent_menus.insert(menu->GetParentMenuItem()); + menu->GetParent()->RemoveChildView(menu); + node_to_menu_id_map_.erase(node_to_menu); + } + } + + // All the bookmarks in |bookmarks| should have the same parent. It's possible + // to support different parents, but this would need to prune any nodes whose + // parent has been removed. As all nodes currently have the same parent, there + // is the DCHECK. + DCHECK(changed_parent_menus.size() <= 1); + + for (std::set::const_iterator i = changed_parent_menus.begin(); + i != changed_parent_menus.end(); ++i) { + (*i)->ChildrenChanged(); + } + + // Remove any descendants of the removed nodes in node_to_menu_id_map_. + for (NodeToMenuIDMap::iterator i = node_to_menu_id_map_.begin(); + i != node_to_menu_id_map_.end(); ) { + bool ancestor_removed = false; + for (std::vector::const_iterator j = bookmarks.begin(); + j != bookmarks.end(); ++j) { + if (i->first->HasAncestor(*j)) { + ancestor_removed = true; + break; + } + } + if (ancestor_removed) { + node_to_menu_id_map_.erase(i++); + } else { + ++i; + } + } +} diff --git a/chrome/browser/views/bookmark_menu_controller_views.h b/chrome/browser/views/bookmark_menu_controller_views.h index f19c3cc..13e1295 100644 --- a/chrome/browser/views/bookmark_menu_controller_views.h +++ b/chrome/browser/views/bookmark_menu_controller_views.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_VIEWS_BOOKMARK_MENU_CONTROLLER_VIEWS_H_ #include +#include #include "app/gfx/native_widget_types.h" #include "chrome/browser/bookmarks/base_bookmark_model_observer.h" @@ -35,7 +36,8 @@ class Profile; // BookmarkMenuController deletes itself as necessary, although the menu can // be explicitly hidden by way of the Cancel method. class BookmarkMenuController : public BaseBookmarkModelObserver, - public views::MenuDelegate { + public views::MenuDelegate, + public BookmarkContextMenuObserver { public: // The observer is notified prior to the menu being deleted. class Observer { @@ -112,7 +114,14 @@ class BookmarkMenuController : public BaseBookmarkModelObserver, virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model, const BookmarkNode* node); + // BookmarkContextMenu::Observer methods. + virtual void WillRemoveBookmarks( + const std::vector& bookmarks); + virtual void DidRemoveBookmarks(); + private: + typedef std::map NodeToMenuIDMap; + // BookmarkMenuController deletes itself as necessary. ~BookmarkMenuController(); @@ -132,6 +141,16 @@ class BookmarkMenuController : public BaseBookmarkModelObserver, views::MenuItemView* menu, int* next_menu_id); + // Returns the menu whose id is |id|. + views::MenuItemView* GetMenuByID(int id); + + // Does the work of processing WillRemoveBookmarks. On exit the set of removed + // menus is added to |removed_menus|. It's up to the caller to delete the + // the menus added to |removed_menus|. + void WillRemoveBookmarksImpl( + const std::vector& bookmarks, + std::set* removed_menus); + Browser* browser_; Profile* profile_; @@ -149,7 +168,7 @@ class BookmarkMenuController : public BaseBookmarkModelObserver, // Mapping from node to menu id. This only contains entries for nodes of type // URL. - std::map node_to_menu_id_map_; + NodeToMenuIDMap node_to_menu_id_map_; // Current menu. views::MenuItemView* menu_; diff --git a/views/controls/menu/menu_controller.cc b/views/controls/menu/menu_controller.cc index ef33bc7..fb35908 100644 --- a/views/controls/menu/menu_controller.cc +++ b/views/controls/menu/menu_controller.cc @@ -149,7 +149,7 @@ MenuItemView* MenuController::Run(gfx::NativeWindow parent, const gfx::Rect& bounds, MenuItemView::AnchorPosition position, int* result_mouse_event_flags) { - exit_all_ = false; + exit_type_ = EXIT_NONE; possible_drag_ = false; bool nested_menu = showing_; @@ -228,15 +228,18 @@ MenuItemView* MenuController::Run(gfx::NativeWindow parent, if (result_mouse_event_flags) *result_mouse_event_flags = result_mouse_event_flags_; - if (nested_menu && result) { - // We're nested and about to return a value. The caller might enter another - // blocking loop. We need to make sure all menus are hidden before that - // happens otherwise the menus will stay on screen. - CloseAllNestedMenus(); - - // Set exit_all_ to true, which makes sure all nested loops exit - // immediately. - exit_all_ = true; + if (exit_type_ == EXIT_OUTERMOST) { + exit_type_ = EXIT_NONE; + } else { + if (nested_menu && result) { + // We're nested and about to return a value. The caller might enter + // another blocking loop. We need to make sure all menus are hidden + // before that happens otherwise the menus will stay on screen. + CloseAllNestedMenus(); + + // Set exit_all_, which makes sure all nested loops exit immediately. + exit_type_ = EXIT_ALL; + } } if (menu_button_) { @@ -291,7 +294,7 @@ void MenuController::Cancel(bool all) { } MenuItemView* selected = state_.item; - exit_all_ = all; + exit_type_ = all ? EXIT_ALL : EXIT_OUTERMOST; // Hide windows immediately. SetSelection(NULL, false, true); @@ -605,7 +608,7 @@ int MenuController::OnPerformDrop(SubmenuView* source, // Set state such that we exit. showing_ = false; - exit_all_ = true; + exit_type_ = EXIT_ALL; if (!IsBlockingRun()) item->GetRootMenuItem()->DropMenuClosed(false); @@ -648,7 +651,7 @@ void MenuController::SetActiveInstance(MenuController* controller) { bool MenuController::Dispatch(const MSG& msg) { DCHECK(blocking_run_); - if (exit_all_) { + if (exit_type_ == EXIT_ALL) { // We must translate/dispatch the message here, otherwise we would drop // the message on the floor. TranslateMessage(&msg); @@ -697,14 +700,14 @@ bool MenuController::Dispatch(const MSG& msg) { } TranslateMessage(&msg); DispatchMessage(&msg); - return !exit_all_; + return exit_type_ == EXIT_NONE; } #else bool MenuController::Dispatch(GdkEvent* event) { gtk_main_do_event(event); - if (exit_all_) + if (exit_type_ == EXIT_ALL) return false; switch (event->type) { @@ -721,7 +724,7 @@ bool MenuController::Dispatch(GdkEvent* event) { break; } - return !exit_all_; + return exit_type_ == EXIT_NONE; } #endif @@ -799,7 +802,7 @@ bool MenuController::OnKeyDown(int key_code MenuController::MenuController(bool blocking) : blocking_run_(blocking), showing_(false), - exit_all_(false), + exit_type_(EXIT_NONE), did_capture_(false), result_(NULL), result_mouse_event_flags_(0), @@ -845,7 +848,12 @@ void MenuController::UpdateInitialLocation( void MenuController::Accept(MenuItemView* item, int mouse_event_flags) { DCHECK(IsBlockingRun()); result_ = item; - exit_all_ = true; + if (item && !menu_stack_.empty() && + !item->GetDelegate()->ShouldCloseAllMenusOnExecute(item->GetCommand())) { + exit_type_ = EXIT_OUTERMOST; + } else { + exit_type_ = EXIT_ALL; + } result_mouse_event_flags_ = mouse_event_flags; } @@ -1114,6 +1122,11 @@ void MenuController::OpenMenu(MenuItemView* item) { return; } + OpenMenuImpl(item, true); + did_capture_ = true; +} + +void MenuController::OpenMenuImpl(MenuItemView* item, bool show) { bool prefer_leading = state_.open_leading.empty() ? true : state_.open_leading.back(); bool resulting_direction; @@ -1122,9 +1135,26 @@ void MenuController::OpenMenu(MenuItemView* item) { state_.open_leading.push_back(resulting_direction); bool do_capture = (!did_capture_ && blocking_run_); showing_submenu_ = true; - item->GetSubmenu()->ShowAt(owner_, bounds, do_capture); + if (show) + item->GetSubmenu()->ShowAt(owner_, bounds, do_capture); + else + item->GetSubmenu()->Reposition(bounds); showing_submenu_ = false; - did_capture_ = true; +} + +void MenuController::MenuChildrenChanged(MenuItemView* item) { + DCHECK(item); + DCHECK(item->GetSubmenu()->IsShowing()); + + // Currently this only supports adjusting the bounds of the last menu. + DCHECK(item == state_.item->GetParentMenuItem()); + + // Make sure the submenu isn't showing for the current item (the position may + // have changed or the menu removed). This also moves the selection back to + // the parent, which handles the case where the selected item was removed. + SetSelection(state_.item->GetParentMenuItem(), true, true); + + OpenMenuImpl(item, false); } void MenuController::BuildPathsAndCalculateDiff( diff --git a/views/controls/menu/menu_controller.h b/views/controls/menu/menu_controller.h index 2ef1bb7..c92f4b9 100644 --- a/views/controls/menu/menu_controller.h +++ b/views/controls/menu/menu_controller.h @@ -98,6 +98,18 @@ class MenuController : public MessageLoopForUI::Dispatcher { void OnDragExitedScrollButton(SubmenuView* source); private: + // Enumeration of how the menu should exit. + enum ExitType { + // Don't exit. + EXIT_NONE, + + // All menus, including nested, should be exited. + EXIT_ALL, + + // Only the outermost menu should be exited. + EXIT_OUTERMOST + }; + class MenuScrollTask; // Tracks selection information. @@ -243,6 +255,14 @@ class MenuController : public MessageLoopForUI::Dispatcher { // in anyway. void OpenMenu(MenuItemView* item); + // Implementation of OpenMenu. If |show| is true, this invokes show on the + // menu, otherwise Reposition is invoked. + void OpenMenuImpl(MenuItemView* item, bool show); + + // Invoked when the children of a menu change and the menu is showing. + // This closes any submenus and resizes the submenu. + void MenuChildrenChanged(MenuItemView* item); + // Builds the paths of the two menu items into the two paths, and // sets first_diff_at to the location of the first difference between the // two paths. @@ -317,8 +337,8 @@ class MenuController : public MessageLoopForUI::Dispatcher { // If true, we're showing. bool showing_; - // If true, all nested run loops should be exited. - bool exit_all_; + // Indicates what to exit. + ExitType exit_type_; // Whether we did a capture. We do a capture only if we're blocking and // the mouse was down when Run. diff --git a/views/controls/menu/menu_delegate.h b/views/controls/menu/menu_delegate.h index cb06f21..cbb295a 100644 --- a/views/controls/menu/menu_delegate.h +++ b/views/controls/menu/menu_delegate.h @@ -84,6 +84,15 @@ class MenuDelegate : Controller { virtual void ExecuteCommand(int id) { } + // If nested menus are showing (nested menus occur when a menu shows a context + // menu) this is invoked to determine if all the menus should be closed when + // the user selects the menu with the command |id|. This returns true to + // indicate that all menus should be closed. Return false if only the + // context menu should be closed. + virtual bool ShouldCloseAllMenusOnExecute(int id) { + return true; + } + // Executes the specified command. mouse_event_flags give the flags of the // mouse event that triggered this to be invoked (views::MouseEvent // flags). mouse_event_flags is 0 if this is triggered by a user gesture diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc index 5e92186..698502b 100644 --- a/views/controls/menu/menu_item_view.cc +++ b/views/controls/menu/menu_item_view.cc @@ -233,6 +233,20 @@ MenuItemView* MenuItemView::GetMenuItemByID(int id) { return NULL; } +void MenuItemView::ChildrenChanged() { + MenuController* controller = GetMenuController(); + if (!controller) + return; // We're not showing, nothing to do. + + // Handles the case where we were empty and are no longer empty. + RemoveEmptyMenus(); + + // Handles the case where we were not empty, but now are. + AddEmptyMenus(); + + controller->MenuChildrenChanged(this); +} + MenuItemView::MenuItemView(MenuItemView* parent, int command, MenuItemView::Type type) { diff --git a/views/controls/menu/menu_item_view.h b/views/controls/menu/menu_item_view.h index c528671..936ff1e 100644 --- a/views/controls/menu/menu_item_view.h +++ b/views/controls/menu/menu_item_view.h @@ -221,6 +221,10 @@ class MenuItemView : public View { // Returns the descendant with the specified command. MenuItemView* GetMenuItemByID(int id); + // Invoke if you remove/add children to the menu while it's showing. This + // recalculates the bounds. + void ChildrenChanged(); + protected: // Creates a MenuItemView. This is used by the various AddXXX methods. MenuItemView(MenuItemView* parent, int command, Type type); @@ -255,8 +259,8 @@ class MenuItemView : public View { int GetDrawStringFlags(); // If this menu item has no children a child is added showing it has no - // children. Otherwise AddEmtpyMenuIfNecessary is recursively invoked on - // child menu items that have children. + // children. Otherwise AddEmtpyMenus is recursively invoked on child menu + // items that have children. void AddEmptyMenus(); // Undoes the work of AddEmptyMenus. diff --git a/views/controls/menu/submenu_view.cc b/views/controls/menu/submenu_view.cc index b6ca627..0170e0c 100644 --- a/views/controls/menu/submenu_view.cc +++ b/views/controls/menu/submenu_view.cc @@ -234,6 +234,10 @@ void SubmenuView::ShowAt(gfx::NativeWindow parent, host_->Init(parent, bounds, scroll_view_container_, do_capture); } +void SubmenuView::Reposition(const gfx::Rect& bounds) { + host_->SetBounds(bounds); +} + void SubmenuView::Close() { if (host_) { host_->Close(); diff --git a/views/controls/menu/submenu_view.h b/views/controls/menu/submenu_view.h index 8cc4461..1e82f73 100644 --- a/views/controls/menu/submenu_view.h +++ b/views/controls/menu/submenu_view.h @@ -76,6 +76,9 @@ class SubmenuView : public View { // coordinates. max_width gives the max width the view should be. void ShowAt(gfx::NativeWindow parent, const gfx::Rect& bounds, bool do_capture); + // Resets the bounds of the submenu to |bounds|. + void Reposition(const gfx::Rect& bounds); + // Closes the menu, destroying the host. void Close(); -- cgit v1.1