906718 - [FIG] Pressing the BACK button no longer moves the user up in the bookmark folder hierarchy

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Adrian Tamas (:AdrianT)]

Asus Transformer TF101 (Android 4.0.4)/ LG Optimus 2X (Android 2.2)
Figgy Fennec 2013-08-19

Steps to reproduce:
1) Setup sync on Figgy
2) Go to the Bookmarks tab and go down the folder hierarchy (Desktop -> Bookmarks Menu -> any personal folder)
3) Use the device BACK button to go up in the folder hierarchy

Expected results:
The user is taken up one level in the folder hierarchy

Actual results:
The app is minimized regardless of where the user is in the bookmark folder hierarchy.

Notes:
On the standard Firefox for Android back would go up in the folder hierarchy

Comment 1

[:Margaret Leibovic]

I originally implemented this in bug 727451, so I can look into fixing it.

Comment 2

[:Margaret Leibovic]

I want to get ibarlow's take on this. The UX is different now because instead of entering a folder that takes up the whole screen, the thumbnails are still on top of the folders. Also, we now show a whole row for the parent folder, with a "go back a folder" icon, so there is a more obvious other way to get back.

For the sake of reducing complexity, I'm in favor of not fixing this, but I may just be a lazy engineer :)

Comment 3

[Ian Barlow (:ibarlow)]

(In reply to :Margaret Leibovic from comment #2)
> I want to get ibarlow's take on this. The UX is different now because
> instead of entering a folder that takes up the whole screen, the thumbnails
> are still on top of the folders. Also, we now show a whole row for the
> parent folder, with a "go back a folder" icon, so there is a more obvious
> other way to get back.

True, but I think most Android users have the idea that "if I click on something, pressing back reverses my action" hard wired into their brains. 

I know it doubles up the ways in which you navigate back up the folder structure, but I think it's important to do both because some people may not immediately see the "parent folder row" and also because most will already have strong muscle memory associated with the device's back button.

Comment 4

[:Margaret Leibovic]

Attachment: patch

My one concern with this patch is that it relies on the fact that the BookmarksListView is focused in order to work. This is generally a fine assumption, since the user will focus the view when tapping on a folder item, but this won't work if the user taps a bookmarks folder, then enters editing mode, then pressing back to dismiss editing mode, then tries pressing back to go up a folder level.

In order to deal with this case, I think we'd need to go through the activity's onBackPressed method, but that would require multiple layers of indirection to get back down to the method we want to call in BookmarksListAdapter, so I'm not sure it's really worth fixing that.

Comment 5

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 792463 [details] [diff] [review]
patch

Review of attachment 792463 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r+ with suggested change.

::: mobile/android/base/home/BookmarksListView.java
@@ +50,5 @@
>  
>          // Scaled touch slop for this context.
>          mTouchSlop = ViewConfiguration.get(context).getScaledTouchSlop();
> +
> +        setOnKeyListener(new View.OnKeyListener() {

Please move this to onAttachedToWindow().

Comment 6

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/71dbfbd1449b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/71dbfbd1449b