[haiku-commits] haiku: hrev52096 - src/kits/interface

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 12 Jul 2018 20:17:01 -0400 (EDT)

hrev52096 adds 1 changeset to branch 'master'
old head: e042d589077430c732fe8c909a9504e426f5a0d1
new head: e4433ad0fa8377e2f5e0e422a0e464687720c94f
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=e4433ad0fa83+%5Ee042d5890774

----------------------------------------------------------------------------

e4433ad0fa83: BMenu: Fix crash and keyboard navigation on 'recent items' menus
  
  * Prevents crash mentioned in Trac, but also enables keyboard navigation
    to 'recent items' menus such as "Open files..." in MediaPlayer and DiskProbe
  * Check selected menu and submenu exist in menu tracking thread before 
accessing
  * Update BMenu::AttachedToWindow to pass in keydown param to _AddDynamicItems
  
  Fixes #9251
  Change-Id: I3031b8e9c1b9dd4ef1187c5a6b8ab7925e3496d2

                                       [ David Murphy <murphman@xxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev52096
Commit:      e4433ad0fa8377e2f5e0e422a0e464687720c94f
URL:         https://git.haiku-os.org/haiku/commit/?id=e4433ad0fa83
Author:      David Murphy <murphman@xxxxxxxxx>
Date:        Sun Jul  8 04:45:07 2018 UTC
Committer:   waddlesplash <waddlesplash@xxxxxxxxx>
Commit-Date: Fri Jul 13 00:16:58 2018 UTC

Ticket:      https://dev.haiku-os.org/ticket/9251

----------------------------------------------------------------------------

1 file changed, 42 insertions(+), 25 deletions(-)
src/kits/interface/Menu.cpp | 67 ++++++++++++++++++++++++++---------------

----------------------------------------------------------------------------

diff --git a/src/kits/interface/Menu.cpp b/src/kits/interface/Menu.cpp
index 98b1054927..2ddcf51795 100644
--- a/src/kits/interface/Menu.cpp
+++ b/src/kits/interface/Menu.cpp
@@ -388,7 +388,14 @@ BMenu::AttachedToWindow()
        _GetOptionKey(sOptionKey);
        _GetMenuKey(sMenuKey);
 
-       fAttachAborted = _AddDynamicItems();
+       // The menu should be added to the menu hierarchy and made visible if:
+       // * the mouse is over the menu,
+       // * the user has requested the menu via the keyboard.
+       // So if we don't pass keydown in here, keyboard navigation breaks since
+       // fAttachAborted will return false if the mouse isn't over the menu
+       bool keyDown = Supermenu() != NULL
+               ? Supermenu()->fState == MENU_STATE_KEY_TO_SUBMENU : false;
+       fAttachAborted = _AddDynamicItems(keyDown);
 
        if (!fAttachAborted) {
                _CacheFontInfo();
@@ -1661,31 +1668,41 @@ BMenu::_Track(int* action, long start)
                        // that our window gets any update message to
                        // redraw itself
                        UnlockLooper();
-                       int submenuAction = MENU_STATE_TRACKING;
-                       BMenu* submenu = fSelected->Submenu();
-                       submenu->_SetStickyMode(_IsStickyMode());
-
-                       // The following call blocks until the submenu
-                       // gives control back to us, either because the mouse
-                       // pointer goes out of the submenu's bounds, or because
-                       // the user closes the menu
-                       BMenuItem* submenuItem = 
submenu->_Track(&submenuAction);
-                       if (submenuAction == MENU_STATE_CLOSED) {
-                               item = submenuItem;
-                               fState = MENU_STATE_CLOSED;
-                       } else if (submenuAction == 
MENU_STATE_KEY_LEAVE_SUBMENU) {
-                               if (LockLooper()) {
-                                       BMenuItem* temp = fSelected;
-                                       // close the submenu:
-                                       _SelectItem(NULL);
-                                       // but reselect the item itself for 
user:
-                                       _SelectItem(temp, false);
-                                       UnlockLooper();
+
+                       // To prevent NULL access violation, ensure a menu has 
actually
+                       // been selected and that it has a submenu. Because 
keyboard and
+                       // mouse interactions set selected items differently, 
the menu
+                       // tracking thread needs to be careful in triggering 
the navigation
+                       // to the submenu.
+                       if (fSelected != NULL) {
+                               BMenu* submenu = fSelected->Submenu();
+                               int submenuAction = MENU_STATE_TRACKING;
+                               if (submenu != NULL) {
+                                       
submenu->_SetStickyMode(_IsStickyMode());
+
+                                       // The following call blocks until the 
submenu
+                                       // gives control back to us, either 
because the mouse
+                                       // pointer goes out of the submenu's 
bounds, or because
+                                       // the user closes the menu
+                                       BMenuItem* submenuItem = 
submenu->_Track(&submenuAction);
+                                       if (submenuAction == MENU_STATE_CLOSED) 
{
+                                               item = submenuItem;
+                                               fState = MENU_STATE_CLOSED;
+                                       } else if (submenuAction == 
MENU_STATE_KEY_LEAVE_SUBMENU) {
+                                               if (LockLooper()) {
+                                                       BMenuItem* temp = 
fSelected;
+                                                       // close the submenu:
+                                                       _SelectItem(NULL);
+                                                       // but reselect the 
item itself for user:
+                                                       _SelectItem(temp, 
false);
+                                                       UnlockLooper();
+                                               }
+                                               // cancel  key-nav state
+                                               fState = MENU_STATE_TRACKING;
+                                       } else
+                                               fState = MENU_STATE_TRACKING;
                                }
-                               // cancel  key-nav state
-                               fState = MENU_STATE_TRACKING;
-                       } else
-                               fState = MENU_STATE_TRACKING;
+                       }
                        if (!LockLooper())
                                break;
                } else if ((item = _HitTestItems(location, B_ORIGIN)) != NULL) {


Other related posts: