Closed Bug 615693 Opened 9 years ago Closed 9 years ago

Improve the inactive window appearance of tabs-on-top

Categories

(Firefox :: Theme, defect)

All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Keywords: polish)

Attachments

(3 files, 3 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
They're too dark and the gradient doesn't smoothly connect to the title bar.
Attachment #494144 - Flags: review?(dao)
Attached image after
Attachment #494146 - Flags: ui-review?(shorlander)
Comment on attachment 494146 [details]
after

Looks seamless :)
Attachment #494146 - Flags: ui-review?(shorlander) → ui-review+
After
Attached patch unbitrotted (obsolete) — Splinter Review
Attachment #494144 - Attachment is obsolete: true
Attachment #510977 - Flags: review?(dao)
Attachment #494144 - Flags: review?(dao)
Attached patch unbitrotted again (obsolete) — Splinter Review
Dão, can I do anything to help you review this sooner?
Attachment #510977 - Attachment is obsolete: true
Attachment #513940 - Flags: review?(dao)
Attachment #510977 - Flags: review?(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.
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?
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.
OK, so this comes down to a pure image drop now. Not sure if this even needs review.
Attachment #513940 - Attachment is obsolete: true
Attachment #516220 - Flags: review?(dao)
Attachment #516220 - Flags: approval2.0?
Attachment #513940 - Flags: review?(dao)
Attachment #516220 - Flags: review?(dao) → review+
Comment on attachment 516220 [details] [diff] [review]
only tab bar background

a=beltzner
Attachment #516220 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/4f543b1fec65
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0

Verified issue.
Status: RESOLVED → VERIFIED
OS: Mac OS X → All
OS: All → Mac OS X
You need to log in before you can comment on or make changes to this bug.