Last Comment Bug 776349 - Main toolbox still shown with all toolbars hidden
: Main toolbox still shown with all toolbars hidden
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on:
Blocks: 763308
  Show dependency treegraph
 
Reported: 2012-07-22 06:42 PDT by Richard Marti (:Paenglab)
Modified: 2012-07-24 01:14 PDT (History)
3 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed


Attachments
patch (3.73 KB, patch)
2012-07-22 07:05 PDT, Richard Marti (:Paenglab)
bwinton: review+
bwinton: approval‑comm‑aurora+
bwinton: approval‑comm‑beta+
Details | Diff | Review

Description Richard Marti (:Paenglab) 2012-07-22 06:42:26 PDT
When all toolbars are hidden through menu View > Toolbars the toolbox is still shown with a height of 41px.
Comment 1 Richard Marti (:Paenglab) 2012-07-22 07:05:52 PDT
Created attachment 644756 [details] [diff] [review]
patch

This patch removes the toolbox's min-height and adds margins to the toolbar. I can't give a min-height to the toolbar because this let's grow the buttons to full height. So I gave the toolbar top- and bottom-margins to become the height we need for the Australis theme. On Win7 and OSX this is easy with only one button height. On XP and Linux with the big and small icons it's a little bit trickier.

I set the :only-of-type to apply only when one toolbar is set. When two toolbars are set they are taller than the minimum height and the margins aren't needed. The only drawback with this is, when two toolbars are defined (with Add toolbar in Customize window) and one is hidden, then the only visible toolbar has no margins. But I think it's really seldom someone uses this configuration.
Comment 2 Blake Winton (:bwinton) (:☕️) (PTO 'til London. Find me there for quick answers!) 2012-07-23 10:25:47 PDT
Comment on attachment 644756 [details] [diff] [review]
patch

I like it.  r=me!

Thanks for the patch!
Comment 3 Richard Marti (:Paenglab) 2012-07-23 10:31:27 PDT
Comment on attachment 644756 [details] [diff] [review]
patch

[Approval Request Comment]
Bug 763308 is in aurora and beta and I think it makes sense this patch can also land there to solve the issue.
Comment 4 Blake Winton (:bwinton) (:☕️) (PTO 'til London. Find me there for quick answers!) 2012-07-23 12:00:55 PDT
Comment on attachment 644756 [details] [diff] [review]
patch

This fixes a fairly annoying case in Australis, and is just a theme change, so I think pulling it forward is the right move here.
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-07-23 16:34:10 PDT
https://hg.mozilla.org/comm-central/rev/f91c69072cf6
https://hg.mozilla.org/releases/comm-aurora/rev/414a1fb495be
https://hg.mozilla.org/releases/comm-beta/rev/1a93a4c3cb2a

Had to unbitrot c-a and c-b a bit, so please confirm that I didn't mess it up. Thanks.
Comment 6 :aceman 2012-07-24 00:18:24 PDT
Is this in any way related to bug 748555 (I wait for the current nightly to test it on Win XP)? If not, can you look at it while in the toolbar code? ;)
Comment 7 Richard Marti (:Paenglab) 2012-07-24 01:14:18 PDT
It isn't related to bug 748555. I'll comment in this bug.

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