summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsky@google.com <sky@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2008-11-13 23:09:49 +0000
committersky@google.com <sky@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2008-11-13 23:09:49 +0000
commitc67b85c499e7b85007d32bac2ee9b01493825e44 (patch)
tree60a19176aad767036f9cc91a8e9cb0898c66e5b3
parent7a64bcecf1febf468f3eaa39695a11e72d24b5dc (diff)
downloadchromium_src-c67b85c499e7b85007d32bac2ee9b01493825e44.zip
chromium_src-c67b85c499e7b85007d32bac2ee9b01493825e44.tar.gz
chromium_src-c67b85c499e7b85007d32bac2ee9b01493825e44.tar.bz2
Three menu bugs:
. If a context menu was shown from a menu, then hidden we did not keep mouse capture and bad things happened (menu wouldn't go away when clicking else where, crash ...). . We only update menu sizes on first show of a menu. That way if a context menu doesn't have icons things don't shift around. . If nothing was selected in the menu pressing the context menu showed the context menu for the root. It doesn't make sense for the root menu to have a context menu. BUG=4364 TEST=see bug Review URL: http://codereview.chromium.org/10706 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5397 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/views/bookmark_bar_view_test.cc68
-rw-r--r--chrome/views/chrome_menu.cc11
-rw-r--r--chrome/views/chrome_menu.h3
3 files changed, 76 insertions, 6 deletions
diff --git a/chrome/browser/views/bookmark_bar_view_test.cc b/chrome/browser/views/bookmark_bar_view_test.cc
index c97bf3b..94d9a9c 100644
--- a/chrome/browser/views/bookmark_bar_view_test.cc
+++ b/chrome/browser/views/bookmark_bar_view_test.cc
@@ -824,3 +824,71 @@ class BookmarkBarViewTest10 : public BookmarkBarViewEventTestBase {
};
VIEW_TEST(BookmarkBarViewTest10, KeyEvents)
+
+// Make sure the menu closes with the following sequence: show menu, show
+// context menu, close context menu (via escape), then click else where. This
+// effectively verifies we maintain mouse capture after the context menu is
+// hidden.
+class BookmarkBarViewTest11 : public BookmarkBarViewEventTestBase {
+ protected:
+ virtual void DoTestOnMessageLoop() {
+ // Move the mouse to the first folder on the bookmark bar and press the
+ // mouse.
+ views::TextButton* button = bb_view_->other_bookmarked_button();
+ ui_controls::MoveMouseToCenterAndPress(button, ui_controls::LEFT,
+ ui_controls::DOWN | ui_controls::UP,
+ CreateEventTask(this, &BookmarkBarViewTest11::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(0);
+ ASSERT_TRUE(child_menu != NULL);
+
+ // Right click on the first child to get its context menu.
+ ui_controls::MoveMouseToCenterAndPress(child_menu, ui_controls::RIGHT,
+ ui_controls::DOWN | ui_controls::UP,
+ CreateEventTask(this, &BookmarkBarViewTest11::Step3));
+ }
+
+ void Step3() {
+ // Send escape so that the context menu hides.
+ ui_controls::SendKeyPressNotifyWhenDone(VK_ESCAPE, false, false, false,
+ CreateEventTask(this, &BookmarkBarViewTest11::Step4));
+ }
+
+ void Step4() {
+ // Make sure the context menu is no longer showing.
+ views::MenuItemView* menu = bb_view_->GetContextMenu();
+ ASSERT_TRUE(!menu || !menu->GetSubmenu() ||
+ !menu->GetSubmenu()->IsShowing());
+
+ // But the menu should be showing.
+ menu = bb_view_->GetMenu();
+ ASSERT_TRUE(menu && menu->GetSubmenu() && menu->GetSubmenu()->IsShowing());
+
+ // Now click on empty space.
+ gfx::Point mouse_loc;
+ views::View::ConvertPointToScreen(bb_view_, &mouse_loc);
+ ui_controls::SendMouseMove(mouse_loc.x(), mouse_loc.y());
+ ui_controls::SendMouseEventsNotifyWhenDone(
+ ui_controls::LEFT, ui_controls::UP | ui_controls::DOWN,
+ CreateEventTask(this, &BookmarkBarViewTest11::Step5));
+ }
+
+ void Step5() {
+ // Make sure the menu is not showing.
+ views::MenuItemView* menu = bb_view_->GetMenu();
+ ASSERT_TRUE(!menu || !menu->GetSubmenu() ||
+ !menu->GetSubmenu()->IsShowing());
+ Done();
+ }
+};
+
+VIEW_TEST(BookmarkBarViewTest11, CloseMenuAfterClosingContextMenu)
diff --git a/chrome/views/chrome_menu.cc b/chrome/views/chrome_menu.cc
index 959c442..d1f4eed 100644
--- a/chrome/views/chrome_menu.cc
+++ b/chrome/views/chrome_menu.cc
@@ -1329,7 +1329,11 @@ void MenuItemView::PrepareForRun(bool has_mnemonics) {
AddEmptyMenus();
- UpdateMenuPartSizes(has_icons_);
+ if (!MenuController::GetActiveInstance()) {
+ // Only update the menu size if there are no menus showing, otherwise
+ // things may shift around.
+ UpdateMenuPartSizes(has_icons_);
+ }
font_ = GetMenuFont();
@@ -1578,8 +1582,6 @@ MenuItemView* MenuController::Run(HWND parent,
pending_state_.monitor_bounds = gfx::Rect(mi.rcMonitor);
}
- did_capture_ = false;
-
any_menu_contains_mouse_ = false;
// Set the selection, which opens the initial menu.
@@ -1624,6 +1626,7 @@ MenuItemView* MenuController::Run(HWND parent,
menu_stack_.pop_back();
} else {
showing_ = false;
+ did_capture_ = false;
}
MenuItemView* result = result_;
@@ -2045,7 +2048,7 @@ bool MenuController::Dispatch(const MSG& msg) {
switch (msg.message) {
case WM_CONTEXTMENU: {
MenuItemView* item = pending_state_.item;
- if (item) {
+ if (item && item->GetRootMenuItem() != item) {
gfx::Point screen_loc(0, item->height());
View::ConvertPointToScreen(item, &screen_loc);
item->GetDelegate()->ShowContextMenu(
diff --git a/chrome/views/chrome_menu.h b/chrome/views/chrome_menu.h
index ec31e35..a92a45c 100644
--- a/chrome/views/chrome_menu.h
+++ b/chrome/views/chrome_menu.h
@@ -635,7 +635,7 @@ class MenuController : public MessageLoopForUI::Dispatcher {
bool update_immediately);
// Cancels the current Run. If all is true, any nested loops are canceled
- // as well. This immediatley hides all menus.
+ // as well. This immediately hides all menus.
void Cancel(bool all);
// An alternative to Cancel(true) that can be used with a OneShotTimer.
@@ -949,4 +949,3 @@ class MenuController : public MessageLoopForUI::Dispatcher {
} // namespace views
#endif // CHROME_VIEWS_CHROME_MENU_H__
-