After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 646740 - gnome-shell logout dialog missing accelerators
gnome-shell logout dialog missing accelerators
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal trivial
: ---
Assigned To: Owen Taylor
gnome-shell-maint
: 648109 648141 648191 648536 (view as bug list)
Depends on:
Blocks: GnomeShell301
 
 
Reported: 2011-04-04 19:49 UTC by Adam Jackson
Modified: 2011-04-24 14:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ShellStack: make this an StWidget (2.08 KB, patch)
2011-04-06 17:27 UTC, Dan Winship
none Details | Review
endSessionDialog: fix keyboard navigation (3.82 KB, patch)
2011-04-06 17:37 UTC, Dan Winship
reviewed Details | Review
endSessionDialog: fix keyboard navigation (3.36 KB, patch)
2011-04-14 21:12 UTC, Dan Winship
committed Details | Review

Description Adam Jackson 2011-04-04 19:49:54 UTC
The logout dialog appears to have a keybinding for Escape to mean Cancel, but neither space nor enter will select Log Out, nor does Tab generate a highlight or change button focus.

Enter or space seem like they should be confirmations like everywhere else.  I could take or leave the tab thing though.

Tested with gnome-shell 2.91.93-1.fc15.
Comment 1 Dan Winship 2011-04-06 17:27:42 UTC
Created attachment 185344 [details] [review]
ShellStack: make this an StWidget

Derive from StGroup rather than ClutterGroup, so that keyboard focus
navigation can pass through it correctly.

For the moment, it does not properly handle allocation with respect to
borders and padding.
Comment 2 Dan Winship 2011-04-06 17:29:07 UTC
Comment on attachment 185344 [details] [review]
ShellStack: make this an StWidget

actually, ignore this. I realized while attaching the patches that there was a simpler fix
Comment 3 Dan Winship 2011-04-06 17:37:56 UTC
Created attachment 185346 [details] [review]
endSessionDialog: fix keyboard navigation

The addition of _backgroundStack to ModalDialog broke focus
navigation, because it was interposed between the focus group root and
all of the interesting content, but since it isn't an StWidget,
st_widget_navigate_focus() was unable to navigate through it. Fix this
by moving the focus root to this._dialogLayout (inside the
_backgroundStack) instead.

Additionally, in EndSessionDialog specifically, _initialKeyFocus
wasn't being set until after opening the dialog, so it was ignored.

Also fix ModalDialog.popModal() to only remember the currently-focused
actor when it's called directly, not when it's called as part of
closing the dialog, so that the focus in the EndSessionDialog properly
reverts back to _initialKeyFocus each time it's reopened.
Comment 4 Adam Jackson 2011-04-14 17:22:20 UTC
I can't comment on the correctness of the patch, but I've tested it and it certainly appears to fix keynav in the logout dialog.  Thanks!
Comment 5 Owen Taylor 2011-04-14 20:46:25 UTC
Review of attachment 185346 [details] [review]:

Works well in testing, code changes generally make sense.

::: js/ui/modalDialog.js
@@ +216,3 @@
             return;
 
+        if (this.state != State.CLOSING) {

This doesn't seem to handle the _fadeOutDialog => close case, maybe it's just better to make close null out _savedKeyFocus?
Comment 6 Dan Winship 2011-04-14 21:12:09 UTC
Created attachment 185982 [details] [review]
endSessionDialog: fix keyboard navigation

re-simplify popModal(), just clear _savedKeyFocus on close()
Comment 7 Owen Taylor 2011-04-14 21:30:04 UTC
Review of attachment 185982 [details] [review]:

Looks good, still works well in testing. Check to make sure you are still happy with the last part of the commit message.
Comment 8 Dan Winship 2011-04-14 21:55:39 UTC
pushed with fixed commit message

Attachment 185982 [details] pushed as 59c3e3a - endSessionDialog: fix keyboard navigation
Comment 9 Dan Winship 2011-04-18 13:32:03 UTC
*** Bug 648109 has been marked as a duplicate of this bug. ***
Comment 10 Rui Matos 2011-04-18 18:37:26 UTC
*** Bug 648141 has been marked as a duplicate of this bug. ***
Comment 11 Rui Matos 2011-04-19 10:17:07 UTC
*** Bug 648191 has been marked as a duplicate of this bug. ***
Comment 12 Dan Winship 2011-04-24 14:30:31 UTC
*** Bug 648536 has been marked as a duplicate of this bug. ***