GNOME Bugzilla – Bug 667439
a11y: deviceItems on the power status menu are not keyboard navigable
Last modified: 2012-08-26 14:57:49 UTC
Well, I think that the summary explains what happens, but not why it is a problem. I understand the reason: they are not navigable as the user is not supposed to interact with them. But, when you use keyboard navigation and Orca, Orca usually only present a object when receive the focus. This is something similar to what happen with a menu on applications like gedit, as if some action is "greyed out", like paste if you don't have anything on the clipboard, keyboard navigation also skip those elements. And although some users complain about that, at least in that case it is just that they don't hear commands that after all they can't use. In this case they are losing information. IMHO, the easiest way to solve this would be allow keyboard navigation on those elements.
Created attachment 221571 [details] [review] Allow navigation on non reactive items Before this patch, navigate_focus return FALSE if the actor is non-reactive. As we are interested on select those elements during the keyboard navigation, this patch removes that condition.
Created attachment 221572 [details] [review] can_focus=!reactive is not true anymore In some cases can_focus was directly set to !reactive. This is not the case anymore, as in some cases the item are non-reactive but we want to focus on it. Note: doing this patch, I saw that gnome-shell javascript has also the concepts of sensitive/insensitive. IMHO, the relation between reactive/non-reactive and sensitive/insensitive is not clear, and it is not explained the difference between sensitive-reactive (or at least I didn't found it). I also have the feeling that in some cases they are used one insted of the other, and that there are a little overlapping on it.
Created attachment 221573 [details] [review] Add a insensitive pseudo class for popupmenuitem Before all these patches, the user knew that they couldn't interact with a item if it didn't allow you to make anything, or navigate to it. As now you can navigate to it, I added this pseudo_class to make that explicit. This patch is optional, IMHO.
Created attachment 221574 [details] Screenshot 1 This screenshot shows the wifi menu using the pseudo class on the selected wifi.
Created attachment 221575 [details] Screenshot 2 This screenshot shows that now you can select the grayed item with the current wifi using keyboard navigation. Orca braille monitor (a Orca debug tool) shows the name of the Wifi selected, and adding "grayed", as a hint to the user that she can't interact with that item.
Created attachment 221576 [details] Screenshot 3 This screenshot shows that the hover effect is working as before the patches. The grayed items can only be selected using keyboard navigation.
Review of attachment 221572 [details] [review]: The "sensitive" style pseudo class is tracked directly with the ClutterActor's "reactive" property directly in StWidget now. http://git.gnome.org/browse/gnome-shell/tree/src/st/st-widget.c#n1407 This makes sense.
Review of attachment 221573 [details] [review]: This should be tracked directly with StWidget now.
Review of attachment 221571 [details] [review]: This is probably going to expose some bugs where we're erroneously setting can_focus, but let's go with it.
(In reply to comment #8) > Review of attachment 221573 [details] [review]: > > This should be tracked directly with StWidget now. Ok, anyway I will wait for some feedback from Allan Day (or any other designer) before reimplementing this, as this means a change on the ui (thats the reason I also added the ui-review keyword)
Created attachment 221774 [details] [review] Revert the other half of "St: don't attempt to give focus to non reactive actors" This reverts commit 26d3b1929e2cd9dcedf829a2224857a533040729.
Review of attachment 221774 [details] [review]: Should you fix UnlockDialog._updateOkButton too? Otherwise, good to commit. ::: js/ui/popupMenu.js @@ +771,3 @@ this._statusBin.child = this._statusLabel; this.actor.reactive = false; + this.actor.can_focus = false; I'm not sure, but it could be that this needs to be true, otherwise AT may miss it.
(In reply to comment #12) > Review of attachment 221774 [details] [review]: > > Should you fix UnlockDialog._updateOkButton too? > Otherwise, good to commit. > > ::: js/ui/popupMenu.js > @@ +771,3 @@ > this._statusBin.child = this._statusLabel; > this.actor.reactive = false; > + this.actor.can_focus = false; > > I'm not sure, but it could be that this needs to be true, otherwise AT may miss > it. True, that should be true. Anyway, instead of setting this.actor.can_focus to true on both paths of the if, probably it makes more sense set can_focus to true at _init
Created attachment 221930 [details] [review] Revert the other half of "St: don't attempt to give focus to non reactive actors" This reverts commit 26d3b1929e2cd9dcedf829a2224857a533040729.
Review of attachment 221930 [details] [review]: LGTM
Created attachment 222040 [details] [review] name label and main status chooser item don't need to be focusable (In reply to comment #9) > Review of attachment 221571 [details] [review]: > > This is probably going to expose some bugs where we're erroneously setting > can_focus, but let's go with it. Like this. After last changes if you key-nav on the user menu, the first item is a big container, that englobes the name, avatar, and status combobox. And we lost the "focusability" of that combobox. This patch sets can_focus=false to the label (as the top panel user menu icon already exposes it) and to the main container (we are not interested on that container but to their contents). This got the status combobox focusable again (in general the same behaviour that we had prior the other patches in this bug). Note: I also tried to get the avatar focusable by setting can_focus=true on it (UserAvatarWidget), but didn't work. So I assume that it is not so trivial like in the other cases. Anyway this is a minor-medium issue, as pressing the avatar just open the user accounts menu, something that you can open in other ways.
Review of attachment 222040 [details] [review]: Good.
Attachment 221930 [details] pushed as 8a86540 - Revert the other half of "St: don't attempt to give focus to non reactive actors"