971630 - Australis: Far right/left selected overflow tabs look bad on session restore.

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Caspy7]

Attachment: Tab Overflow

I have several windows with tabs filling up and overflowing the tab bar.  On most windows the far right tab is selected.  After restoring a session, *all* of my windows whose tab rows are full and that have the rightmost tab selected are scrolled ever so slightly off the tab bar - the bottom curve it seems (see attached screenshot).
I tried reproducing this with a different profile (filling several windows with tabs) and ironically could only get the *leftmost* selected tab to trail off the end, like the screenshot, on session restore.

It's obviously not a showstopper, but it's poor polish and for someone like me with that tinge of OCD I have to go through and hit the scroll arrow on all the cases. It just looks bad.

Comment 2

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

Hm, I could reproduce this last week but can't anymore. Caspy7, can you still reproduce this on a new nightly build?

Comment 3

[Caspy7]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> Hm, I could reproduce this last week but can't anymore. Caspy7, can you
> still reproduce this on a new nightly build?

Still seeing this on the latest nightly.

Comment 4

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

Ok, I can reproduce with the left-most selected tab.

Comment 5

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

I'm pretty sure I've gotten it figured out now. Patch coming soon.

Comment 6

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

Attachment: Patch

Kinda stinks to reach in to private methods and properties of mTabstrip, but it's not the first place within tabbrowser.xml.

Comment 7

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

Attachment: Patch v1.01 (forgot to qref)

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8402892 [details] [diff] [review]
Patch v1.01 (forgot to qref)

I agree on both the gross-ness of sucking out private members of mTabstrip, and also at the sentiment that "we were doing it already, so what the hell."

We really should consider trying to expose those as public read-only members instead. I wouldn't block on it, but can you please file a bug to do that?

Also, have you done a try push to see how this could affect TART? If not, maybe do that. Or not, and take your chances on fx-team / m-c. :)

Comment 9

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

Baseline: https://tbpl.mozilla.org/?tree=Try&rev=41633908c8a5
With patch: https://tbpl.mozilla.org/?tree=Try&rev=a4b0bb1a83b9

Compare talos: http://compare-talos.mattn.ca/?oldRevs=41633908c8a5&newRev=a4b0bb1a83b9&server=graphs.mozilla.org&submit=true

Comment 10

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

Compare-talos shows that there is no TART regression from this patch.

Landed on fx-team: https://hg.mozilla.org/integration/fx-team/rev/6aef8ef7d775

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6aef8ef7d775

Comment 12

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

Comment on attachment 8402892 [details] [diff] [review]
Patch v1.01 (forgot to qref)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): introduction of tab curves missed this case that has to be manually adjusted for the negative margins on tab curves
User impact if declined: session restore can have partial tabs showing (curve is clipped)
Testing completed (on m-c, etc.): on m-c for a few days
Risk to taking this patch (and alternatives if risky): none expected
String or IDL/UUID changes made by this patch: none

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/2200d43e5485


Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/9798420b26fc

Comment 14

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Caspy7, can you try and see if this works now for you with the builds below?
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/29.0b8-candidates/build1/
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/

Comment 15

[Caspy7]

I tested with all builds and the problem appears resolved in all of them.