popupMenu, panelMenu: split up panel and non-panel keynav

PopupMenuManager was pretending that it knew nothing about the menu's
sourceActors, while also trying to handle keynav between them. This
was a big mess, and resulted in bugs in navigation between panel menus
and the Activities button, and it totally gets in the way when trying
to add keynav to the dash (whose menu sources are arranged vertically
rather than horizontally).

Fix this up by moving the panel-specific parts to PanelMenuButton
instead.

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

js/ui/panelMenu.js

@@ -1,7 +1,9 @@
 /* -*- mode: js2; js2-basic-offset: 4; indent-tabs-mode: nil -*- */
 
 const Clutter = imports.gi.Clutter;
+const Gtk = imports.gi.Gtk;
 const St = imports.gi.St;
+
 const Lang = imports.lang;
 const PopupMenu = imports.ui.popupMenu;
 const Main = imports.ui.main;
@@ -20,9 +22,10 @@ Button.prototype = {
                                   track_hover: true });
         this.actor._delegate = this;
         this.actor.connect('button-press-event', Lang.bind(this, this._onButtonPress));
-        this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPress));
+        this.actor.connect('key-press-event', Lang.bind(this, this._onSourceKeyPress));
-        this.menu = new PopupMenu.PopupMenu(this.actor, menuAlignment, St.Side.TOP, /* FIXME */ 0);
+        this.menu = new PopupMenu.PopupMenu(this.actor, menuAlignment, St.Side.TOP, 0);
         this.menu.connect('open-state-changed', Lang.bind(this, this._onOpenStateChanged));
+        this.menu.actor.connect('key-press-event', Lang.bind(this, this._onMenuKeyPress));
         Main.chrome.addActor(this.menu.actor, { visibleInOverview: true,
                                                 affectsStruts: false });
         this.menu.actor.hide();
@@ -32,20 +35,37 @@ Button.prototype = {
         this.menu.toggle();
     },
 
-    _onKeyPress: function(actor, event) {
+    _onSourceKeyPress: function(actor, event) {
         let symbol = event.get_key_symbol();
         if (symbol == Clutter.KEY_space || symbol == Clutter.KEY_Return) {
             this.menu.toggle();
             return true;
+        } else if (symbol == Clutter.KEY_Escape && this.menu.isOpen) {
+            this.menu.close();
+            return true;
         } else if (symbol == Clutter.KEY_Down) {
             if (!this.menu.isOpen)
                 this.menu.toggle();
-            this.menu.activateFirst();
+            this.menu.actor.navigate_focus(this.actor, Gtk.DirectionType.DOWN, false);
             return true;
         } else
             return false;
     },
 
+    _onMenuKeyPress: function(actor, event) {
+        let symbol = event.get_key_symbol();
+        if (symbol == Clutter.KEY_Left || symbol == Clutter.KEY_Right) {
+            let focusManager = St.FocusManager.get_for_stage(global.stage);
+            let group = focusManager.get_group(this.actor);
+            if (group) {
+                let direction = (symbol == Clutter.KEY_Left) ? Gtk.DirectionType.LEFT : Gtk.DirectionType.RIGHT;
+                group.navigate_focus(this.actor, direction, false);
+                return true;
+            }
+        }
+        return false;
+    },
+
     _onOpenStateChanged: function(menu, open) {
         if (open)
             this.actor.add_style_pseudo_class('active');

js/ui/popupMenu.js

@@ -684,28 +684,6 @@ PopupImageMenuItem.prototype = {
     }
 };
 
-function mod(a, b) {
-    return (a + b) % b;
-}
-
-function findNextInCycle(items, current, direction) {
-    let cur;
-
-    if (items.length == 0)
-        return current;
-    else if (items.length == 1)
-        return items[0];
-
-    if (current)
-        cur = items.indexOf(current);
-    else if (direction == 1)
-        cur = items.length - 1;
-    else
-        cur = 0;
-
-    return items[mod(cur + direction, items.length)];
-}
-
 function PopupMenuBase() {
     throw new TypeError('Trying to instantiate abstract class PopupMenuBase');
 }
@@ -861,17 +839,6 @@ PopupMenuBase.prototype = {
         }
     },
 
