68229 - smiley feature should be skinnable

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[andreww]

The images for the smileys element in composer, mail, aim, etc. should be
derived from the theme (modern, classic, etc.) and not hard-coded into the
content. So the images should be removed from content and copied to each theme -
modern, classic windows, and classic mac.

Comment 1

[andreww]

adding cc's for FYI.

Comment 2

[andreww]

changing OS, platform..

Comment 3

[Joseph Elwell]

This prevents our app from having a fully skinnable UI. Users should be able to
come up with their own smiley face images and use them instead of the default
ones.

Comment 4

[Syd Logan]

Anatoliya, please get with andreww on this, or close it (after talking with him) 
if the issue is already resolved.

Comment 5

[Anatoliy Averbukh]

I consider the images as just internal images, those shouldn't be skinnable.
Reason for that: the images have to represent the SAME emotional state
for various OSs, platforms, applications and even for different clients.
They currently represent the states with generally accepted way, the way people 
understand. 
If we put them to skin, we can confuse people about emotional state of another 
side. For instance, how can we consider smiley image with code :-) of RED color?
Obviously, it is NOT the same emotional state like one with yellow color.
Because of that, I consider the images as internal images (like anchor image, 
for instance), those shouldn't be a part of skinnable UI at all.
Even inside of Netscape, there are at least 3 different applications that use 
the images.
So, the images areNOT a part of the applications, they are part of editor. (As 
a side effect of that, such solution eliminates dependencies between editor and 
other packages).
I have discussed that question with editor guys, and we decided NOT to put the 
images to editor skin. Their status is same like anchor image, for instance, or 
other internal images.

Comment 6

[andreww]

adding german to cc:

Comment 7

[german]

Just to make it perfectly clear: 
all images that appear in the chrome must be skinnable. This rule does not
include the image on the toolbar, but also all images that are used in the items
of the menu.

Comment 8

[german]

I meant to say:
this does not *only* include the images on the toolbar, but also the images
appearing in each item of a menu.

Comment 9

[Syd Logan]

*** Bug 70962 has been marked as a duplicate of this bug. ***

Comment 10

[Anatoliy Averbukh]

Attachment: mozilla part for the bug

Comment 11

[Anatoliy Averbukh]

I added:
- 3 new gifs for new smiley images ("short" ones),
- toolbar image and images for each menu item are skinnable now,
- and all smiley images are moved from messenger to editor.
Please review.

Comment 12

[timeless]

Index: mozilla/editor/ui/composer/content/makefile.win
and Makefile.in should be removed from cvs [and the references to them], jar.mn 
now handles chrome packaging.

RCS file: /cvsroot/mozilla/themes/modern/editor/images/MANIFEST,v
\ No newline at end of file
^ please fix

I don't remember if MANIFESTs are still needed. please contact your local mac 
build expert.

Comment 13

[Anatoliy Averbukh]

Attachment: fixed new line

Comment 14

[andreww]

r = andreww 

conditional on fixing the "no newline " issue that timeless mentioned. I am
pretty sure that only the jar.mn files are needed, but those other files might
be getting parsed in some other way which might confuse the build system.

Comment 15

[Simon Fraser [no longer active]]

MANIFESTS which are used just to install jar resources are obsolete. jar.mn does 
that job now.

Comment 16

[Anatoliy Averbukh]

Simon,
It is not a problem to delete the updates to MANIFEST files,
but it is dangerous, until the files in the tree and even more: there are 
references to them (e.g. in MozillaBuildList.pm). I think, we need to continue 
update for those files until they would be deleted ( to avoid confusion).
In any case, it will not affect the bug I have fixed, please SR my change.
Thank you.

Comment 17

[Scott MacGregor]

the manifest files aren't part of the mac build at all so there's no reason to
keep them updated.....

Comment 18

[Anatoliy Averbukh]

Attachment: Deleted all MANIFEST updates, pls SR

Comment 19

[Simon Fraser [no longer active]]

Why is this just commented out and not removed?

-<?xml-stylesheet href="chrome://editor/content/EditorExtra.css" type="text/
css"?>
+<!-- ?xml-stylesheet href="chrome://editor/content/EditorExtra.css" type="text/
css"? -->

Comment 20

[Anatoliy Averbukh]

Attachment: have plan to reuse the file in the close future, but for now remove it

Comment 21

[Simon Fraser [no longer active]]

Fix your bad tabs:

     skin/classic/editor/images/underline.gif                             
(editor/images/underline.gif)
+       skin/modern/editor/images/smile.gif                                                                      
(editor/images/smile.gif)
+    skin/modern/editor/images/s_smile.gif                                                               
(editor/images/smile.gif)
+    skin/modern/editor/images/frown.gif                                                                         
(editor/images/smile.gif)

and sr=sfraser

Comment 22

[timeless]

r=timeless given the fix requested by smfr.

Comment 23

[Anatoliy Averbukh]

bug is fixed

Comment 24

[Ben Bucksch (:BenB)]

*** Bug 39045 has been marked as a duplicate of this bug. ***

Comment 25

[Ben Bucksch (:BenB)]

Moving the images to a non-messenger place is a good idea, though an even more
central one than editor/ would have been preferred (since netwerk is pretty
generic and doesn't care about the editor or even Gecko at all).

Checkin caused bug 71954.

Comment 26

[[e-mail] (gone - use [e-mail] instead)]

Most of the files look fine in terms of spacing/tabs, but I've cleaned 
up /mozilla/editor/jar.mn to get rid of a couple tabs, and align some other 
lines.  I'll attach the patch, but Joe said there is a cleanup coming one of 
these days anyhow.

Comment 27

[[e-mail] (gone - use [e-mail] instead)]

Attachment: Clean up tabs.

Comment 28

[[e-mail] (gone - use [e-mail] instead)]

Nevermind that last patch.  Practically the whole file is tabbed, I'm wondering 
how many others are.  Joe, as long as we have a bug to cover the tabs in these 
files, I can mark this verified.

Comment 29

[[e-mail] (gone - use [e-mail] instead)]

Code is checked into the tree.  Issues with spacing should be filed seperately.