554311 - Shredder layout needs to be align after Bug 538187

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Nomis101]

Attachment: Layout glitches (Mac OS X)

Since Bug 538187 (or Bug 508482) lands, Thunderbird trunk have a few layout glitches. You can see in the screenshot, the activated tab is too bright and the color of the selected account is now grey.
The screenshot is for Mac OS X, I don't know if it looks the same for Windows and Linux.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; de; rv:1.9.3a4pre) Gecko/20100322 Thunderbird/3.2a1pre

Comment 1

[Nomis101]

Attachment: incomplete patch (that partially fixes 3.2, but breaks 3.1)

This patch fixes the broken appearance of tabs in 3.2. But with this patch the tabs aren't bright like the toolbar if TB is not the foreground application, and it breaks 3.1 in the same way. :-(
And this patch doesn't fix the broken appearance of the folder pane and the addressbook.

Comment 2

[Markus Stange [:mstange]]

These are the lines that need changing:
http://mxr.mozilla.org/comm-central/search?string=[active&find=pinstripe.*\.css%24&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

If comm-central is still used for both mozilla-1.9.2 and mozilla-central, then the patch might need some ifdefs.

Comment 3

[Nomis101]

Attachment: still incomplete, but much better patch (for 3.2)

Thanks for this helpfull search string.
I've tried to fix all this with my new patch. This is a much better patch, because it also fixes the folder pane and the addressbook. But it still has issues (for inactive tabs and for the activity clear button), I will try to fix this. And it still only works for 3.2.
If I ifdef the CSS file for 3.1, what should I use? Does something like
> ifdef MOZILLA_1_9_2_BRANCH
work or is this only for makefiles?

Comment 4

[Nomis101]

Attachment: patch for 3.2

OK, this patch fixes all glitches / issues I can see on 3.2 trunk. It only need to ifdef for 3.1...

Comment 5

[Mark Banner (:standard8)]

As I've just sent to tb-planning, the current plan is to branch just before b2, which is in about 2 weeks.

Seeing as this bug seems to be 3.2 only, could it wait on landing until we've branched and avoid the ifdefs (which are quite different to normal in css I believe)?

Comment 6

[Markus Stange [:mstange]]

(In reply to comment #5)
> Seeing as this bug seems to be 3.2 only, could it wait on landing until we've
> branched and avoid the ifdefs (which are quite different to normal in css I
> believe)?

I think it can wait.

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 439221 [details] [diff] [review]
patch for 3.2

>diff -r 793f932b45d5 mail/themes/pinstripe/mail/primaryToolbar.css

>-#messengerWindow:not([active="true"]) > #mail-toolbox > toolbar {
>+#mail-toolbox:-moz-window-inactive > toolbar {

#mail-toolbox > toolbar:-moz-window-inactive might be faster.

>diff -r 793f932b45d5 mail/themes/pinstripe/mail/mailWindow1.css

>-:root:not([active]) #folderPaneHeader,
>-:root:not([active]) #folderTree {
>+#folderPaneHeader,
>+#folderTree {

You're missing :-moz-window-inactive here.

Other than that, this looks great!

Comment 8

[Nomis101]

Attachment: patch for TB 3.2 (v2)

Thanks for looking into my patch. I fixed your comments.
With this patch TB 3.2 looks the same as 3.1 in my quick test and I couldn't find any issue.
And I also think this can wait until we branch (this makes it much easier).

Comment 9

[Nomis101]

Attachment: patch for TB 3.2 (v2)

I guess, because this is only a bugfix, I don't need ui-review for this patch, right?

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 443629 [details] [diff] [review]
patch for TB 3.2 (v2)

Yep, though I'm going redirect to philor as he's better at css and I'm mainly focussed on 3.1 work at the moment.

Comment 11

[Nomis101]

Should I attach before-and-after screenshots, so it is easier to review? Or a patched classic.jar (so you can download a normal 3.2 build, replace the /Thunderbird.app/Contents/MacOS/chrome/classic.jar and see the effect)?

Comment 12

[Nomis101]

Pinging for review (before my patch gets bitrotted)

Comment 13

[Ludovic Hirlimann [:Usul]]

Magnus do you have some bandwith for reviewing this patch ?

Comment 14

[Andreas Nilsson (:andreasn)]

Comment on attachment 443629 [details] [diff] [review]
patch for TB 3.2 (v2)

Woohoo! The code is much more readable now! (and visually it looks good again).

The only thing I reacted on was that line 42 in tabmail.css used to specify .tabmail-strip in #messengerWindow, instead of just .tabmail-strip, but I think that's a good thing, in case a extension wants to do something with a tabmail-strip outside #messengerWindow

r+

Comment 15

[Andreas Nilsson (:andreasn)]

do we need a ui-review on this one as well?

Comment 16

[Nomis101]

Thanks for reviewing. :-)

(In reply to comment #15)
> do we need a ui-review on this one as well?

I've asked in Comment 9 if ui-review is needed for this patch. And if I interpret Marks response (#10) correct, than it isn't.

Comment 17

[Justin Wood (:Callek)]

http://hg.mozilla.org/comm-central/rev/f5e3db79ecb2