-    activateFirst: function() {
-        let children = this.box.get_children();
-        for (let i = 0; i < children.length; i++) {
-            let actor = children[i];
-            if (actor._delegate && actor._delegate instanceof PopupBaseMenuItem && actor.visible && actor.reactive) {
-                actor._delegate.setActive(true);
-                break;
-            }
-        }
-    },
-
     toggle: function() {
         if (this.isOpen)
             this.close(true);
@@ -909,6 +876,8 @@ PopupMenu.prototype = {
         this.actor = this._boxPointer.actor;
         this.actor._delegate = this;
         this.actor.style_class = 'popup-menu-boxpointer';
+        this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
+
         this._boxWrapper = new Shell.GenericContainer();
         this._boxWrapper.connect('get-preferred-width', Lang.bind(this, this._boxGetPreferredWidth));
         this._boxWrapper.connect('get-preferred-height', Lang.bind(this, this._boxGetPreferredHeight));
@@ -937,6 +906,15 @@ PopupMenu.prototype = {
         this.box.allocate(box, flags);
     },
 
+    _onKeyPressEvent: function(actor, event) {
+        if (event.get_key_symbol() == Clutter.Escape) {
+            this.close(true);
+            return true;
+        }
+
+        return false;
+    },
+
     setArrowOrigin: function(origin) {
         this._boxPointer.setArrowOrigin(origin);
     },
@@ -1127,11 +1105,17 @@ PopupSubMenuMenuItem.prototype = {
     },
 
     _onKeyPressEvent: function(actor, event) {
-        if (event.get_key_symbol() == Clutter.KEY_Right) {
+        let symbol = event.get_key_symbol();
+
+        if (symbol == Clutter.KEY_Right) {
             this.menu.open(true);
-            this.menu.activateFirst();
+            this.menu.actor.navigate_focus(null, Gtk.DirectionType.DOWN, false);
+            return true;
+        } else if (symbol == Clutter.KEY_Left && this.menu.isOpen) {
+            this.menu.close();
             return true;
         }
+
         return PopupBaseMenuItem.prototype._onKeyPressEvent.call(this, actor, event);
     },
 
@@ -1158,12 +1142,13 @@ PopupMenuManager.prototype = {
         this.grabbed = false;
 
         this._eventCaptureId = 0;
-        this._keyPressEventId = 0;
         this._enterEventId = 0;
         this._leaveEventId = 0;
+        this._keyFocusNotifyId = 0;
         this._activeMenu = null;
         this._menus = [];
         this._preGrabInputMode = null;
+        this._grabbedFromKeynav = false;
     },
 
     addMenu: function(menu, position) {
@@ -1172,15 +1157,13 @@ PopupMenuManager.prototype = {
             openStateChangeId: menu.connect('open-state-changed', Lang.bind(this, this._onMenuOpenState)),
             destroyId:         menu.connect('destroy', Lang.bind(this, this._onMenuDestroy)),
             enterId:           0,
-            focusInId:         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.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)
@@ -1205,8 +1188,6 @@ PopupMenuManager.prototype = {
             menu.sourceActor.disconnect(menudata.enterId);
         if (menudata.focusInId)
             menu.sourceActor.disconnect(menudata.focusInId);
-        if (menudata.focusOutId)
-            menu.sourceActor.disconnect(menudata.focusOutId);
 
         this._menus.splice(position, 1);
     },
@@ -1215,10 +1196,10 @@ PopupMenuManager.prototype = {
         Main.pushModal(this._owner.actor);
 
         this._eventCaptureId = global.stage.connect('captured-event', Lang.bind(this, this._onEventCapture));
-        this._keyPressEventId = global.stage.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
         // captured-event doesn't see enter/leave events
         this._enterEventId = global.stage.connect('enter-event', Lang.bind(this, this._onEventCapture));
         this._leaveEventId = global.stage.connect('leave-event', Lang.bind(this, this._onEventCapture));
+        this._keyFocusNotifyId = global.stage.connect('notify::key-focus', Lang.bind(this, this._onKeyFocusChanged));
 
         this.grabbed = true;
     },
@@ -1226,49 +1207,47 @@ PopupMenuManager.prototype = {
     _ungrab: function() {
         global.stage.disconnect(this._eventCaptureId);
         this._eventCaptureId = 0;
-        global.stage.disconnect(this._keyPressEventId);
-        this._keyPressEventId = 0;
         global.stage.disconnect(this._enterEventId);
         this._enterEventId = 0;
         global.stage.disconnect(this._leaveEventId);
         this._leaveEventId = 0;
+        global.stage.disconnect(this._keyFocusNotifyId);
+        this._keyFocusNotifyId = 0;
 
         this.grabbed = false;
         Main.popModal(this._owner.actor);
     },
 
     _onMenuOpenState: function(menu, open) {
+        if (open)
+            this._activeMenu = menu;
+
+        // Check what the focus was before calling pushModal/popModal
+        let focus = global.stage.key_focus;
+        let hadFocus = focus && this._activeMenuContains(focus);
+
         if (open) {
             if (!this.grabbed) {
                 this._preGrabInputMode = global.stage_input_mode;
+                this._grabbedFromKeynav = hadFocus;
                 this._grab();
             }
-            this._activeMenu = menu;
 
-            // if the focus is not already associated with the menu,
+            if (hadFocus)
-            // then focus the menu
+                focus.grab_key_focus();
-            let focus = global.stage.key_focus;
+            else
-            if (!this._activeMenuContains(focus))
+                menu.actor.grab_key_focus();
-                menu.sourceActor.grab_key_focus();
         } else if (menu == this._activeMenu) {
-            let focus = global.stage.key_focus;
-            let fromActive = focus && this._activeMenuContains(focus);
-
             if (this.grabbed)
                 this._ungrab();
             this._activeMenu = null;
 
-            // If keynav was in effect before we grabbed, then we need
+            if (this._grabbedFromKeynav) {
-            // to properly re-establish it after we ungrab. (popModal
+                if (this._preGrabInputMode == Shell.StageInputMode.FOCUSED)
-            // will have unset the focus.) If some part of the menu
+                    global.stage_input_mode = Shell.StageInputMode.FOCUSED;
-            // was focused at the time of the ungrab then focus its
+                if (hadFocus && menu.sourceActor)
-            // 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
+                else if (focus)
                     focus.grab_key_focus();
             }
         }
@@ -1296,29 +1275,20 @@ PopupMenuManager.prototype = {
         return false;
     },
 
-    _onKeyFocusOut: function(menu) {
+    _onKeyFocusChanged: function() {
-        if (!this.grabbed || menu != this._activeMenu)
+        if (!this.grabbed || !this._activeMenu)
             return;
 
-        // We want to close the menu if the focus has moved somewhere
+        let focus = global.stage.key_focus;
-        // other than inside the menu or to another menu's sourceActor.
+        if (focus) {
-        // Unfortunately, when key-focus-out is emitted,
+            if (this._activeMenuContains(focus))
-        // stage.key_focus will be null. So we have to wait until
+                return;
-        // after it emits the key-focus-in as well.
+            if (focus._delegate && focus._delegate.menu &&
-        let id = global.stage.connect('notify::key-focus', Lang.bind(this,
+                this._findMenu(focus._delegate.menu) != -1)
-            function () {
+                return;
-                global.stage.disconnect(id);
+        }
 
-                if (menu != this._activeMenu)
+        this._closeMenu();
-                    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(true);
-            }));
     },
 
     _onMenuDestroy: function(menu) {
@@ -1354,18 +1324,6 @@ PopupMenuManager.prototype = {
         return -1;
     },
 
-    _nextMenu: function(pos, direction) {
-        for (let i = 1; i < this._menus.length; i++) {
-            let candidate = mod(pos + i * direction, this._menus.length);
-            let menu = this._menus[candidate].menu;
-            if (!menu.sourceActor || menu.sourceActor.visible)
-                return menu;
-        }
-        // no menu is found? this should not happen
-        // anyway stay on current menu
-        return this._menus[pos];
-    },
-
     _onEventCapture: function(actor, event) {
         if (!this.grabbed)
             return false;
@@ -1383,8 +1341,7 @@ PopupMenuManager.prototype = {
                 this._closeMenu();
                 return true;
             }
-        } else if ((eventType == Clutter.EventType.BUTTON_PRESS && !activeMenuContains)
+        } else if (eventType == Clutter.EventType.BUTTON_PRESS && !activeMenuContains) {
-                   || (eventType == Clutter.EventType.KEY_PRESS && event.get_key_symbol() == Clutter.Escape)) {
             this._closeMenu();
             return true;
         } else if (activeMenuContains || this._eventIsOnAnyMenuSource(event)) {
@@ -1394,27 +1351,6 @@ PopupMenuManager.prototype = {
         return true;
     },
 
-    _onKeyPressEvent: function(actor, event) {
-        if (!this.grabbed || !this._activeMenu)
-            return false;
-        if (!this._eventIsOnActiveMenu(event))
-            return false;
-
-        let symbol = event.get_key_symbol();
-        if (symbol == Clutter.Left || symbol == Clutter.Right) {
-            let direction = symbol == Clutter.Right ? 1 : -1;
-            let pos = this._findMenu(this._activeMenu);
-            let next = this._nextMenu(pos, direction);
-            if (next != this._activeMenu) {
-                this._changeMenu(next);
-                next.activateFirst();
-            }
-            return true;
-        }
-
-        return false;
-    },
-
     _closeMenu: function() {
         if (this._activeMenu != null)
             this._activeMenu.close(true);

src/st/st-focus-manager.c

@@ -199,3 +199,26 @@ st_focus_manager_remove_group (StFocusManager *manager,
 {
   g_hash_table_remove (manager->priv->groups, root);
 }
+
+/**
+ * st_focus_manager_get_group:
+ * @manager: the #StFocusManager
+ * @widget: an #StWidget
+ *
+ * Checks if @widget is inside a focus group, and if so, returns
+ * the root of that group.
+ *
+ * Return value: (transfer none): the focus group root, or %NULL if
+ * @widget is not in a focus group
+ */
+StWidget *
+st_focus_manager_get_group (StFocusManager *manager,
+                            StWidget       *widget)
+{
+  ClutterActor *actor = CLUTTER_ACTOR (widget);
+
+  while (actor && !g_hash_table_lookup (manager->priv->groups, actor))
+    actor = clutter_actor_get_parent (actor);
+
+  return ST_WIDGET (actor);
+}

src/st/st-focus-manager.h

@@ -73,6 +73,8 @@ void            st_focus_manager_add_group     (StFocusManager *manager,
                                                 StWidget       *root);
 void            st_focus_manager_remove_group  (StFocusManager *manager,
                                                 StWidget       *root);
+StWidget       *st_focus_manager_get_group     (StFocusManager *manager,
+                                                StWidget       *widget);
 
 G_END_DECLS