615693 - Improve the inactive window appearance of tabs-on-top

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Markus Stange [:mstange]]

Attachment: v1

They're too dark and the gradient doesn't smoothly connect to the title bar.

Comment 1

[Markus Stange [:mstange]]

Attachment: before

Comment 2

[Markus Stange [:mstange]]

Attachment: after

Comment 3

[Stephen Horlander [:shorlander]]

Comment on attachment 494146 [details]
after

Looks seamless :)

Comment 4

[Jorge R]

After

Comment 5

[Markus Stange [:mstange]]

Attachment: unbitrotted

Comment 6

[Markus Stange [:mstange]]

Attachment: unbitrotted again

Dão, can I do anything to help you review this sooner?

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 513940 [details] [diff] [review]
unbitrotted again

> /* preloading hack */
> #TabsToolbar::after {
>   content: '';
>   display: block;
>   background-image:
>     url(chrome://browser/skin/tabbrowser/tab-top-normal-active.png),
>+    url(chrome://browser/skin/tabbrowser/tab-top-normal-inactive.png),
>     url(chrome://browser/skin/tabbrowser/tab-top-hover-active.png),
>+    url(chrome://browser/skin/tabbrowser/tab-top-hover-inactive.png),
>     url(chrome://browser/skin/tabbrowser/tab-top-selected-active.png),
>+    url(chrome://browser/skin/tabbrowser/tab-top-selected-inactive.png),

I don't like the extra images creeping in here. It's impossible to notice perf regressions in such small chunks.

Comment 8

[Markus Stange [:mstange]]

I'm going to get some perf numbers.
http://tbpl.mozilla.org/?tree=MozillaTry&fromchange=578ea7b06ea4&tochange=daf767ba2b3a

Comment 9

[Markus Stange [:mstange]]

OK, http://perf.snarkfest.net/compare-talos/index.html?oldRevs=f5092fb51609&newRev=daf767ba2b3a&tests=ts,twinopen&submit=true tells us that adding 500 of these images regresses warm startup time by about 16% and doesn't affect twinopen at all. So we can say that the 3 images in this patch will regress startup time by less than 0.1%.

I tried to get cold startup numbers on my Macbook but the numbers were way too noisy to draw any conclusions.

How about we add these 3 images now, and then I'll make another patch that uses color overlays for the hover state and removes all 3 hover images?

Comment 10

[Dão Gottwald [:dao]]

I can't really tell without seeing the patch with the color overlays. But you could just update tabbar-top-bg-inactive.png for now.

Comment 11

[Markus Stange [:mstange]]

Attachment: only tab bar background

OK, so this comes down to a pure image drop now. Not sure if this even needs review.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 516220 [details] [diff] [review]
only tab bar background

a=beltzner

Comment 13

[:Margaret Leibovic]

http://hg.mozilla.org/mozilla-central/rev/4f543b1fec65

Comment 14

[George Carstoiu]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0

Verified issue.