Last Comment Bug 971630 - Australis: Far right/left selected overflow tabs look bad on session restore.
: Australis: Far right/left selected overflow tabs look bad on session restore.
Status: VERIFIED FIXED
[Australis:P3-]
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: unspecified
: x86_64 Windows 7
-- normal (vote)
: Firefox 31
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
: Dão Gottwald [:dao]
Mentors:
: 973340 (view as bug list)
Depends on:
Blocks: australis-tabs
  Show dependency treegraph
 
Reported: 2014-02-12 01:10 PST by Caspy7
Modified: 2014-04-17 10:22 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified
verified
verified


Attachments
Tab Overflow (16.80 KB, image/png)
2014-02-12 01:10 PST, Caspy7
no flags Details
Patch (3.22 KB, patch)
2014-04-07 14:07 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch v1.01 (forgot to qref) (3.39 KB, patch)
2014-04-07 14:08 PDT, Jared Wein [:jaws] (please needinfo? me)
mconley: review+
sledru: approval‑mozilla‑aurora+
sledru: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Caspy7 2014-02-12 01:10:14 PST
Created attachment 8374695 [details]
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 1 User image :Gijs 2014-02-18 13:31:35 PST
*** Bug 973340 has been marked as a duplicate of this bug. ***
Comment 2 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-03 10:29:00 PDT
Hm, I could reproduce this last week but can't anymore. Caspy7, can you still reproduce this on a new nightly build?
Comment 3 User image Caspy7 2014-04-06 03:00:15 PDT
(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 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-07 08:18:04 PDT
Ok, I can reproduce with the left-most selected tab.
Comment 5 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-07 13:12:23 PDT
I'm pretty sure I've gotten it figured out now. Patch coming soon.
Comment 6 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-07 14:07:51 PDT
Created attachment 8402890 [details] [diff] [review]
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 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-07 14:08:48 PDT
Created attachment 8402892 [details] [diff] [review]
Patch v1.01 (forgot to qref)
Comment 8 User image Mike Conley (:mconley) 2014-04-08 08:47:57 PDT
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 10 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-08 14:00:01 PDT
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 User image Carsten Book [:Tomcat] 2014-04-09 05:37:49 PDT
https://hg.mozilla.org/mozilla-central/rev/6aef8ef7d775
Comment 12 User image Jared Wein [:jaws] (please needinfo? me) 2014-04-11 13:30:27 PDT
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 15 User image Caspy7 2014-04-15 08:55:49 PDT
I tested with all builds and the problem appears resolved in all of them.

Note You need to log in before you can comment on or make changes to this bug.