1004025 - Move themes/*/devtools/font-inspector.css into themes/shared/devtools/font-inspector.css

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Brian Grinstead [:bgrins]]

As part of the process of DRYing up our CSS, we should move the font-inspector CSS file into the shared folder.

The main steps for merging these files into one are:

1) Make sure there aren't any differences between the windows/linux/osx versions of the files.  You can use a diffing tool for this, or just look at them:

  diff browser/themes/linux/devtools/font-inspector.css browser/themes/osx/devtools/font-inspector.css
  diff browser/themes/linux/devtools/font-inspector.css browser/themes/windows/devtools/font-inspector.css

2) If there are no changes, then all we need to do is make a new file called browser/themes/shared/devtools/font-inspector.css, and copy the contents of one of the other files into it.  If there are changes, we will need to navigate them individually and decide if they should be kept or not.  We can keep them with preprocessor directives if needed.

3) Delete browser/themes/linux/devtools/font-inspector.css, browser/themes/windows/devtools/font-inspector.css, browser/themes/osx/devtools/font-inspector.css

4) Modify browser/themes/*/jar.mn to replace (devtools/font-inspector.css) with (../shared/devtools/font-inspector.css).  Note that the windows/jar.mn file has two places that need to change.

Comment 1

[Brian Grinstead [:bgrins]]

Johannes, let me know if you need any more info to get started on this.  Basically we want these three files to go away:

browser/themes/linux/devtools/font-inspector.css
browser/themes/osx/devtools/font-inspector.css
browser/themes/windows/devtools/font-inspector.css

And to be replaced with a single new one:

browser/themes/shared/devtools/font-inspector.css

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #0)
> As part of the process of DRYing up our CSS, we should move the
> font-inspector CSS file into the shared folder.
> 
> The main steps for merging these files into one are:
> 
> 1) Make sure there aren't any differences between the windows/linux/osx
> versions of the files.  You can use a diffing tool for this, or just look at
> them:
> 
>   diff browser/themes/linux/devtools/font-inspector.css
> browser/themes/osx/devtools/font-inspector.css
>   diff browser/themes/linux/devtools/font-inspector.css
> browser/themes/windows/devtools/font-inspector.css
> 
> 2) If there are no changes, then all we need to do is make a new file called
> browser/themes/shared/devtools/font-inspector.css, and copy the contents of
> one of the other files into it.  If there are changes, we will need to
> navigate them individually and decide if they should be kept or not.  We can
> keep them with preprocessor directives if needed.
> 
> 3) Delete browser/themes/linux/devtools/font-inspector.css,
> browser/themes/windows/devtools/font-inspector.css,
> browser/themes/osx/devtools/font-inspector.css
> 
> 4) Modify browser/themes/*/jar.mn to replace (devtools/font-inspector.css)
> with (../shared/devtools/font-inspector.css).  Note that the windows/jar.mn
> file has two places that need to change.

I should add instead of step 2, we should actually use this command instead of creating a new brand new file:

hg rename browser/themes/linux/devtools/font-inspector.css browser/themes/shared/devtools/font-inspector.css

This should preserve history of the new shared file.

Comment 3

[Johannes Mittendorfer]

Attachment: movethemes

This should work.

Comment 4

[Johannes Mittendorfer]

OK. I just have seen your latest comment. One moment please.

Comment 5

[Johannes Mittendorfer]

Attachment: movethemesV2

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8415511 [details] [diff] [review]
movethemesV2

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

Looks good to me.  Thanks!

Comment 7

[Johannes Mittendorfer]

Fine. I'm going to flag it for check in.

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Johannes Mittendorfer from comment #7)
> Fine. I'm going to flag it for check in.

Sounds good.  If you don't mind continuing to help with this, my goal is to eventually empty out https://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/devtools/ so we only have one copy of each file.  The next step is to do all of the images in Bug 1004111.

Comment 9

[Johannes Mittendorfer]

Maybe tomorrow. It's a bit late in my time zone. I will ask for the assignment when/if I have time.

Thanks so far!

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to Johannes Mittendorfer from comment #9)
> Maybe tomorrow. It's a bit late in my time zone. I will ask for the
> assignment when/if I have time.
> 
> Thanks so far!

OK, thanks for the help

Comment 11

[Tim Nguyen :ntim]

Apparently a try push is needed with checkin-needed (See : https://bugzilla.mozilla.org/show_bug.cgi?id=989751#21 )

Comment 12

[Brian Grinstead [:bgrins]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=ee605423d710

Comment 13

[Johannes Mittendorfer]

Could you may give me a voucher for commit level 1, so I can run these tests myself the next time? 

https://bugzilla.mozilla.org/show_bug.cgi?id=1004398#c1

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/44f24051261e

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/44f24051261e