399937 - Free Gnomestripe

Status: VERIFIED FIXED

(Firefox :: General, defect, P2)

Description

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Current plans have us diverging enough from winstripe that the number of ifdefs and other hacks needed are just going to become unwieldy. We should make gnomestripe a proper top-level theme.

Comment 1

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

The drivers *adore* this bug title.

Comment 2

[Michael Monreal [:micmon]]

The sooner we get gnomestripe split out, the sooner we can start work ;)

Comment 3

[Mike Connor [:mconnor]]

Ryan: ETA?

Comment 4

[Michael Ventnor]

Isn't this completed already? There were a number of checkins related to this bug earlier.
This is either a P1 or P2 IMO.

Comment 5

[Michael Ventnor]

Oh, Ryan didn't copy the tab images so we must wait for bug 353785 for the theme to become good enough to use.

Comment 6

[Michael Monreal [:micmon]]

Tab images? Can't we just enable native GTK theming for tabs right away?

Comment 7

[Michael Ventnor]

(In reply to comment #6)
> Tab images? Can't we just enable native GTK theming for tabs right away?
> 

Thats exactly why I mentioned bug 353785...

Comment 8

[Michael Monreal [:micmon]]

Sorry never mind, I understood you incorrectly. From what I'm reading though, the fix from 353785 could easily be added to the gnomestripe theme now?

Comment 9

[Michael Ventnor]

(In reply to comment #8)
> Sorry never mind, I understood you incorrectly. From what I'm reading though,
> the fix from 353785 could easily be added to the gnomestripe theme now?

Ryan says he has a patch for it so we need to wait for him to attach it, then wait again for someone else to review the patch.

Comment 10

[Ryan Flint [:rflint] (ping via IRC for reviews)]

(In reply to comment #4)
> Isn't this completed already? There were a number of checkins related to this
> bug earlier.
Yep, other than the tab de-uglification and flipping build bits, gnomestripe is basically ready to go. I'm going to be AWOL for most of the morning, but I'll be wrapping this stuff up at some point this afternoon/evening.

Comment 11

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Flip the switch

Let's do this thing.

Comment 12

[Michael Ventnor]

+ifneq (,$(filter gtk2,$(MOZ_WIDGET_TOOLKIT)))
+DIRS = gnomestripe

Are you sure this shouldn't be DIRS += gnomestripe like in toolkit?

Comment 13

[Ryan Flint [:rflint] (ping via IRC for reviews)]

(In reply to comment #12)
> +ifneq (,$(filter gtk2,$(MOZ_WIDGET_TOOLKIT)))
> +DIRS = gnomestripe
> 
> Are you sure this shouldn't be DIRS += gnomestripe like in toolkit?
> 

Nope, toolkit's winstripe + gnomestripe and DIRS isn't defined anywhere outside of that block on linux.

Comment 14

[Reed Loden [:reed]]

Plus, toolkit treats gnomestripe as an override theme to winstripe rather than a completely separate theme.

Comment 15

[Michael Ventnor]

OK, but just as a suggestion, could this wait until the native icons in bug 381206 lands? If you do it will make the switch more exciting especially among the Linux-oriented press ;-)

Comment 16

[Ryan Flint [:rflint] (ping via IRC for reviews)]

(In reply to comment #15)
> OK, but just as a suggestion, could this wait until the native icons in bug
> 381206 lands? If you do it will make the switch more exciting especially among
> the Linux-oriented press ;-)
> 

Nope :)
We want perf numbers on tinderbox before landing that, but I'll be sure to get everything in before the nightly ;)

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 288792 [details] [diff] [review]
Flip the switch

free the, uh, foot?

Comment 18

[Ryan Flint [:rflint] (ping via IRC for reviews)]

mozilla/browser/themes/Makefile.in                      1.3
mozilla/browser/themes/winstripe/browser/browser.css    1.133
mozilla/browser/themes/winstripe/browser/searchbar.css  1.22

Comment 19

[Tony Chung [:tchung]]

Hi ryan, do you have a testcase for this bug so QA can verify this fix?

Comment 20

[Reed Loden [:reed]]

(In reply to comment #19)
> Hi ryan, do you have a testcase for this bug so QA can verify this fix?

Install Firefox on Linux. See theme?

Comment 21

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050707 Minefield/3.0pre