597649 - Use the Firefox Tabs on Linux

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Richard Marti (:Paenglab)]

This is a adaption of the Firefox Tabs for Linux. With this patch the Tabs in Thunderbird looks similar to Firefox.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: Proposed patch

This is mostly a copy of the Firefox CSS.

Comment 2

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 476498 [details] [diff] [review]
Proposed patch

This looks pretty good to me.  I'm handing the ui-review off to andreas while I get my Linux box building again

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

Attachment: compare of tabs in linux

here's a screenshot comparing the two tab styles.  Current is at the bottom and new is at the top.  I like the new except that the style looks a little odd against the quick filter bar.

Any ideas for the quick filter bar?

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Also I had to move the tab.png icon from qute into gnomestripe directory for the patch to compile correctly.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: Fix the tab.png location

Comment 6

[Richard Marti (:Paenglab)]

Attachment: Tabs with and without Border on bottom

The picture from clarkbw shows only the old Tab style.

My picture shows the new style. At the top like in the patch without border on bottom. At the bottom with border to separate the quick filter bar. Does this look better?

Comment 7

[Andreas Nilsson (:andreasn)]

Thanks for the patch, and sorry for the delay in reply. Been ill.
A border is good, but it would be good if it didn't cover the active tab. If I recall correctly, that is a bit tricky to do due to the issues highlighted in bug 559101. I might be wrong though.

Comment 8

[Andreas Nilsson (:andreasn)]

Attachment: tab color issue

Another issue I noted is that the tab color and the content colors are different. This is because the FX tabs are intended to have a toolbar under them by default that have a lighter color. Not sure how big of an issue that is though.

Comment 9

[Andreas Nilsson (:andreasn)]

Comment on attachment 479075 [details] [diff] [review]
Fix the tab.png location

In general I really like this patch, but it looks odd without the line at the bottom (one that doesn't cover the active tab too), so ui-r- until that's fixed.

Comment 10

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

Comment on attachment 479075 [details] [diff] [review]
Fix the tab.png location

(Clearing out my review request, since it got a ui-r-.

But I will note that we should have spaces after each comma, and try to wrap the lines before 80 characters, and the second part of the "tab.png" line should line up with the second part of the "red_pin.png" line.

Later,
Blake.)

Comment 11

[Richard Marti (:Paenglab)]

Attachment: Patch addressing the comments

First only ui-r

Now with line on bottom except on active tab. Added on qfb-show-filter-bar button a margin-bottom: 1px to match the hovered tabs-alltabs-button.

(In reply to comment #10)
> But I will note that we should have spaces after each comma, and try to wrap
> the lines before 80 characters, 

This should be okay now.

> and the second part of the "tab.png" line
> should line up with the second part of the "red_pin.png" line.

I don't understand what you mean.

Comment 12

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

Here's what I see when I click on the attachment name 

http://cl.ly/7264ea85e0d1d0e13a5c

See how "(mail/icons/tab.png)" doesn't line up with "(mail/icons/red_pin.png)"?

(I won't block the checkin if you can't see where it's happening, but if you can figure it out, it would be nice to fix.)

Thanks,
Blake.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: jar.mn aligned

Now I understand what you meant.

Comment 14

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

Comment on attachment 481039 [details] [diff] [review]
jar.mn aligned

>+++ b/mail/themes/gnomestripe/mail/tabmail.css
>@@ -38,9 +38,11 @@
>-.tabmail-tabs {
>+.tabmail-tabs:not(:-moz-lwtheme) {
>   padding-top: 0px;
>   -moz-padding-start: 0px;

So, if there is a lightweight theme, how much padding will the tabmail-tabs have on the top and start?

>@@ -64,12 +66,61 @@
> .tabmail-tab {
>-  border: none !important;
>-  padding: 0px;
>-  margin-bottom: 1px;
>+  position: static;
>+  -moz-appearance: none;
>+  background: -moz-linear-gradient(hsla(0, 0%, 100%, .2),
>+                           hsla(0, 0%, 45%, .2) 1px, hsla(0, 0%, 32%, .2) 50%);

The indentation on this looks a little weird.

Also, Should this be background-image, like in the other .tabmail-tab… blocks down below?

>+  background-size: 200%;

Could you not skip this and double the appropriate values in the background/background-image element?

Other than those, I think I like it.  Of course, Andreas has to think it looks good, too.  :)

Later,
Blake.

Comment 15

[Andreas Nilsson (:andreasn)]

(In reply to comment #14)
> Comment on attachment 481039 [details] [diff] [review]
> jar.mn aligned
> 
> >+++ b/mail/themes/gnomestripe/mail/tabmail.css
> >@@ -38,9 +38,11 @@
> >-.tabmail-tabs {
> >+.tabmail-tabs:not(:-moz-lwtheme) {
> >   padding-top: 0px;
> >   -moz-padding-start: 0px;
> 
> So, if there is a lightweight theme, how much padding will the tabmail-tabs
> have on the top and start?

I couldn't see any visual difference between using or not using a lightweight theme (did a overlay in GIMP).

Comment 16

[Andreas Nilsson (:andreasn)]

Comment on attachment 481039 [details] [diff] [review]
jar.mn aligned

This looks excellent, thank you so much for porting this over!
No issues that I could find neither using any of the regular themes or any of the lightweight themes.
Strong ui-r+

A small disclamer that this might cause some regression regarding the HighContrast theme(s). The same issue occurs with Firefox(4), but the only way around that right now would be to only use standard ui components everywhere, meaning no custom drawing at all(ugly). I think we should rather look into some kind of hook to -moz-system-metric(highcontrast). We need this for the Windows theme too. Will look into that.

Comment 17

[Richard Marti (:Paenglab)]

Attachment: Patch for checkin

(In reply to comment #14)
> >-.tabmail-tabs {
> >+.tabmail-tabs:not(:-moz-lwtheme) {
> >   padding-top: 0px;
> >   -moz-padding-start: 0px;
> 
> So, if there is a lightweight theme, how much padding will the tabmail-tabs
> have on the top and start?

The padding is always 0px so I removed it.

> >+  background: -moz-linear-gradient(hsla(0, 0%, 100%, .2),
> >+                           hsla(0, 0%, 45%, .2) 1px, hsla(0, 0%, 32%, .2) 50%);
> 
> The indentation on this looks a little weird.

Corrected, also on tabmail-tabs.

> Also, Should this be background-image, like in the other .tabmail-tab… blocks
> down below?

With background:... is also the background-color from tabbox.css overwritten.

> >+  background-size: 200%;
> 
> Could you not skip this and double the appropriate values in the
> background/background-image element?

This and also the negative background-positions are needed to fill the transparent border-image regions.

Comment 18

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/875cbd82622d