Last Comment Bug 890546 - Using lightweight themes causes the tab bar to move down 16px.
: Using lightweight themes causes the tab bar to move down 16px.
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: Thunderbird 25.0
Assigned To: Josiah Bruner [:JosiahOne] (needinfo for responses)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-05 12:31 PDT by Josiah Bruner [:JosiahOne] (needinfo for responses)
Modified: 2013-07-23 06:41 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed


Attachments
Fix. (1.32 KB, patch)
2013-07-05 13:50 PDT, Josiah Bruner [:JosiahOne] (needinfo for responses)
no flags Details | Diff | Review
Fix. (1014 bytes, patch)
2013-07-05 15:38 PDT, Josiah Bruner [:JosiahOne] (needinfo for responses)
richard.marti: review+
Details | Diff | Review
Fix. (1.01 KB, patch)
2013-07-06 09:59 PDT, Josiah Bruner [:JosiahOne] (needinfo for responses)
josiah: review+
standard8: approval‑comm‑aurora+
Details | Diff | Review

Description Josiah Bruner [:JosiahOne] (needinfo for responses) 2013-07-05 12:31:46 PDT
Using lightweight themes causes the tab bar to be pushed down 16px. I know the cause and will have a patch up shortly. Also going to test if this is an issue on all platforms, or just OS X.
Comment 1 Richard Marti (:Paenglab) 2013-07-05 13:27:09 PDT
This also happens with TB 24. This is onlyvisible by applying the LW-theme. After restart with the LW-theme the margin-bottom is correctly on -6px.

I suppose at TB start without LW-theme the titlebar is hidden and the margin calculation in http://mxr.mozilla.org/comm-central/source/mail/base/content/msgMail3PaneWindow.js#1666 can't calculate the correct margin. Applying then a LW-theme bring then the incorrect margin-bottom. After restart with LW-theme the titlebar is not hidden and the calculation can work correctly.
Comment 2 Josiah Bruner [:JosiahOne] (needinfo for responses) 2013-07-05 13:50:31 PDT
Created attachment 771684 [details] [diff] [review]
Fix.

Richard pretty much had it right, except we want the margin-bottom to be 0px, not -6px.

In fact, why this whole margin-manipulation was ever added is beyond me. That is for drawing the tabs in the titlebar, which we aren't doing yet. Probably some mistake when draw in titlebar was brought over to TB for OS X.

Anyway, this patch fixes the issue by completely removing this margin-bottom modification. Mike, could you take a few minutes to review this?
Comment 3 Josiah Bruner [:JosiahOne] (needinfo for responses) 2013-07-05 13:55:06 PDT
Comment on attachment 771684 [details] [diff] [review]
Fix.

So, umm. Mike's on vacation. So I'll let Blake do the review instead. ;)
Comment 4 Richard Marti (:Paenglab) 2013-07-05 14:10:38 PDT
Comment on attachment 771684 [details] [diff] [review]
Fix.

This will break on Windows draw in titlebar. The gap above the tabs has to be 16px (for this is the calculation). Maybe it's better to set in messenger.css on #titlebar a margin-bottom: 0px !important. When draw in titlebar comes to OS X then this can be removed and the calculation is working correct also under OS X.
Comment 5 Josiah Bruner [:JosiahOne] (needinfo for responses) 2013-07-05 15:38:51 PDT
Created attachment 771709 [details] [diff] [review]
Fix.

Well, I really would have preferred to use #ifdefs to avoid CSS overrides, but it seems #ifdefs can't be nested in js files. Therefore, I went ahead and just did the CSS override.

Richard, now you can review this. :)
Comment 6 Richard Marti (:Paenglab) 2013-07-06 01:22:34 PDT
Comment on attachment 771709 [details] [diff] [review]
Fix.

Looks good to me. Please add a linefeed in comment to make the line not longer than 80 chars.
Comment 7 Josiah Bruner [:JosiahOne] (needinfo for responses) 2013-07-06 09:59:43 PDT
Created attachment 771784 [details] [diff] [review]
Fix.
Comment 8 Josiah Bruner [:JosiahOne] (needinfo for responses) 2013-07-06 10:02:01 PDT
Requesting checkin, though I think our tree will be closed for awhile...
Comment 9 Ryan VanderMeulen [:RyanVM] 2013-07-11 08:38:58 PDT
https://hg.mozilla.org/comm-central/rev/490d830d4e1a
Comment 10 Mark Banner (:standard8) 2013-07-23 04:40:15 PDT
Comment on attachment 771784 [details] [diff] [review]
Fix.

[Triage Comment]
Taking to 24 as the comments say we need it there as well, and it is low risk
Comment 11 Mark Banner (:standard8) 2013-07-23 06:41:43 PDT
https://hg.mozilla.org/releases/comm-aurora/rev/776a4660e156

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