[gnome-shell] Use more actor.grab_key_focus() and less stage.connect('key-press-event')



commit 4dd4c9f99f528667eb0b571a71ed23f61919f44b
Author: Dan Winship <danw gnome org>
Date:   Wed Nov 3 13:30:08 2010 -0400

    Use more actor.grab_key_focus() and less stage.connect('key-press-event')
    
    Until recently, the clutter keyboard focus was almost always kept on
    the stage, and bits of code that wanted to do stuff with the keyboard
    would just watch for key-press-events on the stage. In several places,
    the code wasn't even bothering to ensure that the focus was on the
    stage, which caused problems with other actors that explicitly grabbed
    focus.
    
    A previous fix for this (f21403fd) was to always reset the focus to
    the stage after calling pushModal(), but a better fix is to just
    actually make use of the keyboard focus everywhere rather than having
    everyone try to read events off the stage.
    
    Now pushModal(actor) also does actor.grab_key_focus(), and various
    bits of code have been changed to read key events off their own
    toplevels rather than off the stage, meaning there's no chance of them
    accidentally getting someone else's events.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=618885

 js/ui/altTab.js       |    9 +-----
 js/ui/lookingGlass.js |   14 ++--------
 js/ui/main.js         |   15 ++++++-----
 js/ui/messageTray.js  |   62 ++++++------------------------------------------
 js/ui/viewSelector.js |   62 +++++++++++++++++++-----------------------------
 5 files changed, 46 insertions(+), 116 deletions(-)
