Closed Bug 890546 Opened 8 years ago Closed 8 years ago

Using lightweight themes causes the tab bar to move down 16px.


(Thunderbird :: Mail Window Front End, defect)

Not set


(thunderbird24+ fixed)

Thunderbird 25.0
Tracking Status
thunderbird24 + fixed


(Reporter: jsbruner, Assigned: jsbruner)



(1 file, 2 obsolete files)

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.
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 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.
Attached patch Fix. (obsolete) β€” β€” Splinter Review
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?
Attachment #771684 - Flags: review?(mconley)
Comment on attachment 771684 [details] [diff] [review]

So, umm. Mike's on vacation. So I'll let Blake do the review instead. ;)
Attachment #771684 - Flags: review?(mconley) → review?(bwinton)
Comment on attachment 771684 [details] [diff] [review]

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.
Attachment #771684 - Flags: review?(bwinton)
Attached patch Fix. (obsolete) β€” β€” Splinter Review
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. :)
Attachment #771684 - Attachment is obsolete: true
Attachment #771709 - Flags: review?(richard.marti)
Comment on attachment 771709 [details] [diff] [review]

Looks good to me. Please add a linefeed in comment to make the line not longer than 80 chars.
Attachment #771709 - Flags: review?(richard.marti) → review+
Attached patch Fix. β€” β€” Splinter Review
Attachment #771709 - Attachment is obsolete: true
Attachment #771784 - Flags: review+
Requesting checkin, though I think our tree will be closed for awhile...
Keywords: checkin-needed
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 25.0
Comment on attachment 771784 [details] [diff] [review]

[Triage Comment]
Taking to 24 as the comments say we need it there as well, and it is low risk
Attachment #771784 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.