When new tabs are opened in excess to the available estate given in the new tab-bar, the tab-bar should automatically scroll to the new tab (and perhaps, stack the other tabs akin to the side to make room for the new tab). Steps to replicate on Nightly (09/15): keep opening new tabs; see that tabs are off-screen. The tab-bar should scroll.

The new tab should also fit entirely within view. The cut-off tab should not mingle with the tab button

Yeah, there's a lot of work to be done around managing tab visibility in the strip.

Comment on attachment 8511997 [details] [diff] [review] Scroll ensure selected tab is always fully visible (r=mcomella) Ensures the selected tab in the strip becomes full visible.

Comment on attachment 8511997 [details] [diff] [review] Scroll ensure selected tab is always fully visible (r=mcomella) Review of attachment 8511997 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the suggested change (if you agree). ::: mobile/android/base/tabs/TabStripView.java @@ +85,5 @@ > + treeObserver.addOnPreDrawListener(new OnPreDrawListener() { > + @Override > + public boolean onPreDraw() { > + if (treeObserver.isAlive()) { > + treeObserver.removeOnPreDrawListener(this); tldr; Get the view tree observer again instead of keeping the reference. Looking at the Android source, it looks like the observer only gets killed when it is merged into another observer [1]. This only seems to occur with the View.mFloatingTreeObserver getting merged (and killed) into the View.mAttachInfo.mTreeObserver [2]. The mFloatingTreeObserver seems temporary until the mAttachInfo is initialized (View.dispatchAttachedToWindow [3]) and a new instance will be constructed when mAttachInfo is cleared and the view tree observer is needed (View.dispatchDetachedFromWindow [4]). The AttachInfo where the observer is attached to the ViewRootImpl [5] so this AttachInfo will be re-used each time we're attached. I *assume* that we will never have onPreDraw called when we're not attached so we should always be getting an observer that has our listener when calling getViewTreeObserver (it's either on the initial floating tree observer, or stuck forever to the attachInfo). However, I don't think our long-lived reference is necessarily valid. [1]: http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ViewTreeObserver.java#378 [2]: http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/View.java#12577 [3]: http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/View.java#12569 [4]: http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/View.java#12640 [5]: http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ViewRootImpl.java#365

(In reply to Michael Comella (:mcomella) from comment #5) > Comment on attachment 8511997 [details] [diff] [review] > Scroll ensure selected tab is always fully visible (r=mcomella) > > Review of attachment 8511997 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ with the suggested change (if you agree). > > ::: mobile/android/base/tabs/TabStripView.java > @@ +85,5 @@ > > + treeObserver.addOnPreDrawListener(new OnPreDrawListener() { > > + @Override > > + public boolean onPreDraw() { > > + if (treeObserver.isAlive()) { > > + treeObserver.removeOnPreDrawListener(this); > > tldr; Get the view tree observer again instead of keeping the reference. > > Looking at the Android source, it looks like the observer only gets killed > when it is merged into another observer [1]. This only seems to occur with > the View.mFloatingTreeObserver getting merged (and killed) into the > View.mAttachInfo.mTreeObserver [2]. The mFloatingTreeObserver seems > temporary until the mAttachInfo is initialized > (View.dispatchAttachedToWindow [3]) and a new instance will be constructed > when mAttachInfo is cleared and the view tree observer is needed > (View.dispatchDetachedFromWindow [4]). > > The AttachInfo where the observer is attached to the ViewRootImpl [5] so > this AttachInfo will be re-used each time we're attached. > > I *assume* that we will never have onPreDraw called when we're not attached > so we should always be getting an observer that has our listener when > calling getViewTreeObserver (it's either on the initial floating tree > observer, or stuck forever to the attachInfo). However, I don't think our > long-lived reference is necessarily valid. > > [1]: > http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ > ViewTreeObserver.java#378 > [2]: > http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ > View.java#12577 > [3]: > http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ > View.java#12569 > [4]: > http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ > View.java#12640 > [5]: > http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/view/ > ViewRootImpl.java#365 Fair enough, done. Great digging btw :-)