---
diff --git a/js/ui/altTab.js b/js/ui/altTab.js
index b3f074d..824c1b0 100644
--- a/js/ui/altTab.js
+++ b/js/ui/altTab.js
@@ -126,8 +126,8 @@ AltTabPopup.prototype = {
             return false;
         this._haveModal = true;
 
-        this._keyPressEventId = global.stage.connect('key-press-event', Lang.bind(this, this._keyPressEvent));
-        this._keyReleaseEventId = global.stage.connect('key-release-event', Lang.bind(this, this._keyReleaseEvent));
+        this.actor.connect('key-press-event', Lang.bind(this, this._keyPressEvent));
+        this.actor.connect('key-release-event', Lang.bind(this, this._keyReleaseEvent));
 
         this.actor.connect('button-press-event', Lang.bind(this, this._clickedOutside));
         this.actor.connect('scroll-event', Lang.bind(this, this._onScroll));
@@ -383,11 +383,6 @@ AltTabPopup.prototype = {
         if (this._thumbnails)
             this._destroyThumbnails();
 
-        if (this._keyPressEventId)
-            global.stage.disconnect(this._keyPressEventId);
-        if (this._keyReleaseEventId)
-            global.stage.disconnect(this._keyReleaseEventId);
-
         if (this._motionTimeoutId != 0)
             Mainloop.source_remove(this._motionTimeoutId);
         if (this._thumbnailTimeoutId != 0)
diff --git a/js/ui/lookingGlass.js b/js/ui/lookingGlass.js
index 9842555..60bab9b 100644
--- a/js/ui/lookingGlass.js
+++ b/js/ui/lookingGlass.js
@@ -663,7 +663,7 @@ LookingGlass.prototype = {
                                         style_class: 'lg-dialog',
                                         vertical: true,
                                         visible: false });
-
+        this.actor.connect('key-press-event', Lang.bind(this, this._globalKeyPressEvent));
 
         this._interfaceSettings = new Gio.Settings({ schema: 'org.gnome.desktop.interface' });
         this._interfaceSettings.connect('changed::monospace-font-name',
@@ -924,20 +924,15 @@ LookingGlass.prototype = {
         if (this._open)
             return;
 
-        if (!Main.pushModal(this.actor))
+        if (!Main.pushModal(this._entry))
             return;
 
-        this._keyPressEventId = global.stage.connect('key-press-event',
-            Lang.bind(this, this._globalKeyPressEvent));
-
         this.actor.show();
         this.actor.lower(Main.chrome.actor);
         this._open = true;
 
         Tweener.removeTweens(this.actor);
 
-        global.stage.set_key_focus(this._entry);
-
         // We inverse compensate for the slow-down so you can change the factor
         // through LookingGlass without long waits.
         Tweener.addTween(this.actor, { time: 0.5 / St.get_slow_down_factor(),
@@ -950,9 +945,6 @@ LookingGlass.prototype = {
         if (!this._open)
             return;
 
-        if (this._keyPressEventId)
-            global.stage.disconnect(this._keyPressEventId);
-
         this._objInspector.actor.hide();
 
         this._historyNavIndex = -1;
@@ -965,7 +957,7 @@ LookingGlass.prototype = {
             this._borderPaintTarget = null;
         }
 
-        Main.popModal(this.actor);
+        Main.popModal(this._entry);
 
         Tweener.addTween(this.actor, { time: 0.5 / St.get_slow_down_factor(),
                                        transition: 'easeOutQuad',
diff --git a/js/ui/main.js b/js/ui/main.js
index 056baab..f76c33b 100644
--- a/js/ui/main.js
+++ b/js/ui/main.js
@@ -350,13 +350,13 @@ function _findModal(actor) {
  * @actor: #ClutterActor which will be given keyboard focus
  *
  * Ensure we are in a mode where all keyboard and mouse input goes to
- * the stage.  Multiple calls to this function act in a stacking fashion;
- * the effect will be undone when an equal number of popModal() invocations
- * have been made.
+ * the stage, and focus @actor. Multiple calls to this function act in
+ * a stacking fashion; the effect will be undone when an equal number
+ * of popModal() invocations have been made.
  *
- * Next, record the current Clutter keyboard focus on a stack.  If the modal stack
- * returns to this actor, reset the focus to the actor which was focused
- * at the time pushModal() was invoked.
+ * Next, record the current Clutter keyboard focus on a stack. If the
+ * modal stack returns to this actor, reset the focus to the actor
+ * which was focused at the time pushModal() was invoked.
  *
  * Returns: true iff we successfully acquired a grab or already had one
  */
@@ -386,7 +386,7 @@ function pushModal(actor) {
     }
     modalActorFocusStack.push([actor, curFocus]);
 
-    global.stage.set_key_focus(null);
+    global.stage.set_key_focus(actor);
     return true;
 }
 
@@ -416,6 +416,7 @@ function popModal(actor) {
     if (modalCount > 0)
         return;
 
+    global.stage.set_key_focus(null);
     global.end_modal(global.get_current_time());
     global.set_stage_input_mode(Shell.StageInputMode.NORMAL);
 }
diff --git a/js/ui/messageTray.js b/js/ui/messageTray.js
index 08ad5c2..a63744f 100644
--- a/js/ui/messageTray.js
+++ b/js/ui/messageTray.js
@@ -245,7 +245,6 @@ Notification.prototype = {
         this._prevFocusedWindow = null;
         this._prevKeyFocusActor = null;
 
-        this._focusWindowChangedId = 0;
         this._focusActorChangedId = 0;
         this._stageInputModeChangedId = 0;
         this._capturedEventId = 0;
@@ -624,29 +623,16 @@ Notification.prototype = {
         let metaDisplay = global.screen.get_display();
 
         this._prevFocusedWindow = metaDisplay.focus_window;
-        this._prevKeyFocus = global.stage.get_key_focus();
-
-        // We need to use the captured event in the overview, because we don't want to change the stage input mode to
-        // FOCUSED there. On the other hand, using the captured event doesn't work correctly in the main view because
-        // it doesn't allow focusing the windows again correctly. So we are using the FOCUSED stage input mode in the
-        // main view.
-        if (Main.overview.visible) {
-            if (!Main.pushModal(this.actor))
-                return;
-            this._capturedEventId = global.stage.connect('captured-event', Lang.bind(this, this._onCapturedEvent));
-        } else {
-            global.set_stage_input_mode(Shell.StageInputMode.FOCUSED);
+        this._prevKeyFocusActor = global.stage.get_key_focus();
 
-            this._focusWindowChangedId = metaDisplay.connect('notify::focus-window', Lang.bind(this, this._focusWindowChanged));
-            this._stageInputModeChangedId = global.connect('notify::stage-input-mode', Lang.bind(this, this._stageInputModeChanged));
+        if (!Main.overview.visible)
+            global.set_stage_input_mode(Shell.StageInputMode.FOCUSED);
 
-            this._keyPressId = global.stage.connect('key-press-event', Lang.bind(this, this._onKeyPress));
-        }
+        // Use captured-event to notice clicks outside the notification
+        // without consuming them.
+        this._capturedEventId = global.stage.connect('captured-event', Lang.bind(this, this._onCapturedEvent));
 
-        // We need to listen to this signal in the overview, as well as in the main view, to make the key bindings such as
-        // Alt+F2 work. When a notification has key focus, which is the case with chat notifications, all captured KEY_PRESS
-        // events have the actor with the key focus as their source. This makes it impossible to distinguish between the chat
-        // window input and the key bindings based solely on the KEY_PRESS event.
+        this._stageInputModeChangedId = global.connect('notify::stage-input-mode', Lang.bind(this, this._stageInputModeChanged));
         this._focusActorChangedId = global.stage.connect('notify::key-focus', Lang.bind(this, this._focusActorChanged));
 
         this._hasFocus = true;
@@ -654,19 +640,6 @@ Notification.prototype = {
             Main.messageTray.lock();
     },
 
-    _focusWindowChanged: function() {
-        let metaDisplay = global.screen.get_display();
-        // this._focusWindowChanged() will be called when we call
-        // global.set_stage_input_mode(Shell.StageInputMode.FOCUSED) ,
-        // however metaDisplay.focus_window will be null in that case. We only
-        // want to ungrab focus if the focus has been moved to an application
-        // window.
-        if (metaDisplay.focus_window) {
-            this._prevFocusedWindow = null;
-            this.ungrabFocus();
-        }
-    },
-
     _focusActorChanged: function() {
         let focusedActor = global.stage.get_key_focus();
         if (!focusedActor || !this.actor.contains(focusedActor)) {
@@ -676,15 +649,6 @@ Notification.prototype = {
     },
 
     _stageInputModeChanged: function() {
-        let focusedActor = global.stage.get_key_focus();
-        // TODO: We need to set this._prevFocusedWindow to null in order to
-        // get the cursor in the run dialog. However, that also means it's
-        // set to null when the application menu is activated, which defeats
-        // the point of keeping the name of the previously focused application
-        // in the panel. It'd be good to be able to distinguish between these
-        // two cases.
-        this._prevFocusedWindow = null;
-        this._prevKeyFocusActor = null;
         this.ungrabFocus();
     },
 
@@ -743,10 +707,6 @@ Notification.prototype = {
             return;
 
         let metaDisplay = global.screen.get_display();
-        if (this._focusWindowChangedId > 0) {
-            metaDisplay.disconnect(this._focusWindowChangedId);
-            this._focusWindowChangedId = 0;
-        }
 
         if (this._focusActorChangedId > 0) {
             global.stage.disconnect(this._focusActorChangedId);
@@ -759,20 +719,14 @@ Notification.prototype = {
         }
 
         if (this._capturedEventId > 0) {
-            Main.popModal(this.actor);
             global.stage.disconnect(this._capturedEventId);
             this._capturedEventId = 0;
         }
 
-        if (this._keyPressId > 0) {
-            global.stage.disconnect(this._keyPressId);
-            this._keyPressId = 0;
-        }
-
         this._hasFocus = false;
         Main.messageTray.unlock();
 
-        if (this._prevFocusedWindow) {
+        if (this._prevFocusedWindow && !metaDisplay.focus_window) {
             metaDisplay.set_input_focus_window(this._prevFocusedWindow, false, global.get_current_time());
             this._prevFocusedWindow = null;
         }
diff --git a/js/ui/viewSelector.js b/js/ui/viewSelector.js
index c2c5b69..96d7a97 100644
--- a/js/ui/viewSelector.js
+++ b/js/ui/viewSelector.js
@@ -16,12 +16,12 @@ const SearchDisplay = imports.ui.searchDisplay;
 const Tweener = imports.ui.tweener;
 
 
-function SearchEntry() {
-    this._init();
+function SearchEntry(focusBase) {
+    this._init(focusBase);
 }
 
 SearchEntry.prototype = {
-    _init : function() {
+    _init : function(focusBase) {
         this.actor = new St.Entry({ name: 'searchEntry',
                                     hint_text: _("Search your computer") });
         this.entry = this.actor.clutter_text;
@@ -45,11 +45,12 @@ SearchEntry.prototype = {
         this.pane = null;
 
         this._capturedEventId = 0;
+        this._focusBase = focusBase;
     },
 
     _updateCursorVisibility: function() {
         let focus = global.stage.get_key_focus();
-        if (focus == global.stage || focus == this.entry)
+        if (focus == this._focusBase || focus == this.entry)
             this.entry.set_cursor_visible(true);
         else
             this.entry.set_cursor_visible(false);
@@ -81,8 +82,8 @@ SearchEntry.prototype = {
 
         this.entry.text = '';
 
-        // Return focus to the stage
-        global.stage.set_key_focus(null);
+        // Return focus to the viewSelector
+        global.stage.set_key_focus(this._focusBase);
 
         this.entry.set_cursor_visible(true);
         this.entry.set_selection(0, 0);
@@ -117,11 +118,11 @@ SearchEntry.prototype = {
                 }
                 return false;
             case Clutter.EventType.KEY_PRESS:
-                // If neither the stage nor our entry have key focus, some
-                // "special" actor grabbed the focus (run dialog, looking
-                // glass); we don't want to interfere with that
+                // If some "special" actor grabbed the focus (run
+                // dialog, looking glass); we don't want to interfere
+                // with that
                 let focus = global.stage.get_key_focus();
-                if (focus != global.stage && focus != this.entry)
+                if (focus != this._focusBase && focus != this.entry)
                     return false;
 
                 let sym = event.get_key_symbol();
@@ -231,22 +232,23 @@ ViewTab.prototype = {
 };
 
 
-function SearchTab() {
-    this._init();
+function SearchTab(focusBase) {
+    this._init(focusBase);
 }
 
 SearchTab.prototype = {
     __proto__: BaseTab.prototype,
 
-    _init: function() {
+    _init: function(focusBase) {
         this._searchActive = false;
         this._searchPending = false;
         this._keyPressId = 0;
         this._searchTimeoutId = 0;
+        this._focusBase = focusBase;
 
         this._searchSystem = new Search.SearchSystem();
 
-        this._searchEntry = new SearchEntry();
+        this._searchEntry = new SearchEntry(focusBase);
         this._searchResults = new SearchDisplay.SearchResults(this._searchSystem);
         BaseTab.prototype._init.call(this,
                                      this._searchEntry.actor,
@@ -274,15 +276,15 @@ SearchTab.prototype = {
         BaseTab.prototype.show.call(this);
 
         if (this._keyPressId == 0)
-            this._keyPressId = global.stage.connect('key-press-event',
-                                                    Lang.bind(this, this._onKeyPress));
+            this._keyPressId = this._searchEntry.entry.connect('key-press-event',
+                                                               Lang.bind(this, this._onKeyPress));
     },
 
     hide: function() {
         BaseTab.prototype.hide.call(this);
 
         if (this._keyPressId > 0) {
-            global.stage.disconnect(this._keyPressId);
+            this._searchEntry.entry.disconnect(this._keyPressId);
             this._keyPressId = 0;
         }
         this._searchEntry.reset();
@@ -317,14 +319,7 @@ SearchTab.prototype = {
         this._searchTimeoutId = Mainloop.timeout_add(150, Lang.bind(this, this._doSearch));
     },
 
-    _onKeyPress: function(stage, event) {
-        // If neither the stage nor the search entry have key focus, some
-        // "special" actor grabbed the focus (run dialog, looking glass);
-        // we don't want to interfere with that
-        let focus = stage.get_key_focus();
-        if (focus != stage && focus != this._searchEntry.entry)
-            return false;
-
+    _onKeyPress: function(entry, event) {
         let symbol = event.get_key_symbol();
         if (symbol == Clutter.Up) {
             if (!this._searchActive)
@@ -393,7 +388,7 @@ ViewSelector.prototype = {
         this._tabs = [];
         this._activeTab = null;
 
-        this._searchTab = new SearchTab();
+        this._searchTab = new SearchTab(this.actor);
         this._searchArea.set_child(this._searchTab.title);
         this._addTab(this._searchTab);
 
@@ -559,14 +554,7 @@ ViewSelector.prototype = {
             }));
     },
 
-    _onKeyPress: function(stage, event) {
-        // Only process events if the stage has key focus - search is handled
-        // by the search tab, and we do not want to interfere with "special"
-        // actors grabbing focus (run dialog, looking glass, notifications).
-        let focus = stage.get_key_focus();
-        if (focus != stage)
-            return false;
-
+    _onKeyPress: function(actor, event) {
         let modifiers = Shell.get_event_state(event);
         let symbol = event.get_key_symbol();
         if (symbol == Clutter.Escape) {
@@ -600,8 +588,8 @@ ViewSelector.prototype = {
             this._overviewHidingId = Main.overview.connect('hiding',
                                                            Lang.bind(this, this._switchDefaultTab));
         if (this._keyPressId == 0)
-            this._keyPressId = global.stage.connect('key-press-event',
-                                                    Lang.bind(this, this._onKeyPress));
+            this._keyPressId = this.actor.connect('key-press-event',
+                                                  Lang.bind(this, this._onKeyPress));
 
         this._switchDefaultTab();
     },
@@ -610,7 +598,7 @@ ViewSelector.prototype = {
         this._searchTab.setFindAsYouType(false);
 
         if (this._keyPressId > 0) {
-            global.stage.disconnect(this._keyPressId);
+            this.actor.disconnect(this._keyPressId);
             this._keyPressId = 0;
         }
 



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]