771816 - Create a pref to disable "Draw in titlebar"

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Richard Marti (:Paenglab)]

With landing of bug 755793 the toolbars are now shifted into the titlebar. With the menubar always shown this makes the area above the tabs tall and dominant. This bug is to investigate if we should leave it as it is or we should move the menubar below the tabs like under Vista/Win7.

Comment 1

[Richard Marti (:Paenglab)]

Andreas and Blake, I add you to this bug for a decision where the menubar should stay.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: Comparison with menu on top and below the tabs

What do you think, where should the menubar be placed?

I think we don't need to follow the system defaults because also the draw in titlebar isn't following the defaults.

Comment 3

[Andreas Nilsson (:andreasn)]

Comment on attachment 639971 [details]
Comparison with menu on top and below the tabs

I'm leaning towards drawing them under the tabs, like we do on Aero. It feels more solid.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 639971 [details]
Comparison with menu on top and below the tabs

So, I've emailed a few people who use XP on a regular basis, and the reaction has been… mixed.

I think this is one of those cases where we should add an option, since there are a couple of conflicting use-cases that I think we want to accommodate.  (Specifically, people who switch between XP and 7 want it to be below, but people who only use XP want it to be above.)

So, I'm going to say f-, but if it had an option (which defaulted to menu-on-top), I would give that an f+.

Thanks,
Blake.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

I agree that this is where we'd want a toggling option.

Here's what I suggest that we let users toggle between:

1)  Current Australis stuff, drawing in the titlebar, with the menu *below* the tabs for all Windows themes

2)  Menu above tabs, not drawing in titlebar. For Luna, we'd still have curvy tabs, but the background would be tan/grey instead of blue.  Not sure what it'd need to look like on Aero.

How does that sound?

-Mike

Comment 6

[Jim Porter (:squib)]

I think that sounds good. We definitely need to be able to disable drawing in the titlebar on XP, since on a few of the shipping-by-default themes (Luna Silver, for instance), it just looks broken.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Another question - do we expose this preference in the UI somewhere, or leave it in about:config?

Considering our timeline to merge to aurora, if we want to have it in the UI somewhere, we'd better either put the pedal to the metal, or maybe prepare and apply a backout patch for aurora for the TabsInTitlebar stuff after the merge takes place.

-Mike

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Richard:

What's the story on this? Are we OK to back out the TabsInTitlebar work until we can get this user pref in?

-Mike

Comment 9

[Richard Marti (:Paenglab)]

Mike, I'm okay with backing out. Especially for aurora.

Comment 10

[Richard Marti (:Paenglab)]

I renamed this bug to reflect what is needed.

Comment 11

[u382332]

May I ask what the Target Milestone will be for adding the pref?

Comment 12

[Mike Conley (:mconley) (:⚙️)]

