popupMenu: fix up grab/ungrab handling

Fix the panel menus to avoid unnecessarily bouncing out of modal (bug
634194) and to do a better job of keeping the keyboard focus in the
right place

https://bugzilla.gnome.org/show_bug.cgi?id=618885

js/ui/panelMenu.js

@@ -47,12 +47,9 @@ Button.prototype = {
     },
 
     _onOpenStateChanged: function(menu, open) {
-        if (open) {
+        if (open)
             this.actor.add_style_pseudo_class('pressed');
-            let focus = global.stage.get_key_focus();
+        else
-            if (!focus || (focus != this.actor && !menu.actor.contains(focus)))
-                this.actor.grab_key_focus();
-        } else
             this.actor.remove_style_pseudo_class('pressed');
     }
 };

js/ui/popupMenu.js

@@ -999,23 +999,24 @@ PopupMenuManager.prototype = {
         this._leaveEventId = 0;
         this._activeMenu = null;
         this._menus = [];
-        this._delayedMenus = [];
+        this._preGrabInputMode = null;
     },
 
     addMenu: function(menu, position) {
         let menudata = {
             menu:              menu,
             openStateChangeId: menu.connect('open-state-changed', Lang.bind(this, this._onMenuOpenState)),
-            activateId:        menu.connect('activate', Lang.bind(this, this._onMenuActivated)),
             destroyId:         menu.connect('destroy', Lang.bind(this, this._onMenuDestroy)),
             enterId:           0,
-            focusId:           0
+            focusInId:         0,
+            focusOutId:        0
         };
 
         let source = menu.sourceActor;
         if (source) {
             menudata.enterId = source.connect('enter-event', Lang.bind(this, function() { this._onMenuSourceEnter(menu); }));
-            menudata.focusId = source.connect('key-focus-in', Lang.bind(this, function() { this._onMenuSourceEnter(menu); }));
+            menudata.focusInId = source.connect('key-focus-in', Lang.bind(this, function() { this._onMenuSourceEnter(menu); }));
+            menudata.focusOutId = source.connect('key-focus-out', Lang.bind(this, function() { this._onKeyFocusOut(menu); }));
         }
 
         if (position == undefined)
@@ -1034,18 +1035,19 @@ PopupMenuManager.prototype = {
 
         let menudata = this._menus[position];
         menu.disconnect(menudata.openStateChangeId);
-        menu.disconnect(menudata.activateId);
         menu.disconnect(menudata.destroyId);
 
         if (menudata.enterId)
             menu.sourceActor.disconnect(menudata.enterId);
-        if (menudata.focusId)
+        if (menudata.focusInId)
-            menu.sourceActor.disconnect(menudata.focusId);
+            menu.sourceActor.disconnect(menudata.focusInId);
+        if (menudata.focusOutId)
+            menu.sourceActor.disconnect(menudata.focusOutId);
 
         this._menus.splice(position, 1);
     },
 
-    grab: function() {
+    _grab: function() {
         Main.pushModal(this._owner.actor);
 
         this._eventCaptureId = global.stage.connect('captured-event', Lang.bind(this, this._onEventCapture));
@@ -1057,7 +1059,7 @@ PopupMenuManager.prototype = {
         this.grabbed = true;
     },
 
-    ungrab: function() {
+    _ungrab: function() {
         global.stage.disconnect(this._eventCaptureId);
         this._eventCaptureId = 0;
         global.stage.disconnect(this._keyPressEventId);
@@ -1073,40 +1075,99 @@ PopupMenuManager.prototype = {
 
     _onMenuOpenState: function(menu, open) {
         if (open) {
-            this._activeMenu = menu;
+            if (!this.grabbed) {
-            if (!this.grabbed)
+                this._preGrabInputMode = global.stage_input_mode;
-                this.grab();
+                this._grab();
-        } else if (menu == this._activeMenu) {
-            this._activeMenu = null;
-            if (this.grabbed)
-                this.ungrab();
             }
+            this._activeMenu = menu;
+
+            // if the focus is not already associated with the menu,
+            // then focus the menu
+            let focus = global.stage.key_focus;
+            if (!this._activeMenuContains(focus))
+                menu.sourceActor.grab_key_focus();
+        } else if (menu == this._activeMenu) {
+            let focus = global.stage.key_focus;
+            let fromActive = this._activeMenuContains(focus);
+
+            if (this.grabbed)
+                this._ungrab();
+            this._activeMenu = null;
+
+            // If keynav was in effect before we grabbed, then we need
+            // to properly re-establish it after we ungrab. (popModal
+            // will have unset the focus.) If some part of the menu
+            // was focused at the time of the ungrab then focus its
+            // sourceActor. Otherwise just reset the focus to where it
+            // was right before the ungrab.
+            if (this._preGrabInputMode == Shell.StageInputMode.FOCUSED) {
+                global.stage_input_mode = Shell.StageInputMode.FOCUSED;
+                if (fromActive)
+                    menu.sourceActor.grab_key_focus();
+                else
+                    focus.grab_key_focus();
+            }
+        }
+    },
+
+    // change the currently-open menu without dropping grab
+    _changeMenu: function(newMenu) {
+        if (this._activeMenu) {
+            // _onOpenMenuState will drop the grab if it sees
+            // this._activeMenu being closed; so clear _activeMenu
+            // before closing it to keep that from happening
+            let oldMenu = this._activeMenu;
+            this._activeMenu = null;
+            oldMenu.close();
+        }
+        newMenu.open();
     },
 
     _onMenuSourceEnter: function(menu) {
         if (!this.grabbed || menu == this._activeMenu)
             return false;
 
-        if (this._activeMenu != null)
+        this._changeMenu(menu);
-            this._activeMenu.close();
-        menu.open();
         return false;
     },
 
-    _onMenuActivated: function(menu, item) {
+    _onKeyFocusOut: function(menu) {
-        if (this.grabbed)
+        if (!this.grabbed || menu != this._activeMenu)
-            this.ungrab();
+            return;
+
+        // We want to close the menu if the focus has moved somewhere
+        // other than inside the menu or to another menu's sourceActor.
+        // Unfortunately, when key-focus-out is emitted,
+        // stage.key_focus will be null. So we have to wait until
+        // after it emits the key-focus-in as well.
+        let id = global.stage.connect('notify::key-focus', Lang.bind(this,
+            function () {
+                global.stage.disconnect(id);
+
+                if (menu != this._activeMenu)
+                    return;
+
+                let focus = global.stage.key_focus;
+                if (!focus || this._activeMenuContains(focus))
+                    return;
+                if (focus._delegate && this._findMenu(focus._delegate.menu) != -1)
+                    return;
+                menu.close();
+            }));
     },
 
     _onMenuDestroy: function(menu) {
         this.removeMenu(menu);
     },
 
-    _eventIsOnActiveMenu: function(event) {
+    _activeMenuContains: function(actor) {
-        let src = event.get_source();
         return this._activeMenu != null
-                && (this._activeMenu.actor.contains(src) ||
+                && (this._activeMenu.actor.contains(actor) ||
-                    (this._activeMenu.sourceActor && this._activeMenu.sourceActor.contains(src)));
+                    (this._activeMenu.sourceActor && this._activeMenu.sourceActor.contains(actor)));
+    },
+
+    _eventIsOnActiveMenu: function(event) {
+        return this._activeMenuContains(event.get_source());
     },
 
     _eventIsOnAnyMenuSource: function(event) {
@@ -1168,10 +1229,7 @@ PopupMenuManager.prototype = {
             let pos = this._findMenu(this._activeMenu);
             let next = this._menus[mod(pos + direction, this._menus.length)].menu;
             if (next != this._activeMenu) {
-                let oldMenu = this._activeMenu;
+                this._changeMenu(next);
-                this._activeMenu = next;
-                oldMenu.close();
-                next.open();
                 next.activateFirst();
             }
             return true;