Whoops, forgot description. Basically, for Australis we want to be able to draw in the window titlebar, so that we can do away with the window icon / title, and move our tabs into the freed real-estate.

Mental notes to self (trunk is building): on window xul: * chromemargin: 0,2,2,2; * padding-top: 20px; #messengerWindow { -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag"); } #tabs-toolbar { -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag"); }

Initial stab

Known issues: * Personas hides the min-max-close buttons * Need to skip the top padding when window is maximized.

If anyone runs this patch DON'T MAXIMIZE. It's apparently quite hard to get back again.

(In reply to Andreas Nilsson (:andreasn) from comment #5) > If anyone runs this patch DON'T MAXIMIZE. > It's apparently quite hard to get back again. Actually, false alarm. That was with a newer version of this patch where I had introduced a bunch of randomly exploding things.

Checkpoint before I start adding in exploding stuff.

This fixes the issues with the basic and classic themes so that we now get windows controls for that (only the minimize button works so far though).

This allows for lightweight themes to work, for some reason currently incompatible with -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag"); so that's commented out for now.

Allow dragging in tabs toolbar and using lwtheme now, but currently don't work for Aero.

All dragging works now, still some small quirks with the lwtheme window buttons though.

Just a small update allowing the maximize/resize and close buttons work in Basic under Win7.

Paenglab mentioned that we'll want to port Firefox's TabsInTitleBar to Thunderbird, so I'm going to try to do that today.

Checkpointing work, and stashing patch on Bugzilla so I can move it over to my Windows machine.

This is a patch with all needed XUL (I added the #ifdef CAN_DRAW_IN_TITLEBAR) and CSS changes. But still with a fixed margin-bottom of -12px for the titlebar. This needs to be dynamic for the different heights on different themes. Not included in this patch is Mikes patch to calculate the captionButtonsBox width. Mike, I saw you are also calculating the margin-bottom for the titlebar, right? In shorlander's mockups there is always a space of 16px between the window top and the tabs. Could you please attend this in this calculation? I would say margin-bottom= -(titlebar height - 16px) should do the trick.

I know, the tabbar buttons on the right collide with the caption buttons. This is due the missing width calculation of the captionButtonsBox. Also the space between the window top and the tabs isn't correct.

Mike, I forgot to write before, when you calculate the titlebar margin-bottom, could you set the margin when in #main-window[sizemode="maximized"] the gap is only 2px? When in maximized we don't need the gap of 16px for easier mouse grab to move the window.

(In reply to Mike Conley (:mconley) from comment #15) > Created attachment 633537 [details] [diff] [review] > Checkpointing work > > Checkpointing work, and stashing patch on Bugzilla so I can move it over to > my Windows machine. Mike, I tried your patch. When I start TB non-maximized then the calculations aren't done. When I maximize TB the calculations are done. The margin-bottom for the titlebar is only applied when maximied.

Fixed some margins to position the captionButtons on the right height.

Ok, I think we've got a working port here. Does this work for you, Richard?

I still see the problem I wrote in comment 19. I tried also the pref mail.tabs.drawInTitlebar set to false. The only change I see is the spacer box for the windowCaptions has a width of 0px. I see also never a attribute of tabsintitlebar="true" on #messengerWindow. Do we need a pref to choose between normal and draw in titlebar or should we hard code the draw in titlebar? When we hard code then we need no pref and all the calculations for the spacer box and the negative margin on titlebar should be done all the time.

I asked Andreas on IRC and he thinks, no pref is needed.

(In reply to Richard Marti [:paenglab] from comment #22) > I still see the problem I wrote in comment 19. Ah yeah, I'm seeing that too. Investigating a fix...

(In reply to Richard Marti [:paenglab] from comment #23) > I asked Andreas on IRC and he thinks, no pref is needed. So you're suggesting we axe the mail.tabs.drawInTitlebar pref?

(In reply to Mike Conley (:mconley) from comment #24) > (In reply to Richard Marti [:paenglab] from comment #22) > > I still see the problem I wrote in comment 19. > > Ah yeah, I'm seeing that too. Investigating a fix... Richard: So it looks like this issue is caused because there's a rule for #titlebar in mailWindow1.css that's causing margin-bottom to be -12px initially. If you remove the margin-bottom rule, does this fix the issue for you? -Mike

Hm, nevermind. That seems to break the maximized case. :/

(In reply to Mike Conley (:mconley) from comment #25) > (In reply to Richard Marti [:paenglab] from comment #23) > > I asked Andreas on IRC and he thinks, no pref is needed. > > So you're suggesting we axe the mail.tabs.drawInTitlebar pref? Yes, the draw in titlebar would be always enabled. Australis has this also on non-maximized windows. > So it looks like this issue is caused because there's a rule for #titlebar > in mailWindow1.css that's causing margin-bottom to be -12px initially. When the automatic calculation works also in non-maximized mode then this rule can be removed.

Okay, let's go this patch through review. Mike, please check my changes in JS carefully. You know I'm a noob in JS :)

Thing are looking really good! I've noticed that under both Aero and Basic there is a difference between the edges and the area for the tabs and window controls, but this can be addressed in a followup patch. Richard: If I recall correctly you have a XP VM. Can you post a screenshot how the patch looks under XP? I've only tried it it compability mode.

This is from a original XP. Thanks to my empoyer ;)

Comment on attachment 636093 [details] [diff] [review] patch for review Thanks for the screenshots! ui-review approved.

Comment on attachment 636093 [details] [diff] [review] patch for review Review of attachment 636093 [details] [diff] [review]: ----------------------------------------------------------------- This looks good - just one nit, and and a question. See below. ::: mail/base/content/msgMail3PaneWindow.js @@ +1601,5 @@ > + if (event.target != window) > + return; > + let sizemode = document.documentElement.getAttribute("sizemode"); > + TabsInTitlebar.allowedBy("sizemode", > + sizemode == "normal" || Hrm. I think sizemode can only have the values "normal", "maximized" and "fullscreen", so essentially, we're saying allowedBy("sizemode", true). In Firefox's implementation of TabsInTitlebar, they don't have sizemode == "normal" in there... What happens if we remove this? Do things still work as expected? (And, before you say it - I know, I'm likely the person who added sizemode == "normal". But I'm not entirely sure it's necessary anymore). ::: mail/themes/qute/mail/mailWindow1.css @@ +396,5 @@ > + background-color: InactiveCaption; > + } > +} > + > +/* Luna silver needs special colors. Default don't look good */ Grammar nit: should be "doesn't", not "don't".

(In reply to Mike Conley (:mconley) from comment #33) > Comment on attachment 636093 [details] [diff] [review] > Hrm. I think sizemode can only have the values "normal", "maximized" and > "fullscreen", so essentially, we're saying allowedBy("sizemode", true). It works with ("sizemode", true). Changed to this in new patch. > > +/* Luna silver needs special colors. Default don't look good */ > > Grammar nit: should be "doesn't", not "don't". fixed Taking over ui-r+ from previous patch.

(In reply to Richard Marti [:paenglab] from comment #34) > Created attachment 639362 [details] [diff] [review] > Patch v11 > > (In reply to Mike Conley (:mconley) from comment #33) > > Comment on attachment 636093 [details] [diff] [review] > > Hrm. I think sizemode can only have the values "normal", "maximized" and > > "fullscreen", so essentially, we're saying allowedBy("sizemode", true). > > It works with ("sizemode", true). Changed to this in new patch. Yeah, it works, but *should* it? Like, shouldn't we be doing what Firefox does and set allowedBy to only be when sizemode is maximized / fullscreen? Does it work with just those two?

(In reply to Mike Conley (:mconley) from comment #35) > Yeah, it works, but *should* it? Like, shouldn't we be doing what Firefox > does and set allowedBy to only be when sizemode is maximized / fullscreen? > > Does it work with just those two? I'd say it should also be in normal mode. When you look at http://25.media.tumblr.com/tumblr_m0iwtyy6nh1qkoea4o1_1280.png you see it's a window in normal mode. Also on http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-designSpecs-windowsXP-lunaBlue-mainWindow.html and the other XP color pages or http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-designSpecs-windows7-mainWindow.html you can see the windows in draw in titlebar mode. On the Win7 page you can also see a image with "Tabs in a Maximized Window". The only problem I'm seeing is on XP with menu enabled. I think the menu should be placed below the tabs like on Win7 and not on top in the titlebar. For this I'm planning to open a bug after this bug's landing.

Comment on attachment 639362 [details] [diff] [review] Patch v11 Review of attachment 639362 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Richard Marti [:paenglab] from comment #36) > (In reply to Mike Conley (:mconley) from comment #35) > > Yeah, it works, but *should* it? Like, shouldn't we be doing what Firefox > > does and set allowedBy to only be when sizemode is maximized / fullscreen? > > > > Does it work with just those two? > > I'd say it should also be in normal mode. > > When you look at > http://25.media.tumblr.com/tumblr_m0iwtyy6nh1qkoea4o1_1280.png you see it's > a window in normal mode. Also on > http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis- > designSpecs-windowsXP-lunaBlue-mainWindow.html and the other XP color pages > or > http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis- > designSpecs-windows7-mainWindow.html you can see the windows in draw in > titlebar mode. On the Win7 page you can also see a image with "Tabs in a > Maximized Window". > > The only problem I'm seeing is on XP with menu enabled. I think the menu > should be placed below the tabs like on Win7 and not on top in the titlebar. > For this I'm planning to open a bug after this bug's landing. Ah, fair enough, I was going off of what was currently in Firefox's implementation. Ok, on that note, just one final fix, and then r=me. Great job Richard, as usual. ::: mail/base/content/msgMail3PaneWindow.js @@ +1599,5 @@ > + this.allowedBy("sizemode", false); > + window.addEventListener("resize", function (event) { > + if (event.target != window) > + return; > + let sizemode = document.documentElement.getAttribute("sizemode"); Since we're not checking the window's sizemode anymore, we can get rid of line 1603.

Patch addressing the last comment. Taking the r+ and ui-r+ from previous patch.

Sorry, but I had to back this out due to test failures on Mac and Linux https://hg.mozilla.org/comm-central/rev/a9ea38d7a17c https://tbpl.mozilla.org/php/getParsedLog.php?id=13278934&tree=Thunderbird-Trunk TEST-START | /home/cltbld/talos-slave/test/build/mozmill/account/test-account-port-setting.js | teardownModule Test Failure: You need to specify a tab! TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/mozmill/account/test-account-port-setting.js | test-account-port-setting.js::teardownModule

Grrr - I forgot that we were using preprocessor stuff here, and forgot to test on Linux and OSX (since this patch really doesn't affect them). Anyhow, that's what broke us - in the case where CAN_DRAW_IN_TITLEBAR is false, we rendered busted Javascript.

Comment on attachment 639721 [details] [diff] [review] Fix preprocessor stuff for pinstripe / Linux Blake: I already reviewed Paenglab's version of this, but I had to modify it by adding some more preprocessor stuff in TabsInTitlebar. rs me? -Mike

Comment on attachment 639721 [details] [diff] [review] Fix preprocessor stuff for pinstripe / Linux rs=bwinton on this. I think we're OK to mark this as checkin-needed now.

Marking this as tracking TB 16, because we want to back it out once we hit the Aurora channel.

Paenglab: Here's the backout patch - but it doesn't apply cleanly. It seems to conflict with changes in primaryToolbar-aero.css. Can you resolve the conflict? -Mike

Bug 767162 made the problem. Patch adapted for this change.

Backed out of comm-aurora (TB 16): https://hg.mozilla.org/releases/comm-aurora/rev/6c2c3ee473c6

Just wanted to note that drawing in titlebar causes unreadable visuals with the official Zune theme [1]. [1]: http://go.microsoft.com/fwlink/?LinkID=75078

Of course, it's all OK if mail.tabs.drawInTitlebar disabled.

It looks I haven't the official Zune theme. Please can you open a new bug and CC me to it?

Is the Zune theme specific only to Vista? I don't seem to see it in Win7.

Zune is Win XP.

Fixed? This looks awful in Windows XP using any custom theme I have tried. The title bar simply is not being drawn - it is showing as the colour of the menu background. The work being done on Firefox to implement the Australis design looks to be working well with custom themes - I'm guessing similar has not been done here? I can turn the 'draw in titlebar' off for now, but it would be nice if it worked as I would like the tabs up there. Also, turning this on by default without warning seemed a poor decision as it looks so bad on some systems

Hi Jonrandy, what you describe sounds like a different issue to me, but one I would like to see fixed. Could you please file a new bug, cc-ing Paneglab and I, listing the steps you take to see the issue, what you see, and what you expected to see? (Also, if you could avoid phrases like "looks awful" in the new bug, that would make it easier for me to convince people to help fix it… :) Thank you, Blake.