Closed Bug 906718 Opened 11 years ago Closed 11 years ago

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

Categories

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

All
Android
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: AdrianT, Assigned: Margaret)

References

Details

Attachments

(1 file)

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
Priority: -- → P1
I originally implemented this in bug 727451, so I can look into fixing it.
Assignee: nobody → 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 :)
Flags: needinfo?(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.
Flags: needinfo?(ibarlow)
Attached patch patchSplinter Review
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.
Attachment #792463 - Flags: review?(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().
Attachment #792463 - Flags: review?(sriram) → review+
tracking-fennec: --- → ?
Hardware: ARM → All
https://hg.mozilla.org/mozilla-central/rev/71dbfbd1449b
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
tracking-fennec: ? → ---
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: