Use the Firefox Tabs on Linux

RESOLVED FIXED in Thunderbird 3.3a1

Status

Thunderbird
Toolbars and Tabs
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

Trunk
Thunderbird 3.3a1
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 4 obsolete attachments)

(Assignee)

Description

8 years ago
This is a adaption of the Firefox Tabs for Linux. With this patch the Tabs in Thunderbird looks similar to Firefox.
(Assignee)

Comment 1

8 years ago
Created attachment 476498 [details] [diff] [review]
Proposed patch

This is mostly a copy of the Firefox CSS.
Assignee: nobody → richard.marti
Attachment #476498 - Flags: ui-review?(clarkbw)
Attachment #476498 - Flags: review?(bwinton)
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
Attachment #476498 - Flags: ui-review?(clarkbw) → ui-review?(nisses.mail)
Created attachment 478870 [details]
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?
Also I had to move the tab.png icon from qute into gnomestripe directory for the patch to compile correctly.
(Assignee)

Comment 5

8 years ago
Created attachment 479075 [details] [diff] [review]
Fix the tab.png location
Attachment #476498 - Attachment is obsolete: true
Attachment #479075 - Flags: ui-review?(nisses.mail)
Attachment #479075 - Flags: review?(bwinton)
Attachment #476498 - Flags: ui-review?(nisses.mail)
Attachment #476498 - Flags: review?(bwinton)
(Assignee)

Comment 6

8 years ago
Created attachment 479145 [details]
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?
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.
Created attachment 480896 [details]
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 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.
Attachment #479075 - Flags: ui-review?(nisses.mail) → ui-review-
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.)
Attachment #479075 - Flags: review?(bwinton)
(Assignee)

Comment 11

8 years ago
Created attachment 481033 [details] [diff] [review]
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.
Attachment #479075 - Attachment is obsolete: true
Attachment #481033 - Flags: ui-review?(nisses.mail)
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.
(Assignee)

Comment 13

8 years ago
Created attachment 481039 [details] [diff] [review]
jar.mn aligned

Now I understand what you meant.
Attachment #481033 - Attachment is obsolete: true
Attachment #481039 - Flags: ui-review?(nisses.mail)
Attachment #481033 - Flags: ui-review?(nisses.mail)
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.
Attachment #481039 - Flags: review+
(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 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.
Attachment #481039 - Flags: ui-review?(nisses.mail) → ui-review+
(Assignee)

Comment 17

8 years ago
Created attachment 481315 [details] [diff] [review]
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.
Attachment #481039 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/875cbd82622d
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a1
You need to log in before you can comment on or make changes to this bug.