980043 - [Australis] new tab button not moved out of tab overflow area when there is only one tab

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Christian Ascheberg]

Attachment: screenshot.png

The "new tab" button is not moved out of the tab overflow area when there is only one tab open (at minimum window with).

STR:
1. open a window with only one tab open (it is even worse in private mode)
2. tab groups icon needs to be visible (use that feature once)
3. resize window to minimum width
4. open more tabs

Result:
- the "new tab" button is within the tab overflow area all the time and not visible, see screenshot

I think this bug can occur since bug 951928 landed.

Comment 1

[Justin Dolske [:Dolske]]

Hmm, I don't seem to be able to reproduce this on OS X.

What happens after resizing the window to make it bigger? Does the new tab button return?

Comment 2

[Christian Ascheberg]

(In reply to Justin Dolske [:Dolske] from comment #1)
> What happens after resizing the window to make it bigger? Does the new tab
> button return?

The button does not fully disappear, it is still reachable by scrolling the tabstrip (although scrolling behaves somewhat strangely in this narrow window). So yes, it becomes visible if the window is wider.
I just think the tab button should be outside the scrollable area, between the scroll-arrow-icon and the group tabs icon (as it is the case when there are two or more tabs that can be scrolled).

Comment 3

[Christian Ascheberg]

Attachment: screenshot, comparison

upper window:
- same as the first screenshot, but tabstrip scrolled to the right
- one tab open, new tab button is within the scrollable area

lower window:
- correct behavior
- two tabs open, new tab button is outside the scrollable area

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

I'm pretty sure this is due to http://hg.mozilla.org/mozilla-central/annotate/28dc0100f9b0/browser/base/content/tabbrowser.xml#l3888

Comment 5

[Christian Ascheberg]

Attachment: patch v1

This fixes the bug by reverting half of the patch of bug 905695. I am not sure about the other half though.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Christian, do you have access to tryservers? I think we can just back out the whole patch for bug 905695, but we should push the before+after to tryserver and run it through the Talos test suite to see if removing it will hurt perf.

You can use the following try syntax:
try: -b do -p linux,macosx64,win32 -u mochitest-bc -t other,svgr

Comment 7

[Christian Ascheberg]

No, I don't have tryserver access.
Don't you think just backing out half of the patch might keep some perf improvements? Or does it cause other problems?

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Ok, I can do the try push. We never noticed a big win from that patch, and introducing special cases introduces subtle bugs (like this one).

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Baseline: https://tbpl.mozilla.org/?tree=Try&rev=758e3f586aa7
W/ backout: https://tbpl.mozilla.org/?tree=Try&rev=8aeccaa5aafc
Compare talos: http://compare-talos.mattn.ca/?oldRevs=758e3f586aa7&newRev=8aeccaa5aafc&server=graphs.mozilla.org&submit=true

Comment 10

[Christian Ascheberg]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> Compare talos: http://compare-talos.mattn.ca/?oldRevs=758e3f586aa7&newRev=8aeccaa5aafc&server=graphs.mozilla.org&submit=true

What do you think about the results?

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Sorry, I meant to follow up on this already. The results show that we can just do a full backout of that patch. I'll request review on the backout.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: backout-7e0adf1211a7

We don't normally request review on a straight backout, but since this has hit release already I'll request review.

Comment 13

[Matthew N. [:MattN]]

I re-triggered the remaining talos jobs with only one run since this was showing a 5% tpaint regression on Win8 (which is one of the test+platform combos we had a win with the original patch).

Comment 14

[Matthew N. [:MattN]]

Comment on attachment 8429488 [details] [diff] [review]
backout-7e0adf1211a7

Okay, that was just noise.

Comment 15

[Carsten Book [:Tomcat]]

landed as https://hg.mozilla.org/integration/fx-team/rev/e017c15325ae

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/e017c15325ae

Comment 17

[Cristina Madaras, QA [:CristinaM]]

Reproduced the issue on Nightly (2012-03-05) buildID: 20140305030201, verified as fixed on:
Latest Nightly 33.0a2 (buildID: 20140715030207) 
Latest Aurora 32.0a1 (buildID: 20140715004003)under Windows 7 64bit.