65745 - reorganize and reformat theme files

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect, P1)

Description

[Joe Hewitt (gone)]

The current state of the modern and classic themes is messy, especially modern.
 There is a lack of organization in the file structure, lack of consistency in
file names and css formatting, and a whole bunch of extra image files and css
rules.  This needs to change asap, so here is what I plan to do:

1. delete un-used files  - there are several hundred of them)
2. organize files - too many files lopped into the same folder (see
themes/modern/global) make it hard to find files you're looking for.
3. re-name image files - image files are too inconsistently named.  For
instance, if an image represents an active state, it currently may include the
words cl, clicked, pressed, etc... We need a standard glossary to use when
naming files.
4. consistent css comment styles - it would be nice it files were commented in a
consistent way so that the comments helped you quickly scan files and find what
you want
5. consistent css formatting - indentation and spacing throughout the css should
be uniform throughout the theme

Comment 1

[timeless]

this is probably beyond the scope of this bug, but...

a chrome://package/images/imageName urlspec could be really neat

basic organization: package supplies a default image 'imageName' and skins 
provide replacement images. either overlay to redirect url path or just by 
having the same name.

many themes use the same css and different pictures, if we used this approach, 
the duplicate css could be omitted.

Comment 2

[Joe Hewitt (gone)]

timeless, point well taken, but I disagree.  In the case of images that seem to
be needlessly duplicated in each theme (like, say the mailnews smiley icons or
pane-layout image), it's better to give theme designers the ability to change
these in their theme if they so desire.  If the package owner wishes to
hard-code the images, then they should just be included in with the content.

Comment 3

[hangas]

Themes Triage Team nsbeta1+

Comment 4

[Blake Ross]

6. get rid of hundreds of redundant or unnecessary style rules?

Comment 5

[Joe Hewitt (gone)]

I've spent the last week cleaning up and re-organizing the css and image files
in the Modern theme.  It was often tempting to make XUL changes to help clean up
the theme CSS even more, but I tried to keep these changes to a minimum.  Lots
of XUL-related cleanup will happen in future bugs.  I did, however, have to make
a small number of XUL changes in xpfe/global, mail, editor, and xpinstall.  I'll
post these changes as a patch and request review from each of those component
owners.

As far as the patch to the modern theme goes.... MUAHAHAHA! There will be no
patch, the changes are too widespread.  Instead, I will simply give you the
newly packaged modern.jar.  I'd like as many people as possible to open up the
jar and give me some feedback on the new organization.  Most files have been
renamed, moved, and re-organized internally.  In addition to organization, I've
gotten rid of tons of unnecessary rules and simplified some overly complex rules.

I've done quite a bit of testing to make sure I haven't caused any regressions,
and so far everything looks good.  I'd appreciate as much testing in this area
as possible by reviewers.

Comment 6

[Joe Hewitt (gone)]

Attachment: xul patch (for global)

Comment 7

[Joe Hewitt (gone)]

Attachment: xul patch (for mail)

Comment 8

[Joe Hewitt (gone)]

Attachment: xul patch (for editor)

Comment 9

[Joe Hewitt (gone)]

Attachment: xul patch (for xpinstall)

Comment 10

[Joe Hewitt (gone)]

Attachment: modern.jar

Comment 11

[Daniel Veditz [:dveditz]]

r=dveditz for xpinstall xul

Comment 12

[Charles Manske]

For editor: Why the big block of lines deleted, then inserted in the middle?
There doesn't seem to be any change between those lines. It would be best if
you didn't touch those lines if theres no change.
Otherwise, r=cmanske.
FYI, I use this for diffs:
cvs diff -u -w

Comment 13

[Joe Hewitt (gone)]

Actually, there was a reason for the deleted lines.  Previously there was a
menubar and toolbar free floating, but I put them inside a toolbox.  CVS
interpreted this as a total deletion because I also re-indented the
menubar/toolbar to suit their new nested position.

Comment 14

[Daniel Veditz [:dveditz]]

Be cautious using the -w diff option, it often leads to patches that won't 
apply correctly and certainly results in a file that no one else could check-in 
due to whitespace munging.

Comment 15

[Blake Ross]

This stuff looks great.

Some comments:

- Some people prefer using <vbox .../> over <box orient="vertical"/>
- Horizontal is the default box orientation, so no need to explicitly specify it
- It seems like a large skip to go from /global to specific widget directories 
(e.g. /global/tabcontrol).  I was thinking perhaps a /widget layer would be 
better organized, but I suppose it's fine as is if there's nothing else 
floating around in /global.
- I'm not sure about the clarity of the icon# naming convention.  Any chance 
for a more descriptive name?
- You didn't give the same lovin' to Classic :-)

Otherwise, things look good.  Thanks for doing this, r=blake

Comment 16

[Joe Hewitt (gone)]

> It seems like a large skip to go from /global to specific widget directories

Since global itself is 99% widgets, having just a /widget subdirectory would be
sort of useless.  I've always found it to be painful trying to find image files
in global, and splitting them up by widget seems to be the best way to make it
easier.

> I'm not sure about the clarity of the icon# naming convention.

I agree, but I've yet to think of a name that sounds just right.  Any ideas?

> You didn't give the same lovin' to Classic :-)

I had intended to, but Ben told me not to.

Comment 17

[chuang]

r=chuang for mail.  Sorry for the late review, I just get back from vacation.

Comment 18

[andreww]

r=andreww for modern.jar.

I didnt find any style rules for hbox or vbox in box.css. Do they need to be 
included anywhere?  

Comment 19

[Joe Hewitt (gone)]

All fixed for the Modern theme. Whether I ever do Classic remains to be seen.

Comment 20

[Patty Mac]

Marking verified (2001-03-19-12-Mtrunk).

Comment 21

[R.K.Aa.]

after checkins on the 15th these files in mozilla/themes/modern/editor/icons
all have a size of 0 byte:

align-act.gif
align-dis.gif
align-hov.gif
align.gif

When updating from CVS i don't see any signs that these files are reduntant.
Have i done something wrong?

Comment 22

[R.K.Aa.]

also: a file that was updated on the 15th is not readable or are reported
corrupted when i try to open it with both the Gimp, ImageMagick and XView:

mozilla/themes/modern/communicator/bookmarks/location.gif