GNOME Bugzilla – Bug 663437
Shutdown Dialog cannot be navigated with the keyboard (most of the time)
Last modified: 2012-04-07 17:20:41 UTC
When invoking the Shutdown dialog (with the shutdown key in my keyboard, or by selecting "power off" in the "me" menu of the topbar, the displayed shutdown dialog doesn't have any of the three buttons "focused" and pressing the arrow keys or "enter" is pointless, nothing happens. The dialog is only responding to the "ESC" key, which will close it. I have to say that on very (very) rare occasions, upon display of the shutdown dialog one of the buttons (power off) would come pre-focused (the expected behaviour) and it was responsive to the arrow keys and the enter key. However, I am unable to reproduce the "working" behavior anymore. For reasons unknown there is some weird inconsistency. Using gnome shell 3.2.1 on ubuntu 11.10.
(In reply to comment #0) > When invoking the Shutdown dialog (with the shutdown key in my keyboard, or by > selecting "power off" in the "me" menu of the topbar, the displayed shutdown > dialog doesn't have any of the three buttons "focused" and pressing the arrow > keys or "enter" is pointless, nothing happens. The dialog is only responding to > the "ESC" key, which will close it. > > I have to say that on very (very) rare occasions, upon display of the shutdown > dialog one of the buttons (power off) would come pre-focused (the expected > behaviour) and it was responsive to the arrow keys and the enter key. However, > I am unable to reproduce the "working" behavior anymore. For reasons unknown > there is some weird inconsistency. > > Using gnome shell 3.2.1 on ubuntu 11.10.
Created attachment 207405 [details] [review] modalDialog: Fix setting the initial key focus after the 1st time Checking if _buttonLayout contains _initialKeyFocus always fails since we destroy all children before. Instead, use a signal handler id when explicitly setting the initial key focus which is zeroed if/when the actor is destroyed.
Created attachment 209363 [details] [review] modalDialog: Fix setting the initial key focus after the 1st time -- * Use a better name: _initialKeyFocusDestroyId * Disconnect any previous handler if setting again Thanks to Florian for catching these.
Review of attachment 209363 [details] [review]: Looks good. ::: js/ui/modalDialog.js @@ +134,3 @@ x_alignment = St.Align.MIDDLE; + if (!this._initialKeyFocusDestroyId) Not entirely convinced of using a signal handler id as test condition for whether we should set/move the default focus ... using a separate initialKeyFocusSet boolean would be overkill, but maybe it's clearer to leave the original condition and reset _initialKeyFocus to _dialogLayout in the destroy handler? (not saying that the original condition is particularly pretty though) @@ +210,3 @@ + + this._initialKeyFocusDestroyId = actor.connect('destroy', Lang.bind(this, function() { + this._initialKeyFocusDestroyId = 0; It feels a bit odd to reset the signal handler id, but not _initialKeyFocus here
Created attachment 209455 [details] [review] modalDialog: Fix setting the initial key focus after the 1st time -- (In reply to comment #4) > ::: js/ui/modalDialog.js > @@ +134,3 @@ > x_alignment = St.Align.MIDDLE; > > + if (!this._initialKeyFocusDestroyId) > > Not entirely convinced of using a signal handler id as test condition for > whether we should set/move the default focus ... using a separate > initialKeyFocusSet boolean would be overkill, but maybe it's clearer to leave > the original condition and reset _initialKeyFocus to _dialogLayout in the > destroy handler? (not saying that the original condition is particularly pretty > though) It wasn't intended but _initialKeyFocusId actually made more sense here IMO. > @@ +210,3 @@ > + > + this._initialKeyFocusDestroyId = actor.connect('destroy', > Lang.bind(this, function() { > + this._initialKeyFocusDestroyId = 0; > > It feels a bit odd to reset the signal handler id, but not _initialKeyFocus > here Amended.
[ Ups, forgot to add this ] Using the original condition doesn't work because we set this._initialKeyFocus to a button so (this._initialKeyFocus == this._dialogLayout) only the 1st time again.
Review of attachment 209455 [details] [review]: (In reply to comment #6) > Using the original condition doesn't work because we set this._initialKeyFocus > to a button so (this._initialKeyFocus == this._dialogLayout) only the 1st time > again. But the original condition is (this._initialKeyFocus == this._dialogLayout || this._buttonLayout.contains(this._initialKeyFocus)) which should cover the "we set _initialKeyFocus to a button" case ... (still, code looks good, so just go ahead)
(In reply to comment #7) > (In reply to comment #6) > > Using the original condition doesn't work because we set this._initialKeyFocus > > to a button so (this._initialKeyFocus == this._dialogLayout) only the 1st time > > again. > > But the original condition is (this._initialKeyFocus == this._dialogLayout || > this._buttonLayout.contains(this._initialKeyFocus)) which should cover the "we > set _initialKeyFocus to a button" case ... It doesn't and that's why this bug happens, please see the commit message. Attachment 209455 [details] pushed as b58425d - modalDialog: Fix setting the initial key focus after the 1st time
(In reply to comment #8) > It doesn't and that's why this bug happens, please see the commit message. I still don't understand why it doesn't (the buttonLayout.contains() part "keeps" the focus on right as buttons are added) - the destroy handler reset the initialKeyFocus, so that case should be covered by the "== dialogLayout" part. (or in other words: if setting _initialKeyFocus = _dialogLayout in the destroy handler and testing for _initialKeyFocus == _dialogLayout does not work, then setting _initialKeyFocusDestroyId = 0 and testing for _initialKeyfocusDestroyId == 0 wouldn't work either)
(In reply to comment #9) > (In reply to comment #8) > > It doesn't and that's why this bug happens, please see the commit message. > > I still don't understand why it doesn't (the buttonLayout.contains() part > "keeps" the focus on right as buttons are added) - the destroy handler reset > the initialKeyFocus, so that case should be covered by the "== dialogLayout" > part. > > (or in other words: if setting _initialKeyFocus = _dialogLayout in the destroy > handler and testing for _initialKeyFocus == _dialogLayout does not work, then > setting _initialKeyFocusDestroyId = 0 and testing for _initialKeyfocusDestroyId > == 0 wouldn't work either) If setInitialKeyFocus() is never called, the code in setButtons() will only work right the first time because _initialKeyFocus is set to a button and so on subsequent calls to setButtons(), _initialKeyFocus won't be either == _dialogLayout nor contained in buttonLayout (because all previous buttons got removed at the beggining of setButtons()).
Which exact version of Gnome-Shell will integrate this fix? Currently, I'm testing Linux Mint 12 with Gnome-Shell 3.2.2.1, but the above fix seems not to be in this version, as the shutdown dialog is only navigable by keyboard once per session.
(In reply to comment #11) > Which exact version of Gnome-Shell will integrate this fix? 3.4.0 which was officially released on March 28.