(In reply to Bozz from comment #11)
> May I ask what the Target Milestone will be for adding the pref?

I'm hoping to clear this bug early next week. The target milestone will likely be TB 17.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Attachment: WIP Patch 1

Richard,

Here's my first crack at it. I've created a new pref 'mail.tabs.drawInTitlebar' - try toggling that with this patch.

We still might want to reduce some of the margin between the titlebar and the tabs when mail.tabs.drawInTitlebar is false.

Can you work with this?

-Mike

Comment 14

[Richard Marti (:Paenglab)]

Attachment: patch

Thanks a lot Mike for the important part. I had to move the two setAttribute lines a little bit upwards to be initialized before the widths and heights are calculated.

Blake, I've set ui-r? also when this patch adds no new UI but affects the UI.

Comment 15

[Richard Marti (:Paenglab)]

Attachment: patch v2

Removed two spaces in function rect(ele) ele.getBoundingClientRect();
and had to put a !important in mailWindow1-aero.css.

Comment 16

[Axel Grude [:realRaven]]

Attachment: windows xp with various luna themes

Here is the current broken look when using a theming program (windowblinds 7.41) on windows xp, which changes the blue (olive/silver) window frames. With the new menu that floats to the top menu items appear truncated. I have also raised [Bug 780303] which could be marked as duplicate I suppose.

Comment 17

[Axel Grude [:realRaven]]

(In reply to Richard Marti [:paenglab] from comment #2)
> Created attachment 639971 [details]
> Comparison with menu on top and below the tabs
> 
> What do you think, where should the menubar be placed?
> 
> I think we don't need to follow the system defaults because also the draw in
> titlebar isn't following the defaults.

We should enable a "OS standards compliant" mode, that allows to have the menu at the top (where it is with 99% of all windows software). For my personal work flow, I would not like the menu to move down, as I use it far less often than Thunderbird Tabs, Toolbar Buttons and QuickFolder Tabs. 

In order of frequency (and hence importance) - in my experience, I am using the UI elements that are the closest to the mail (center of screen) the most often (QF Tabs, for chaning folders), the buttons for mail actions and navigation, then Thunderbird Tabs (mainly to switch to tabs or calendar), and as least frequently used the menu items (debugging, options, or to make the add-ons tab visible). So I think to relocate the menu "just because it looks more solid" while at the same time disregarding ergonomics is a mistake in my opinion.

Comment 18

[Axel Grude [:realRaven]]

Comment on attachment 648899 [details]
windows xp with various luna themes

These are three different OS style themes: Ecliz, Vista32px and gaiety - among other things these support alpha transparency, shadows, blur, even in Windows XP. Also note that the system menu buttons (minimize/restore/max) are supplied by the Windowblinds Theme.

There are other theming engines that I would think use a similar algorithm to style window borders, titlebars and widgets, which "paint over" existing elements. Interestingly, when using themes which override OS widgets, these are honored by Windowblinds, so I believe titlebars are a special case, as they are probably tied closer to windows explorer or Luna.

Comment 19

[Richard Marti (:Paenglab)]

(In reply to Axel Grude [:realRaven] from comment #17)
> 
> We should enable a "OS standards compliant" mode, that allows to have the
> menu at the top (where it is with 99% of all windows software). For my
> personal work flow, I would not like the menu to move down, as I use it far
> less often than Thunderbird Tabs, Toolbar Buttons and QuickFolder Tabs.

It was already decided to leave the menu bar where it is. Also cause of this the bug topic was changed to the draw in titlebar yes/no pref.

Comment 20

[Richard Marti (:Paenglab)]

Windowblinds is a "hack" because it isn't registered on theme engine with all his colors. Good visible on top of your screenshot where Luna Silver background is used instead of the black background of the window frame. It overwrites the default window borders.

To make it short, we are only a user of the FX draw in titlebar functionality and can't change it, and with this bug's pref you can switch it off and all should be as before the draw in titlebar.

Comment 22

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 647467 [details] [diff] [review]
patch v2

UI-wise, I mostly like it, but I think (on Windows 7) we could make the tabs be pushed up more into the title bar.  Like having a margin-bottom of -24px instead of -14px on the #titlebar.  ui-r=me with that change.

>+++ b/mail/app/profile/all-thunderbird.js
>@@ -764,15 +767,15 @@
> pref("mail.cloud_files.learn_more_url", "https://support.mozillamessaging.com/kb/filelink-large-attachments");
> 
> // Sanitize dialog window
>-pref("privacy.cpd.cookies", true);
>+pref("privacy.cpd.cookies", true);
> pref("privacy.cpd.cache", true);
> 
>-// What default should we use for the time span in the sanitizer:
>-// 0 - Clear everything
>-// 1 - Last Hour
>-// 2 - Last 2 Hours
>-// 3 - Last 4 Hours
>-// 4 - Today
>+// What default should we use for the time span in the sanitizer:
>+// 0 - Clear everything
>+// 1 - Last Hour
>+// 2 - Last 2 Hours
>+// 3 - Last 4 Hours
>+// 4 - Today
> pref("privacy.sanitize.timeSpan", 1);

I'm not sure what changed here, but I suspect we don't want this change.  (Unless it's a windows newline thing that I just can't see. ;)

>+++ b/mail/themes/qute/mail/tabmail.css
>@@ -392,19 +392,22 @@
> @media (-moz-windows-theme: luna-blue) {
>-  .tabs-alltabs-button:not(:hover) > .toolbarbutton-menu-dropmarker:not(:-moz-lwtheme) {
>+  #messengerWindow[tabsintitlebar] .tabs-alltabs-button:not(:hover) >
>+  .toolbarbutton-menu-dropmarker:not(:-moz-lwtheme) {

I think this (and the next two cases of this) should be indented, so that we know that it's a continuation of the previous rule, and not a new rule.

Aside from those, I'm pretty happy with it.  r=me, with those fixed.

Thanks,
Blake.

Comment 23

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #22)
> Comment on attachment 647467 [details] [diff] [review]
> patch v2
> 
> UI-wise, I mostly like it, but I think (on Windows 7) we could make the tabs
> be pushed up more into the title bar.  Like having a margin-bottom of -24px
> instead of -14px on the #titlebar.  ui-r=me with that change.

You mean in normal window mode? It's intended to have a 16px space, like the Australis mockups are showing. I leave it like it is now, if you still want a slimmer titlebar we should do this in a own bug.

> >+++ b/mail/app/profile/all-thunderbird.js
> >@@ -764,15 +767,15 @@
> > pref("mail.cloud_files.learn_more_url", "https://support.mozillamessaging.com/kb/filelink-large-attachments");
> > 
> > // Sanitize dialog window
> >-pref("privacy.cpd.cookies", true);
> >+pref("privacy.cpd.cookies", true);
> > pref("privacy.cpd.cache", true);
> > 
> >-// What default should we use for the time span in the sanitizer:
> >-// 0 - Clear everything
> >-// 1 - Last Hour
> >-// 2 - Last 2 Hours
> >-// 3 - Last 4 Hours
> >-// 4 - Today
> >+// What default should we use for the time span in the sanitizer:
> >+// 0 - Clear everything
> >+// 1 - Last Hour
> >+// 2 - Last 2 Hours
> >+// 3 - Last 4 Hours
> >+// 4 - Today
> > pref("privacy.sanitize.timeSpan", 1);
> 
> I'm not sure what changed here, but I suspect we don't want this change. 
> (Unless it's a windows newline thing that I just can't see. ;)

It wasn't intended by me but it's a CR/LF to LF change.

> >+++ b/mail/themes/qute/mail/tabmail.css
> >@@ -392,19 +392,22 @@
> > @media (-moz-windows-theme: luna-blue) {
> >-  .tabs-alltabs-button:not(:hover) > .toolbarbutton-menu-dropmarker:not(:-moz-lwtheme) {
> >+  #messengerWindow[tabsintitlebar] .tabs-alltabs-button:not(:hover) >
> >+  .toolbarbutton-menu-dropmarker:not(:-moz-lwtheme) {
> 
> I think this (and the next two cases of this) should be indented, so that we
> know that it's a continuation of the previous rule, and not a new rule.

I changed it to this:

+  #messengerWindow[tabsintitlebar] .tabs-closebutton,
+  #messengerWindow[tabsintitlebar] .tabmail-tab:not([selected]) > .tab-stack >
+    .tab-content > .tab-close-button:not(:hover):not(:active) {
     -moz-image-region: rect(0, 64px, 16px, 48px);
   }

Okay?

Comment 24

[Blake Winton (:bwinton) (:☕️)]

(In reply to Richard Marti [:paenglab] from comment #23)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #22)
> > UI-wise, I mostly like it, but I think (on Windows 7) we could make the tabs
> > be pushed up more into the title bar.  Like having a margin-bottom of -24px
> > instead of -14px on the #titlebar.  ui-r=me with that change.
> 
> You mean in normal window mode? It's intended to have a 16px space, like the
> Australis mockups are showing. I leave it like it is now, if you still want
> a slimmer titlebar we should do this in a own bug.

Huh, so it is.  Wow, that seems really tall.  Okay, let's leave it.

> > >+++ b/mail/app/profile/all-thunderbird.js
> > >@@ -764,15 +767,15 @@
> > I'm not sure what changed here, but I suspect we don't want this change. 
> > (Unless it's a windows newline thing that I just can't see. ;)
> It wasn't intended by me but it's a CR/LF to LF change.

Okay, I like that.

> > >+++ b/mail/themes/qute/mail/tabmail.css
> > >@@ -392,19 +392,22 @@
> > I think this (and the next two cases of this) should be indented, so that we
> > know that it's a continuation of the previous rule, and not a new rule.
> 
> I changed it to this:
> 
> +  #messengerWindow[tabsintitlebar] .tabs-closebutton,
> +  #messengerWindow[tabsintitlebar] .tabmail-tab:not([selected]) >
> .tab-stack >
> +    .tab-content > .tab-close-button:not(:hover):not(:active) {
>      -moz-image-region: rect(0, 64px, 16px, 48px);
>    }
> 
> Okay?

Perfect!

Thanks,
Blake.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/2ffbab67153f