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 726760 - app picker keynav: focus escapes from app folders
app picker keynav: focus escapes from app folders
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: overview
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2014-03-20 03:03 UTC by Matthias Clasen
Modified: 2014-05-27 17:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
viewSelector: Don't re-navigate into the active page (1.54 KB, patch)
2014-05-23 16:32 UTC, Florian Müllner
committed Details | Review

Description Matthias Clasen 2014-03-20 03:03:34 UTC
when I keynav around the app picker, I can open a folder by hitting enter. The folder opens and the focus is inside it. Escape closes the folder, and I can move the focus around in it using the arrow keys. Left/Right/Up arrows just stop when I hit the limits of the app folder. But the down arrow makes the focus escape from the folder in a way that feels unintended. I'm left with the focus outside the open app folder and no way to either get the focus back in the folder, or close the folder (by keynav, at least).
Comment 1 Florian Müllner 2014-05-23 16:32:02 UTC
Created attachment 277081 [details] [review]
viewSelector: Don't re-navigate into the active page

Starting keynav into the active page is handled from a key-press
handler on the stage, however we should not "start" keynav when
we are already navigating elsewhere - the latter can happen when
keynav fails (for instance because the focus is trapped inside an
open app folder or at the end of the dash), and the event bubbles
up to the stage. So make sure to only handle the event to actually
start keynav, to not interfere with the normal navigation handling.

Thanks to Carlos Soriano <carlos.soriano89@gmail.com> for the
debugging footwork.
Comment 2 Carlos Soriano 2014-05-23 20:03:45 UTC
Review of attachment 277081 [details] [review]:

One thing I experienced with this patch is that, if you are in the frequent view,
go down until the view selector. Choose all. Try to move focus
to the page. You can't.

I think this is something that when this bug (current one we are trying to solve) is present it doesn't happens,
since we always ended giving focus to the active page.

But I would say this is another bug than now we are hitting due to fixing this, so not related to this patch.
Comment 3 Carlos Soriano 2014-05-23 20:04:40 UTC
So imho, fine to push it.
Comment 4 Florian Müllner 2014-05-23 20:22:34 UTC
(In reply to comment #2)
> Review of attachment 277081 [details] [review]:
> 
> One thing I experienced with this patch is that, if you are in the frequent
> view,
> go down until the view selector. Choose all. Try to move focus
> to the page. You can't.

Mmmh, interesting. I can reproduce this with up-arrow, tab/shift-tab work as expected. Without the patch, down-arrow does work (obviously), but that's actually a bug - arrow keys are not supposed to wrap around.

Note that up-arrow will work when the last page is selected. In any case, it's unrelated to this patch.
Comment 5 Carlos Soriano 2014-05-23 20:25:15 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > Review of attachment 277081 [details] [review] [details]:
> > 
> > One thing I experienced with this patch is that, if you are in the frequent
> > view,
> > go down until the view selector. Choose all. Try to move focus
> > to the page. You can't.
> 
> Mmmh, interesting. I can reproduce this with up-arrow, tab/shift-tab work as
> expected. Without the patch, down-arrow does work (obviously), but that's
> actually a bug - arrow keys are not supposed to wrap around.
> 
> Note that up-arrow will work when the last page is selected. In any case, it's
> unrelated to this patch.

Right
Comment 6 Florian Müllner 2014-05-27 17:52:56 UTC
Attachment 277081 [details] pushed as c228a9a - viewSelector: Don't re-navigate into the active page