Fixed problem with some popup menus (check ticket #1679)

Moved GetMouse() calls near the check for exit conditions.
Reorganized a bit the code, and hopefully simplified it in some places.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@23229 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/os/interface/Menu.h

@@ -182,8 +182,8 @@ private:
 
 			void			_UpdateStateOpenSelect(BMenuItem* item,
 								bigtime_t& openTime, bigtime_t& closeTime);
-			void			_UpdateStateClose(BMenuItem* item,
+			void			_UpdateStateClose(BMenuItem* item, const BPoint& where,
-								const BPoint& where, const uint32& buttons);
+								const uint32& buttons);
 
 			bool			_AddItem(BMenuItem* item, int32 index);
 			bool			_RemoveItems(int32 index, int32 count,

headers/private/interface/MenuPrivate.h

@@ -8,8 +8,38 @@ enum menu_states {
 	MENU_STATE_CLOSED = 5
 };
 
-
 extern const char *kEmptyMenuLabel;
 
 
+// Note: since sqrt is slow, we don't use it and return the square of the distance
+#define square(x) ((x) * (x))
+static inline float
+point_distance(const BPoint &pointA, const BPoint &pointB)
+{
+	return square(pointA.x - pointB.x) + square(pointA.y - pointB.y);
+}
+
+/*
+static float
+point_rect_distance(const BPoint &point, const BRect &rect)
+{
+	float horizontal = 0;
+	float vertical = 0;
+	if (point.x < rect.left)
+		horizontal = rect.left - point.x;
+	else if (point.x > rect.right)
+		horizontal = point.x - rect.right;
+
+	if (point.y < rect.top)
+		vertical = rect.top - point.y;
+	else if (point.y > rect.bottom)
+		vertical = point.y - rect.bottom;
+	
+	return square(horizontal) + square(vertical);
+}
+*/
+
+#undef square
+
+
 #endif // __MENU_PRIVATE_H

headers/private/interface/MenuWindow.h

@@ -26,6 +26,8 @@ class BMenuWindow : public BWindow {
 		BMenuWindow(const char *name);
 		virtual ~BMenuWindow();
 
+		virtual void DispatchMessage(BMessage *message, BHandler *handler);
+	
 		void AttachMenu(BMenu *menu);
 		void DetachMenu();
 	

src/kits/interface/Menu.cpp

@@ -1175,7 +1175,6 @@ BMenu::Track(bool sticky, BRect *clickToOpenRect)
 	int action;
 	BMenuItem *menuItem = _Track(&action);
 
-	_SetStickyMode(false);
 	fExtraRect = NULL;
 
 	return menuItem;
@@ -1253,7 +1252,7 @@ BMenu::_InitData(BMessage* archive)
 		archive->FindFloat("_maxwidth", &fMaxContentWidth);
 
 		BMessage msg;
-       	for (int32 i = 0; archive->FindMessage("_items", i, &msg) == B_OK; i++) {
+       		for (int32 i = 0; archive->FindMessage("_items", i, &msg) == B_OK; i++) {
 			BArchivable *object = instantiate_object(&msg);
 			if (BMenuItem *item = dynamic_cast<BMenuItem *>(object)) {
 				BRect bounds;
@@ -1367,6 +1366,14 @@ BMenu::_Track(int *action, long start)
 	if (fSuper != NULL)
 		fSuper->fState = MENU_STATE_TRACKING_SUBMENU;
 
+	BPoint location;
+	uint32 buttons;
+	if (LockLooper()) {	
+		GetMouse(&location, &buttons);
+		UnlockLooper();	
+	}
+	
+	bool releasedOnce = buttons == 0;
 	while (true) {
 		if (_CustomTrackingWantsToQuit())
 			break;
@@ -1375,11 +1382,6 @@ BMenu::_Track(int *action, long start)
 		if (!locked)
 			break;
 		
-		bigtime_t snoozeAmount = 50000;
-		BPoint location;
-		uint32 buttons;
-		GetMouse(&location, &buttons, true);
-
 		BMenuWindow *window = static_cast<BMenuWindow *>(Window());
 
 		BPoint screenLocation = ConvertToScreen(location);
@@ -1387,8 +1389,11 @@ BMenu::_Track(int *action, long start)
 			item = NULL;
 		} else {
 			item = _HitTestItems(location, B_ORIGIN);
-			if (item != NULL)
+			if (item != NULL) {
 				_UpdateStateOpenSelect(item, openTime, closeTime);
+				if (!releasedOnce)
+					releasedOnce = true;			
+			}
 		}
 
 		// Track the submenu
@@ -1397,58 +1402,58 @@ BMenu::_Track(int *action, long start)
 			locked = false;
 			int submenuAction = MENU_STATE_TRACKING;
 			BMenu *submenu = fSelected->Submenu();
-			bool wasSticky = _IsStickyMode();
+			submenu->_SetStickyMode(_IsStickyMode());
-			if (wasSticky)
-				submenu->_SetStickyMode(true);
 			BMenuItem *submenuItem = submenu->_Track(&submenuAction);
-
-			// check if the user started holding down a mouse button in a submenu
-			if (wasSticky && !_IsStickyMode()) {
-				buttons = 1;
-					// buttons must have been pressed in the meantime
-			}
-
 			if (submenuAction == MENU_STATE_CLOSED) {
 				item = submenuItem;
-				fState = submenuAction;
+				fState = MENU_STATE_CLOSED;			
-				break;
 			}
-
-			locked = LockLooper();
-			if (!locked)
-				break;
 		} else if (item == NULL) {
-			if (_OverSuper(screenLocation)) {
+			if (_OverSuper(screenLocation))
 				fState = MENU_STATE_TRACKING;
-				UnlockLooper();
+			else {
-				break;
+				if (!_OverSubmenu(fSelected, screenLocation)
-			}
+					&& system_time() > closeTime + kHysteresis
+					&& fState != MENU_STATE_TRACKING_SUBMENU) {
+					_SelectItem(NULL);
+					fState = MENU_STATE_TRACKING;
+				}
 
-			if (!_OverSubmenu(fSelected, screenLocation)
+				if (fSuper != NULL) {
-				&& system_time() > closeTime + kHysteresis
+					// Give supermenu the chance to continue tracking
-				&& fState != MENU_STATE_TRACKING_SUBMENU) {
+					*action = fState;
-				_SelectItem(NULL);
+					if (locked)
-				fState = MENU_STATE_TRACKING;
+						UnlockLooper();
-			}
+					return NULL;
-
+				}
-			if (fSuper != NULL) {
-				// Give supermenu the chance to continue tracking
-				*action = fState;
-				if (locked)
-					UnlockLooper();
-
-				return NULL;
 			}
 		}
 
-		if (locked)
+		if (!locked)
-			UnlockLooper();
+			locked = LockLooper();
 	
-		_UpdateStateClose(item, location, buttons);
+		BPoint newLocation;
+		uint32 newButtons;		
+		if (locked) {
+			GetMouse(&newLocation, &newButtons, true);
+			UnlockLooper();
+			locked = false;
+		}
+
+		if (newLocation != location || newButtons != buttons) {
+			if (!releasedOnce && newButtons == 0 && buttons != 0)
+				releasedOnce = true;				
+			location = newLocation;
+			buttons = newButtons;
+		}
+		
+		if (releasedOnce)
+			_UpdateStateClose(item, location, buttons);
 		
 		if (fState == MENU_STATE_CLOSED)
 			break;
 
+		bigtime_t snoozeAmount = 50000;
 		snooze(snoozeAmount);
 	}
 
@@ -1460,9 +1465,6 @@ BMenu::_Track(int *action, long start)
 		UnlockLooper();
 	}
 
-	if (_IsStickyMode())
-		_SetStickyMode(false);
-
 	// delete the menu window recycled for all the child menus
 	_DeleteMenuWindow();
 
@@ -1503,17 +1505,14 @@ BMenu::_UpdateStateClose(BMenuItem* item, const BPoint& where,
 	if (buttons != 0 && _IsStickyMode()) {
 		if (item == NULL)
 			fState = MENU_STATE_CLOSED;
-		else {
+		else
-			BMenu *supermenu = Supermenu();
-			for(; supermenu; supermenu = supermenu->Supermenu())
-				supermenu->_SetStickyMode(false);
 			_SetStickyMode(false);
-		}
 	} else if (buttons == 0 && !_IsStickyMode()) {
 		if (fExtraRect != NULL && fExtraRect->Contains(where)) {
 			_SetStickyMode(true);
 			fExtraRect = NULL;
-				// This code should be executed only once
+				// Setting this to NULL will prevent this code
+				// to be executed next time
 		} else
 			fState = MENU_STATE_CLOSED;
 	}
@@ -2159,7 +2158,16 @@ BMenu::_SetIgnoreHidden(bool on)
 void
 BMenu::_SetStickyMode(bool on)
 {
-	if (fStickyMode != on) {
+	if (fStickyMode == on)
+		return;
+	
+	fStickyMode = on;
+
+	// If we are switching to sticky mode, propagate the status
+	// back to the super menu
+	if (fSuper != NULL)
+		fSuper->_SetStickyMode(on);
+	else {
 		// TODO: Ugly hack, but it needs to be done right here in this method
 		BMenuBar *menuBar = dynamic_cast<BMenuBar *>(this);
 		if (on && menuBar != NULL && menuBar->LockLooper()) {
@@ -2168,14 +2176,7 @@ BMenu::_SetStickyMode(bool on)
 			menuBar->_StealFocus();
 			menuBar->UnlockLooper();
 		}
-
-		fStickyMode = on;
 	}
-
-	// If we are switching to sticky mode, propagate the status
-	// back to the super menu
-	if (on && fSuper != NULL)
-		fSuper->_SetStickyMode(on);
 }
 
 
@@ -2222,22 +2223,20 @@ BMenu::_ChooseTrigger(const char *title, int32& index, uint32& trigger,
 
 	// two runs: first we look out for uppercase letters
 	// TODO: support Unicode characters correctly!
-
 	for (uint32 i = 0; (c = title[i]) != '\0'; i++) {
 		if (!IsInsideGlyph(c) && isupper(c) && !triggers.HasTrigger(c)) {
 			index = i;
 			trigger = tolower(c);
-			return triggers.AddTrigger(c);;
+			return triggers.AddTrigger(c);
 		}
 	}
 
 	// then, if we still haven't found anything, we accept them all
-
 	index = 0;
 	while ((c = UTF8ToCharCode(&title)) != 0) {
 		if (!isspace(c) && !triggers.HasTrigger(c)) {
 			trigger = tolower(c);
-			return triggers.AddTrigger(c);;
+			return triggers.AddTrigger(c);
 		}
 
 		index++;

src/kits/interface/MenuBar.cpp

@@ -446,17 +446,6 @@ BMenuBar::_TrackTask(void *arg)
 }
 
 
-// Note: since sqrt is slow, we don't use it and return the square of the distance
-// TODO: Move this to some common place, could be used in BMenu too.
-#define square(x) ((x) * (x))
-static float
-point_distance(const BPoint &pointA, const BPoint &pointB)
-{
-	return square(pointA.x - pointB.x) + square(pointA.y - pointB.y);
-}
-#undef square
-
-
 BMenuItem *
 BMenuBar::_Track(int32 *action, int32 startIndex, bool showMenu)
 {	
@@ -466,12 +455,15 @@ BMenuBar::_Track(int32 *action, int32 startIndex, bool showMenu)
 	BWindow *window = Window();
 	fState = MENU_STATE_TRACKING;
 
-	if (startIndex != -1) {
+	BPoint where;
-		be_app->ObscureCursor();
+	uint32 buttons;
-		if (window->Lock()) {
+	if (window->Lock()) {
+		if (startIndex != -1) {
+			be_app->ObscureCursor();		
 			_SelectItem(ItemAt(startIndex), true, true);
-			window->Unlock();
 		}
+		GetMouse(&where, &buttons);
+		window->Unlock();
 	}
 	
 	while (true) {
@@ -480,10 +472,6 @@ BMenuBar::_Track(int32 *action, int32 startIndex, bool showMenu)
 		if (!locked)
 			break;
 
-		BPoint where;
-		uint32 buttons;
-		GetMouse(&where, &buttons, true);
-
 		BMenuItem *menuItem = _HitTestItems(where, B_ORIGIN);
 		if (menuItem != NULL) {
 			// Select item if:
@@ -520,35 +508,26 @@ BMenuBar::_Track(int32 *action, int32 startIndex, bool showMenu)
 			locked = false;
 			snoozeAmount = 30000;
 			bool wasSticky = _IsStickyMode();
-			if (wasSticky)
+			menu->_SetStickyMode(wasSticky);
-				menu->_SetStickyMode(true);
 			int localAction;
 			fChosenItem = menu->_Track(&localAction);
-			if (menu->State(NULL) == MENU_STATE_TRACKING
-				&& menu->_IsStickyMode())
-				menu->_SetStickyMode(false);
 
-			// check if the user started holding down a mouse button in a submenu
+			// The mouse could have meen moved since the last time we
-			if (wasSticky && !_IsStickyMode()) {
+			// checked its position, or buttons might have been pressed.
-				buttons = 1;
+			// Unfortunately our child menus don't tell
-					// buttons must have been pressed in the meantime
+			// us the new position.
+			// TODO: Maybe have a shared struct between all menus
+			// where to store the current mouse position ?
+			// (Or just use the BView mouse hooks)			
+			BPoint newWhere;
+			if (window->Lock()) {
+				GetMouse(&newWhere, &buttons);
+				window->Unlock();
 			}
 
 			// This code is needed to make menus
 			// that are children of BMenuFields "sticky" (see ticket #953)
 			if (localAction == MENU_STATE_CLOSED) {
-				// The mouse could have meen moved since the last time we
-				// checked its position. Unfortunately our child menus don't tell
-				// us the new position.
-				// TODO: Maybe have a shared struct between all menus
-				// where to store the current mouse position ? 				
-				BPoint newWhere;
-				uint32 newButtons;
-				if (window->Lock()) {
-					GetMouse(&newWhere, &newButtons);
-					window->Unlock();
-				}
-
 				if (fExtraRect != NULL && fExtraRect->Contains(where)
 					// 9 = 3 pixels ^ 2 (since point_distance() returns the square of the distance)					
 					&& point_distance(newWhere, where) < 9) {
@@ -563,8 +542,14 @@ BMenuBar::_Track(int32 *action, int32 startIndex, bool showMenu)
 			fState = MENU_STATE_TRACKING;
 		}
 
-		if (locked)
+		if (!locked)
+			locked = window->Lock();
+	
+		if (locked) {
+			GetMouse(&where, &buttons, true);
 			window->Unlock();
+			locked = false;
+		}
 
 		if (fState == MENU_STATE_CLOSED 
 			|| (buttons != 0 && _IsStickyMode() && menuItem == NULL))

src/kits/interface/MenuWindow.cpp

@@ -243,6 +243,13 @@ BMenuWindow::~BMenuWindow()
 }
 
 
+void
+BMenuWindow::DispatchMessage(BMessage *message, BHandler *handler)
+{
+	BWindow::DispatchMessage(message, handler);
+}
+
+
 void
 BMenuWindow::AttachMenu(BMenu *menu)
 {