Applied patch by Ziusudra in ticket #4930 to avoid a deadlock when

navigating the menus via keyboard.
I also factored some code into functions, in particular the code to
add the dynamic items. Also keep track if the dynamic items have been added
or not (using a new boolean class member).
I tested for regressions but couldn't find any.



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

headers/os/interface/Menu.h

@@ -183,7 +183,8 @@ private:
 			BMenu&				operator=(const BMenu& other);
 
 			void				_InitData(BMessage* archive);
-			bool				_Show(bool selectFirstItem = false);
+			bool				_Show(bool selectFirstItem = false,
+									bool keyDown = false);
 			void				_Hide();
 			BMenuItem*			_Track(int* action, long start = -1);
 
@@ -235,18 +236,22 @@ private:
 			void				_Uninstall();
 			void				_SelectItem(BMenuItem* item,
 									bool showSubmenu = true,
-									bool selectFirstItem = false);
+									bool selectFirstItem = false,
+									bool keyDown = false);
 			bool				_SelectNextItem(BMenuItem* item, bool forward);
 			BMenuItem*			_NextItem(BMenuItem* item, bool forward) const;
 			void				_SetIgnoreHidden(bool on);
 			void				_SetStickyMode(bool on);
 			bool				_IsStickyMode() const;
+			void				_GetIsAltCommandKey(bool &value) const;
 			void				_CalcTriggers();
 			bool				_ChooseTrigger(const char* title, int32& index,
 									uint32& trigger,
 									BPrivate::TriggerList& triggers);
 			void				_UpdateWindowViewSize(const bool &updatePosition);
-			bool				_OkToProceed(BMenuItem* item);
+			bool				_AddDynamicItems();
+			bool				_OkToProceed(BMenuItem* item,
+									bool keyDown = false);
 
 			bool				_CustomTrackingWantsToQuit();
 
@@ -277,7 +282,10 @@ private:
 
 			LayoutData*			fLayoutData;
 
-			int32				_reserved;
+			bool				fDynamicItemsAdded;
+			bool				_reserved1;
+			bool				_reserved2;
+			bool				_reserved3;
 
 			char				fTrigger;
 			bool				fResizeToFit;

src/kits/interface/Menu.cpp

@@ -210,6 +210,7 @@ BMenu::BMenu(const char* name, menu_layout layout)
 	fMaxContentWidth(0.0f),
 	fInitMatrixSize(NULL),
 	fExtraMenuData(NULL),
+	fDynamicItemsAdded(false),
 	fTrigger(0),
 	fResizeToFit(true),
 	fUseCachedMenuLayout(false),
@@ -244,6 +245,7 @@ BMenu::BMenu(const char* name, float width, float height)
 	fMaxContentWidth(0.0f),
 	fInitMatrixSize(NULL),
 	fExtraMenuData(NULL),
+	fDynamicItemsAdded(false),
 	fTrigger(0),
 	fResizeToFit(true),
 	fUseCachedMenuLayout(false),
@@ -279,6 +281,7 @@ BMenu::BMenu(BMessage* archive)
 	fMaxContentWidth(0.0f),
 	fInitMatrixSize(NULL),
 	fExtraMenuData(NULL),
+	fDynamicItemsAdded(false),
 	fTrigger(0),
 	fResizeToFit(true),
 	fUseCachedMenuLayout(false),
