Closed Bug 678230 Opened 8 years ago Closed 8 years ago

Thunderbird tabs lack background image at the bottom since the landing of Bug #609714

Categories

(Thunderbird :: Toolbars and Tabs, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 8.0

People

(Reporter: bugzilla.i.sekler, Assigned: Paenglab)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

I have no idea whether the checkin <http://hg.mozilla.org/mozilla-central/rev/89c17ca55124> in Bug #609714 broke the rendering of Thunderbird tabs or just revealed a preexisting flaw in Thunderbird GUI. Anyway, with this checkin, the background image in Thunderbird tabs doesn't fill the tab anymore, leaving a stripe of background color at the bottom. This looks especially ugly with dark gtk2 themes like the default Ubuntu theme.

Verified by local backout.
Pinging Richard in case the bug could be on Thunderbird side.
This also happens under Windows.

According to #developers on IRC it should be resolved with landing of bug 677091. So let's wait if after this check-in the problem is resolved
Status: UNCONFIRMED → NEW
Depends on: 677091
Ever confirmed: true
(In reply to Richard Marti [:paenglab] from comment #2)

> According to #developers on IRC it should be resolved with landing of bug
> 677091.

Thank you, but I have tried the patch <https://bugzilla.mozilla.org/attachment.cgi?id=551901> on top of m-c revision ffb2a6be641a and it doesn't fix the Thunderbird issue.

c-c rev. a4076abb21b7
Adding Jim to this bug (because he gave me the info with the other bug). Maybe he can help.

I know how to solve this problem with CSS, but this is more a workaround and the real cause should be found.

Ilja: could it be possible to try a build with m-c changset 89c17ca55124 (http://hg.mozilla.org/mozilla-central/rev/89c17ca55124) backed out? According to the commit summary it could be this the causer. I can't check, I have no build system.
(In reply to Richard Marti [:paenglab] from comment #4)

> Ilja: could it be possible to try a build with m-c changset 89c17ca55124
> (http://hg.mozilla.org/mozilla-central/rev/89c17ca55124) backed out?
> According to the commit summary it could be this the causer.

Yes, a backout of this commit fixes the issue. This is what the bug description actually says ;-) I have retested again with the same positive result on top of m-c and c-c revisions mentioned in comment #3.

So you think this is a core regression, right?
It's hard to know whether this is a Thunderbird bug or a Core bug without knowing what the Thunderbird GUI is doing to produce that effect.
It's also possible the behavior change was deliberate.  The patch changed gradient rendering when a background-size is specified (I had to fix a few reftests to account for the change), and it's conceivable the TB GUI was relying on the old behavior.

But yes, without seeing the DOM/CSS that's causing this it's hard to know.  More details along those lines, please.  :-)
The related CSS resides in <http://mxr.mozilla.org/comm-central/source/mail/themes/gnomestripe/mail/tabmail.css#70> and in <http://mxr.mozilla.org/comm-central/source/mail/themes/gnomestripe/mail/tabmail.css#75>

 .tabmail-tab {
   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%);
-  background-position: -5px -2px;
+  background-position: -5px -0px;
   background-repeat: no-repeat;
   background-size: 200%;
   margin: 0;
   padding: 0;
-  -moz-border-image: url(chrome://messenger/skin/icons/tab.png) 4 5 3 6 /
-                     4px 5px 3px 6px repeat stretch;
+  -moz-border-image: url(chrome://messenger/skin/icons/tab.png) 4 5 0 6 /
+                     4px 5px 0px 6px repeat stretch;
   border-radius: 10px 8px 0 0;
   min-height: 25px; /* reserve space for the sometimes hidden close button */
 }


make tabs in Thunderbird look like they did prior to the patch.
This leaves on top of the tab a 2px transparent area. This works and is similar to Firefox:

 .tabmail-tab {
   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%);
   background-position: -5px -2px;
   background-repeat: no-repeat;
-  background-size: 200%;
+  background-size: -moz-calc(100% + 11px) -moz-calc(100% + 7px);
   margin: 0;
   padding: 0;
   -moz-border-image: url(chrome://messenger/skin/icons/tab.png) 4 5 3 6 /
                      4px 5px 3px 6px repeat stretch;
   border-radius: 10px 8px 0 0;
   min-height: 25px; /* reserve space for the sometimes hidden close button */
 }

Tested under Linux and Windows.
(In reply to Richard Marti [:paenglab] from comment #9)

> This works and is similar to Firefox:
> 
> [...]
> 
> -  background-size: 200%;
> +  background-size: -moz-calc(100% + 11px) -moz-calc(100% + 7px);

Oh, I see, thanks. "background-size: 200%;", identical to "background-size: 200% auto;" now equals to "background-size: 200% 100%;" if there is no intrinsic size for the background image like here. This means, this should be fixed in Thunderbird and you have provided the solution.
Keywords: regression
Attached patch Change the background-size (obsolete) — Splinter Review
I hope this can land before the merge to comm-aurora to fix only on one place.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #552633 - Flags: review?(bwinton)
Well, to be precise, since I gave little detail in comment 7:

Per CSS3, "background-size: X" should be interpreted as "background-size: X auto".

"X auto" behavior depends on two things: the intrinsic dimensions of the image (width/height) and the intrinsic proportion (4x3, 16x9, 8.5x11, etc.).  Gradients have no intrinsic dimensions or proportion, so that 'auto' should be treated as if 100%.  So for gradients "background-size: X" should be identical to "background-size: X auto" should be identical to "background-size: X 100%".

But it wasn't.  In the intrinsic dimensions/proportion language, the gradient scaled as if it had the intrinsic proportions of the background positioning area (the area a repeating background image with "background-size: 100% 100%" would fill).  Intrinsic proportions are *intrinsic* -- there's no feedback loop from the embedding context to inform the image that it should have particular proportions.  An image just has them, regardless where or how it's used.  If the spec required old gradient behavior, gradients would have to be a special form.

Bug 609714, among its changes, makes Gecko properly treat gradients like images with no intrinsic dimensions or proportion.

To preserve rendering here, the easiest fix is to change "background-size: 200%;" to "background-size: 200% 200%;".  That's what I did to two gradient reftests we had that depended on old behavior:

http://hg.mozilla.org/mozilla-central/diff/89c17ca55124/layout/reftests/bugs/538909-1-ref.html
http://hg.mozilla.org/mozilla-central/diff/89c17ca55124/layout/reftests/image-element/gradient-html-07b.html

Maybe there's some good reason to use calc instead.  If so, go for it.  But just duplicating the percent to the other dimension should be computationally less expensive for Gecko, so all else equal that seems preferable to me.
Duplicate of this bug: 678495
Assignee: richard.marti → nobody
Component: Layout → Toolbars and Tabs
OS: Linux → All
Product: Core → Thunderbird
Hardware: x86 → All
Assignee: nobody → richard.marti
QA Contact: layout → toolbars-tabs
I've added also the QFB button background because bug 667244 would not come in comm-aurora.
Attachment #552633 - Attachment is obsolete: true
Attachment #552633 - Flags: review?(bwinton)
Attachment #552669 - Flags: review?(bwinton)
Comment on attachment 552669 [details] [diff] [review]
Change the background-size

Giving the r? to Jim because :bwinton has no time.
Attachment #552669 - Flags: review?(bwinton) → review?(squibblyflabbetydoo)
Comment on attachment 552669 [details] [diff] [review]
Change the background-size

Review of attachment 552669 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!
Attachment #552669 - Flags: review?(squibblyflabbetydoo) → review+
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/8cbc5bf82c4c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 8.0
Was Waldo's suggestion in comment 12 tested at all, as a simpler alternative? In particular, note the last paragraph...
(In reply to Daniel Holbert [:dholbert] from comment #18)
> Was Waldo's suggestion in comment 12 tested at all, as a simpler
> alternative? In particular, note the last paragraph...

Yes, but on Windows it was 130% needed and under Linux 125%, 200% was to high. With -moz-cal both definitions are equal. And a similar definition is used on Firefox for the tabs with no problems as I know.
You need to log in before you can comment on or make changes to this bug.