User Agent: Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130208 Firefox/21.0 Build ID: 20130208031053 Steps to reproduce: Current implementation shows the first tabs where as we should show current tab.

This works for me. I have 15 tabs open, select the 13th, close the tab panel, re-open the tab panel and it's currently selected and in view. Do you have any better steps to reproduce, also can you try 20130210?

1: Have a bunch of tabs open 2: Open new tab via awesomescreen 3: Open tab drawer 4: Notice that I'm at the top of the tab drawer rather than at the bottom/having the selected tab in view This has just been confirmed in 20130210

Still seeing this on 20120223, can you confirm Aaron?

20130223*

Yes, I see this when the newly opened tab is not in view. Also see this with the new landscape horizontal scroller.

See also: https://bugzilla.mozilla.org/show_bug.cgi?id=757844

I can reproduce this bug using the steps from comment 2.

Margaret, looks like a potential regression from bug 846569. Could you check?

(In reply to Lucas Rocha (:lucasr) from comment #8) > Margaret, looks like a potential regression from bug 846569. Could you check? If this was a regression from bug 846569, how was it filed a year ago? Was it fixed by something else at some point, but then got broken by bug 846569?

Last good revision: f003c386c77a (2013-11-08) First bad revision: 9e571ad29946 (2013-11-09) Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f003c386c77a&tochange=9e571ad29946

Regression window for the fix from an year ago: Last bad revision: 73f0c5b00572 (2013-02-26) First good revision: e7632ab657e5 (2013-02-27) Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=73f0c5b00572&tochange=e7632ab657e5

cade02c975db Sriram Ramasubramanian — Bug 842609: Scroll to selected tab in tabs ui. [r=mfinkle] ?

The patch for bug 846569 changed the logic order in updateSelectedPosition, and that's what appears to have caused this problem. I'm not familiar with TwoWayView, but I'm not sure if this is expected behavior, or perhaps some unexpected assumptions are baked into setSelectionFromOffset.

Comment on attachment 8379241 [details] [diff] [review] Update the selected tab style before calling setSelection Review of attachment 8379241 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/TabsTray.java @@ +201,5 @@ > > // Updates the selected position in the list so that it will be scrolled to the right place. > private void updateSelectedPosition() { > int selected = getPositionForTab(Tabs.getInstance().getSelectedTab()); > + updateSelectedStyle(selected); This is smelling like a TwoWayView bug. However, I also noticed that updateSelectedStyle() is going through all adapter items and setting their checked state one by one which is not really necessary and might be causing a excessive amount of requestLayout() calls in the view. The TabsTray view is using choiceMode=single which means we can just setItemChecked(selectedPosition, true) and TwoWayView will do the right thing for us. I'm fine with landing your patch but, before doing so, could you try just changing updateSelectedStyle() to just call TabsTray.this.setItemChecked(selected, true) to see if that fixes the bug?

(In reply to Lucas Rocha (:lucasr) from comment #14) > Comment on attachment 8379241 [details] [diff] [review] > Update the selected tab style before calling setSelection > > Review of attachment 8379241 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/TabsTray.java > @@ +201,5 @@ > > > > // Updates the selected position in the list so that it will be scrolled to the right place. > > private void updateSelectedPosition() { > > int selected = getPositionForTab(Tabs.getInstance().getSelectedTab()); > > + updateSelectedStyle(selected); > > This is smelling like a TwoWayView bug. However, I also noticed that > updateSelectedStyle() is going through all adapter items and setting their > checked state one by one which is not really necessary and might be causing > a excessive amount of requestLayout() calls in the view. The TabsTray view > is using choiceMode=single which means we can just > setItemChecked(selectedPosition, true) and TwoWayView will do the right > thing for us. > > I'm fine with landing your patch but, before doing so, could you try just > changing updateSelectedStyle() to just call > TabsTray.this.setItemChecked(selected, true) to see if that fixes the bug? Looking back at history, we used to to this, but we changed it at one point: http://hg.mozilla.org/mozilla-central/rev/fc9b2b7ebfbd In bug 834525 comment 8, Sriram mentions doing this to fix a focus issue. We can investigate whether or not this would still pose a problem, but I'd rather do that in another bug.

Comment on attachment 8379241 [details] [diff] [review] Update the selected tab style before calling setSelection [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 846569 User impact if declined: tabs tray is not scrolled to current tab when tabs tray is opened Testing completed (on m-c, etc.): tested locally, landed on fx-team Risk to taking this patch (and alternatives if risky): low-risk, reverting logic re-ordering from bug 846569 String or IDL/UUID changes made by this patch: none

Verified as fixed in builds: - 28 beta 6; - 29.0a2 (2014-02-25); - 30.0a1 (2014-02-25); Device: Google Nexus 7 (Android 4.4.2).