@@ -367,35 +370,11 @@ BMenu::AttachedToWindow()
 {
 	BView::AttachedToWindow();
 
-	// TODO: Move into init_interface_kit().
+	_GetIsAltCommandKey(sAltAsCommandKey);
-	// Currently we can't do that, as get_key_map() blocks forever
+		
-	// when called on input_server initialization, since it tries
+	bool attachAborted = _AddDynamicItems();
-	// to send a synchronous message to itself (input_server is
-	// a BApplication)
 
-	BMenu::sAltAsCommandKey = true;
+	if (!attachAborted) {
-	key_map* keys = NULL;
-	char* chars = NULL;
-	get_key_map(&keys, &chars);
-	if (keys == NULL || keys->left_command_key != 0x5d
-		|| keys->left_control_key != 0x5c)
-		BMenu::sAltAsCommandKey = false;
-	free(chars);
-	free(keys);
-
-	BMenuItem* superItem = Superitem();
-	BMenu* superMenu = Supermenu();
-	if (AddDynamicItem(B_INITIAL_ADD)) {
-		do {
-			if (superMenu != NULL && !superMenu->_OkToProceed(superItem)) {
-				AddDynamicItem(B_ABORT);
-				fAttachAborted = true;
-				break;
-			}
-		} while (AddDynamicItem(B_PROCESSING));
-	}
-
-	if (!fAttachAborted) {
 		_CacheFontInfo();
 		_LayoutItems(0);
 		_UpdateWindowViewSize(false);
@@ -515,7 +494,11 @@ BMenu::KeyDown(const char* bytes, int32 numBytes)
 				_SelectNextItem(fSelected, true);
 			else {
 				if (fSelected && fSelected->Submenu()) {
-					_SelectItem(fSelected, true, true);
+					fSelected->Submenu()->_SetStickyMode(true); 
+						// fix me: this shouldn't be needed but dynamic menus 
+						// aren't getting it set correctly when keyboard 
+						// navigating, which aborts the attach 
+					_SelectItem(fSelected, true, true, true);
 				} else if (dynamic_cast<BMenuBar*>(Supermenu())) {
 					// if we have no submenu and we're an
 					// item in the top menu below the menubar,
@@ -1261,6 +1244,7 @@ BMenu::BMenu(BRect frame, const char* name, uint32 resizingMode, uint32 flags,
 	fMaxContentWidth(0.0f),
 	fInitMatrixSize(NULL),
 	fExtraMenuData(NULL),
+	fDynamicItemsAdded(false),
 	fTrigger(0),
 	fResizeToFit(resizeToFit),
 	fUseCachedMenuLayout(false),
@@ -1462,7 +1446,7 @@ BMenu::_InitData(BMessage* archive)
 
 
 bool
-BMenu::_Show(bool selectFirstItem)
+BMenu::_Show(bool selectFirstItem, bool keyDown)
 {
 	// See if the supermenu has a cached menuwindow,
 	// and use that one if possible.
@@ -1486,7 +1470,19 @@ BMenu::_Show(bool selectFirstItem)
 		return false;
 
 	if (window->Lock()) {
+		bool attachAborted = false;
+		if (keyDown)
+			attachAborted = _AddDynamicItems();	
+		
+		if (attachAborted) {
+			if (ourWindow)
+				window->Quit();
+			else
+				window->Unlock();
+			return false;
+		}
 		fAttachAborted = false;
+		
 		window->AttachMenu(this);
 
 		if (ItemAt(0) != NULL) {
@@ -2499,7 +2495,8 @@ BMenu::_Uninstall()
 
 
 void
-BMenu::_SelectItem(BMenuItem* menuItem, bool showSubmenu, bool selectFirstItem)
+BMenu::_SelectItem(BMenuItem* menuItem, bool showSubmenu,
+	bool selectFirstItem, bool keyDown)
 {
 	// Avoid deselecting and then reselecting the same item
 	// which would cause flickering
@@ -2519,7 +2516,7 @@ BMenu::_SelectItem(BMenuItem* menuItem, bool showSubmenu, bool selectFirstItem)
 	if (fSelected != NULL && showSubmenu) {
 		BMenu* subMenu = fSelected->Submenu();
 		if (subMenu != NULL && subMenu->Window() == NULL) {
-			if (!subMenu->_Show(selectFirstItem)) {
+			if (!subMenu->_Show(selectFirstItem, keyDown)) {
 				// something went wrong, deselect the item
 				fSelected->Select(false);
 				fSelected = NULL;
@@ -2619,6 +2616,29 @@ BMenu::_IsStickyMode() const
 }
 
 
+void
+BMenu::_GetIsAltCommandKey(bool &value) const
+{
+	// TODO: Move into init_interface_kit().
+	// Currently we can't do that, as get_key_map() blocks forever
+	// when called on input_server initialization, since it tries
+	// to send a synchronous message to itself (input_server is
+	// a BApplication)
+
+	bool altAsCommand = true;
+	key_map* keys = NULL;
+	char* chars = NULL;
+	get_key_map(&keys, &chars);
+	if (keys == NULL || keys->left_command_key != 0x5d
+		|| keys->left_control_key != 0x5c)
+		altAsCommand = false;
+	free(chars);
+	free(keys);
+
+	value = altAsCommand;
+}
+
+
 void
 BMenu::_CalcTriggers()
 {
@@ -2729,7 +2749,34 @@ BMenu::_UpdateWindowViewSize(const bool &move)
 
 
 bool
-BMenu::_OkToProceed(BMenuItem* item)
+BMenu::_AddDynamicItems()
+{
+	if (fDynamicItemsAdded)
+		return false;
+		
+	bool attachAborted = false;
+	BMenuItem* superItem = Superitem();
+	BMenu* superMenu = Supermenu();
+	if (AddDynamicItem(B_INITIAL_ADD)) {
+		do {
+			if (superMenu != NULL
+				&& !superMenu->_OkToProceed(superItem)) {
+				AddDynamicItem(B_ABORT);
+				attachAborted = true;
+				break;
+			}
+		} while (AddDynamicItem(B_PROCESSING));
+	}
+	
+	if (!attachAborted)
+		fDynamicItemsAdded = true;
+	
+	return attachAborted;
+}
+
+
+bool
+BMenu::_OkToProceed(BMenuItem* item, bool keyDown)
 {
 	BPoint where;
 	ulong buttons;
@@ -2743,7 +2790,8 @@ BMenu::_OkToProceed(BMenuItem* item)
 	// Deskbar, though.
 	if ((buttons != 0 && stickyMode)
 		|| ((dynamic_cast<BMenuBar*>(this) == NULL
-			&& (buttons == 0 && !stickyMode)) || _HitTestItems(where) != item))
+			&& (buttons == 0 && !stickyMode))
+		|| ((_HitTestItems(where) != item) && !keyDown)))
 		return false;
 
 	return true;