718268 - After closing tab, tab list scrolls back to the top

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Martijn Wargers (dead)]

Steps to reproduce:
- Open 8 tabs or more
- Open the tab list popup by tapping on the thing on the right of the url bar
- Select the last tab in the tab list
- Close the last tab

Expected result:
Last tab is closed, the second to last tab is the last tab now and is selected and in view.

Actual result:
Last tab is closed, the second to last tab is the last tab now and is selected, but is not in view. The tab list has scrolled to the beginning.

Comment 1

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

setSelection() (my bad, I didn't know about it) perfectly positions the view. Now that we know when a List is inflated (basically after requestLayout() on the ListContainer), we can safely do a setSelection() for the list.
This also solves the problem in Eclair.

Also, closing Tab 1 when Tab 6 (obscured from the view) is the selected tab, should not scroll the list to Tab 6. The mScrollToSelectedTab flag takes care of the same.

Comment 2

[Sriram Ramasubramanian [:sriram]]

I think there is a problem with this patch. If there are 10 tabs with tabs 4-6 shown to the user, and user closes tab 5, it still scrolls to the top.
Do we really want to show tabs 4,6,7 to the user? (Requires more calculations and scrolling properly). Or do we want to scroll to the selected tab every time a tab is closed?
Please treat this patch to be a WIP until we know what we want to do ;)

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 595217 [details] [diff] [review]
Patch

Do we really need the mScrollToSelectedTab code at all? When a user closes a tab, we should not attempt to scroll at all, right?

Changing from review to feedback

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This seems a better approach to me. Here we just remove the tab from the adapter and invalidate the list's views. Thereby the scroll position is retained (by listview itself), the views are redrawn as needed.

Comment 5

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1c5c5e78f370

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/1c5c5e78f370

Comment 7

[Camelia Urian]

Nightly 13.0a1 (2012-02-15)
Device: Samsung Nexus S - Android 2.3.6

When closing the last tab, the second to last tab is the last tab now and is selected and in view.