Closed Bug 57576 Opened 24 years ago Closed 23 years ago

Apps in the suite should be able to use different/distinct icons

Categories

(SeaMonkey :: UI Design, defect, P2)

Product:
SeaMonkey
Component:
UI Design
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.2

People

(Reporter: bugzilla, Assigned: vishy)

References

(
URL
)

Details

(Whiteboard: fix in hand, have r/sr/a.)

Attachments

(7 files)

Enables specifying icon via attribute on <window> tag.
5.54 KB, patch
Details | Diff | Splinter Review
Adds support for list-style-image to nsComputedDOMStyle.cpp
1.75 KB, patch
Details | Diff | Splinter Review
New patch that gets icon "id" from css property.
8.44 KB, patch
Details | Diff | Splinter Review
Additional patch for nsXULWindow.h (missing from previous attachment)
690 bytes, patch
Details | Diff | Splinter Review
Complete patch, with suggested improvements
10.17 KB, patch
Details | Diff | Splinter Review
Updated patch; same exact code
10.17 KB, patch
Details | Diff | Splinter Review
Fix for LoadImageW not being implemented on Win9x
2.29 KB, patch
Details | Diff | Splinter Review 
Each main app in the suite needs to use a different icon.  If I have 10 windows 
open, a various mixture of Composer, Navigator and MailNews windows, and all 
that's shown for each is "Bug 4..." and a lizard icon, it's basically 
impossible to tell whether I'm viewing a bug in navigator, looking at bugmail, 
or editing a Bugzilla bug site.
This applies not just to window icons for an application, but also to file icons 
in the installation (e.g. the `Mozilla Navigator, `Mozilla Messenger', `Mozilla 
Composer', etc icons in the Mac installation), and to icons for specific windows 
within a component (e.g. different icons for the Messenger three-pane window, for 
a message composition window, and for the Address Book window).

There is a bug `Use a different icon for Mail', which should be dependent on this 
bug (but Bugzilla isn't letting me do searches now so I can't find it).

The XPToolkit chicken is coming home to roost ...
OS: Windows 98 → All
Hardware: PC → All
Keywords: 4xp
this has been a big complaint of mine, adding myself to the CC.
Seems like this is kind of an xptoolkit thing, but I have some ideas about how
to make it happen
pav sez he has this working in gfx2, and kerz says he's working on it.
it frightens me that this is in gfx2, but maybe I don't know enough about gfx
vs. widget :)
I'll take this from don..
Assignee: don → alecf
Target Milestone: --- → mozilla1.0
Status: NEW → ASSIGNED
this is pretty easy....

i have the apis needed in gfx2 to do this... i brought this up a while ago, but
people said don't worry about it...

i can help if you need
*** Bug 63512 has been marked as a duplicate of this bug. ***
I agree with MPT that it would be desirable to have the ability to set a 
different icon for each type of window, rather than just one per 'app'.   It 
would be good if the implementation supported this, as it is a more general 
solution.
nav triage team:

Nice to have, but don't think we have time in beta1 timeframe. Nsbeta1-
nav triage team:

Would be nice, but not a beta1 stopper, especially if we need gfx2, not that 
that's a bad thing, but don't think we can get it in for this release. Marking 
nsbeta1-
Keywords: nsbeta1-
*** Bug 47779 has been marked as a duplicate of this bug. ***
*** Bug 65292 has been marked as a duplicate of this bug. ***
Adding to tracking bug 29303.
Blocks: icons
Several people are working on making new icons (bug 43647, bug 27746).  I take 
it that this bug is for the backend.
as per the Netscape PRD, this is a Netscape beta stopper. changing to nsbeta1.
Alec - Bill Law had this feature so I am reassigning this bug to him. thanks,
Vishy.
Assignee: alecf → law
Status: ASSIGNED → NEW
Priority: P3 → --
Target Milestone: mozilla1.0 → ---
this doesn't seem to be a netscape issue since they will create their own icons.
Mozilla needs its own icons too.

This should be done by mozillla1.0 at the absolute latest. Therefore i suggest
to set a milestone for this bug.
Peter: the way you nominate a bug to be fixed by a target milestone is to add
that milestone name to the Keywords field; a comment alone won't help anyone
query such nominations and measure them against target milestone settings via
bugzilla.  I've set the keyword for you.

/be
Keywords: mozilla1.0
I've attached a patch that changes nsIWidget to add a SetIcon method and to
nsXULWindow that uses that method to set the icon from the icon= attribute on
the XUL <window>.

Also included is the change to implement this method in the Windows version of
nsWindow.cpp.  I would need help for the Mac and Linux equivalent.  Note that
I've provided an empty definition in nsBaseWidget.

The other thing that's necessary, of course, is to come up with the additional
icons and to change the various .xul files.  The icons go in
mozilla/xpfe/bootstrap in splash.rc (on Windows, at least).
+    docShellElement->GetAttribute(NS_ConvertASCIItoUCS2("icon"), windowIcon);

please use the new string foo. NS_LITERAL_STRING("icon") i think. otherwise it 
looks good.
ooh ooh, I was looking at doing something similar to this on Mac. Please take a 
look at bug 66814 and bug 36305. I'll take a gander at your patch too.

timeless: did you cc: me on this? coolness

Ok, I looked at how this is implemented, overall the backend changes look fine, 
but I have some comments:

1/ This has the side effect that any Widget can have an icon set. I think this is 
a good side effect, but worth mentioning

2/ I assume you'd write something like

	<window icon="navigator.ico">

which is principle is fine... but a couple of things

* Its not skinnable. Perhaps this is good. Modern and Classic should probably 
keep the same icons in this instance, as these are a part of Moz's external 
representation in the rest of the operating system. (the application's icon and 
the splash screen aren't skinnable after all, so why would the window icon be...)

* More seriously, I can't see how to take this design cross platform. I can see 
how using the splash.rc gives you a layer of indirection... but funamentally you 
can't change the value of icon= on a platform by platform basis (unless I'm 
missing something). If we do what to fix this then we want to say that the syntax 
is icon="chrome://something/whatever.gif".

Correct me if I'm wrong but, each .xul file exists only once in CVS - so if the 
navigator components it contains <window icon="navigator.ico"> then there's no 
way for the Linux one to say <window icon="navigator.xpm" or the Mac one to say <
window icon="navigator.rsrc"> Even if I wrote code to support loading and setting 
the window icon out of a Mac resource file, I couldn't specify the file to use.

Am I right here or did I miss something?

Otherwise I like the API and it'll certainly come in handy for one of those other 
bugs I mentioned. 

>please use the new string foo. NS_LITERAL_STRING("icon") i think. otherwise it
>looks good.

Yes, thank you; I'll make that change.

>1/ This has the side effect that any Widget can have an icon set. I think this is
>a good side effect, but worth mentioning

Acknowledged.  There does not seem to be an interface (at the widget layer at
least) for the widget corresponding to top-level windows.  There are a number of
methods in nsIWidget that only apply to that subset of widgets (e.g., SetTitle).

>* Its not skinnable.

Not as skinnable as we'd like, correct.  It might be possible to switch to using
::LoadImage vs. ::LoadIcon, in which case somebody might be able to drop in a
new set of .ico files.  I don't know the story on this for Unix/Mac.

But the plan is to enhance this at some point so that this <window> attribute
can be specified via CSS.  It is relatively straightforward to change the
nsXULWindow code so that it gets the icon name from the style system.  That
might also enable some per-platform specification of the icon.

>* More seriously, I can't see how to take this design cross platform.

It is conceivably as "cross platform" as say, URIs.  It lets you specify a
unique string (the string isn't necessarily the name of a file) per window/icon.
 Each platform must then define some convention for how to map that string to
the unique platform-specific icon image required for the platform.  I've come up
with a simplistic mapping for Windows.  I eyeballed the gtk code and think it
can be done there.  Mac people tell me it can be done for the Mac.

If by "chrome://something/whatever.gif" you really mean "whatever.gif" then the
problem with that is that no single file works for all platforms (without
considerable effort).  But perhaps some chrome: url (and chrome registry) magic
*could* map some *generic* "icon identifier" to some platform- and skin-specific
file.

But that still works if the nsIWidget method takes a string and we can embellish
this further when we know more about how to specify these icons in a more
platform- and skin-friendly manner.

Thanks for the feedback.

BTW, we don't actually have any icons, so let's not get ahead of ourselves :-).




I think this is a great first step, but I don't think we should check it in as
is - or at least the XUL window side of things (we can probably check in the
widget side of things today) I think the next step is to go towards CSS and at
least allow you to say

window[type=mail] {
    -moz-icon: @url(chrome://messenger/skin/ms-windows-icon-foo.ico)
}

as a next step. Hyatt is the best person to ask about how to do this.
I think that gfx2 is going to be the last step we need to be able to use .gif
files instead of native-format files.

cc'ing hyatt and hixie for their comments.
Changing keyword to "nsbeta1+."  It was set to "nsbeta1-" accidentally.
Keywords: nsbeta1- → nsbeta1+
I did an icon set which includes icons for Navigator, Mail & News and Composer.
Here's the link: http://www.crosswinds.net/~ggc/
gcavallanti@gmx.it, they are really cool!!! You can make also a complete theme
based on this design! I would use it by default...
Giovanni's icons are VERY good. They should appear in the NEXT nightlies!!! ;-)
Blocks: 43647
Take a look at this:
 nsCOMPtr<nsIScriptGlobalObject> global;
1838             mDocument->GetScriptGlobalObject(getter_AddRefs(global));
1839             nsCOMPtr<nsIDOMViewCSS> viewCSS(do_QueryInterface(global));
1840             if (viewCSS) {
1841  			   nsCOMPtr<nsIDOMCSSStyleDeclaration> cssDecl;
1842               nsAutoString empty;
1843               nsCOMPtr<nsIDOMElement> 
elt(do_QueryInterface(NS_STATIC_CAST(nsIHTMLContent *, this)));
1844               viewCSS->GetComputedStyle(elt, empty, 
getter_AddRefs(cssDecl));
1845               if (cssDecl) {
1846                 nsAutoString behavior; behavior.Assign(NS_LITERAL_STRING("-
moz-binding"));
1847                 nsAutoString value;
1848                 cssDecl->GetPropertyValue(behavior, value);
1849                 if (!value.IsEmpty()) {
1850                   // We have a binding that must be installed.

From:
http://lxr.mozilla.org/mozilla/source/layout/base/src/nsGenericElement.cpp#1844

Looks like it would be pretty easy to adapt this to read the "list-style-image" 
CSS style from a given window... assuming we can get an object which implements 
nsIDOMViewCSS on the window.

That way each window can simply by styled
	list-style-image: url(chrome:/foo/bar/baz/icon.png)
which is skinnable and inherantly "XP-isable".

From there its a matter of using an ImageGroup/ImageObserver to load the nsIImage 
in the normal fashion. 

I just got through implementing a function to convert an nsIImage into a platform 
native icon on MacOS (bug 66814), but I'm on the road for a week so if someone 
feels able to sort out the CSS part I'll get to wiring the Mac specific part of 
the implementation when I get back.

This is much closer to what Alec Flett was talking about, and has the  advantage 
of being very flexible, if we can join up the last few dots...

ugh, the imagerequest,etc are all going away...
I'm a little reluctant to sign up to provide icon-twiddling code for each
platform.  A less risky strategy might be to use the list-image-style attribute
for <window>s to specify a platform-independent psuedo-chrome URL of the form:

   resource:///chrome/icons/<package>/<window>

E.g.,
   resource:///chrome/icons/navigator/navigator
   resource:///chrome/icons/mail/compose

Each platform would then embellish those URLs to come up with appropriate native
resource identifiers.  For instance, on Windows, we might simply add ".ico".
Then we would use platform-specific APIs to deploy those icon resources (e.g.,
::LoadImage on the .ico file for Windows).

This wouldn't prevent deploying a more elaborare scheme on a given platform.

Comments?
This still has a couple of problems:

> resource:///chrome/icons/<package>/<window>

Its not truly skinable - as its a "pseudo" chrome URL as you say... if each skin 
was to specify a different URL that'd mean some magic translation under /icons/ 
was needed and probably make it harder for 3rd parties to package themes, not to 
mention introducing yet another implicit convention for storing certain files.

Also troublesome still is that only the Classic skin is neatly divided into 
Windows/Unix/Mac directories. Other skins only have a single layer of .css files, 
so I'm not sure its possible to include the embelishments you talk about, unless 
you're saying we should hardcode this into the C++ ... which could get really 
ugly by the time every platform is catered for.

I think the real issue here is whether these sorts of icons should be skinnable 
or not. If they should be, then they should be in an XP format (PNG) and be 
accessed through a normal chrome URL. If they're meant to be the same in every 
skin, then perhaps having a special url scheme for icons and hardcoding the logic 
for things like adding .ico to the end of the name is acceptable.

Who's call is this ultimately?

> if each skin was to specify a different URL that'd mean some magic translation
> under /icons/ was needed

No "magic," really.  The Foobar skin's .css would specify
resource:///chrome/icons/foobar/navigator (for the navigator window's icon).
The only trick is getting the .ico files into chrome/icons/foobar when the skin
is "installed."

The "embellishment" would definitely be in the C++.  It sort of has to be,
because it is the same platform-specific C++ code that provides the
implementation of the nsIWidget::SetIcon function.  If some of those C++
implementation can, and want to, share implementation, then that's OK.

I hear what you say about the limitations of the "skinnability" of the icons,
but sometimes we have to make compromises.  This scheme enables us to have
separate icons for each window and for Mozilla distributions (such as
Netscape's), and users, to replace those icons.

Ultimately, the call is made by the implementor.  If I can get the widget owner
(and style, since it seems that code has to be tweaked also) to accept the
implemenation I provide, then it's a done deal.  We'd probably be real happy if
somebody else provided a better implementation.
this seems like alot of work to make a non-skinnable solution.... if you're
going to do this much work, we might as well do it the right way.
Huh?  Everything I've done (or have talked about doing) has to be done regardless.

The only issue is whether we do lots *more* work to suck .png's out of jar files
(or whatever) and convert those to platform-specific icon resources.
as long as we use css to specify the url i think packages would naturally 
specify
chrome://<packagename>/<something>/<imagename>
personally, i'd prefer for <something> to be 'images' and that we create an 
images binding with the following attribute:
searches chrome://package/skin/image/ for <imagename>, on failure, search 
chrome://package/content/image/ for <imagename>.
i'm flexible about whether image/ is hard coded or specified in contents.rdf 
(probably better as specifyable).

png's make sense however we should be able to use native icons if available

perhaps image:chrome://navigator/images/browser!icon
where image: tries to find the most suitable icon in browser/ (based on os 
preferences)
<browser/icons.list>
image/vnd.microsoft.windows.icon browser.ico
image/vnd.apple.macos.icon browser.rscr
image/xbm browser.xbm
image/png browser.png
EOF
actually, putting all of the icons provided by a package into a single rdf 
file, might make sense

navigator
*appicon
 *{mimetypes}
  *{filepath}
*history
 *{mimetypes}
  *{filepath}

in this fassion, a css file would indicate the icon name which is a top level 
in my picture, and mozilla would select the appropriate icon/image file based 
on the available mimetypes and its preferences.
Somehow, this doesn't have target milestone 0.9 (well, 0.8.1, but I haven't done
the mass move yet). Setting target milestone to mozilla0.9
Target Milestone: --- → mozilla0.9
Mass moving most of mozilla0.9 bugs to mozilla0.8.1
Target Milestone: mozilla0.9 → mozilla0.8.1
Some rough code I'm working on for icons in Mac menus, given an nsIDOMNode, which 
I know also implements nsIContent and nsIDOMElement, I find the value of list-
style-image. 


  nsCOMPtr<nsIContent> content (do_QueryInterface(mDOMNode));
  nsCOMPtr<nsIDocument> document;
  content->GetDocument(getter_AddRefs(document));
  nsCOMPtr<nsIScriptGlobalObject> global;
  document->GetScriptGlobalObject(getter_AddRefs(global));
  nsCOMPtr<nsIDOMViewCSS> viewCSS (do_QueryInterface(global));
  if (viewCSS) {
  
    nsCOMPtr<nsIDOMCSSStyleDeclaration> cssDecl;
    nsAutoString empty;
    nsCOMPtr<nsIDOMElement> domElement (do_QueryInterface(mDOMNode));
    viewCSS->GetComputedStyle(domElement, empty, getter_AddRefs(cssDecl));
    if (cssDecl) {
      nsAutoString behavior; behavior.Assign(NS_LITERAL_STRING("list-style-
image"));
      nsAutoString value;
      cssDecl->GetPropertyValue(behavior, value);
      if (!value.IsEmpty()) {
        // element has an image

      }
    }  
  }

THIS IS COMPLETELY UNTESTED.... but its based on code that's in the tree. Search 
lxr for GetComputedStyle to see examples...

Thanks for the additional code.  I've got this working up to the point where the
GetPropertyValue call fails (doesn't work).  It seems that we don't have
full-enough DOM Level 2 support and only handle certain attributes.  Some
additional code needs to be written to actually get the list-style-image from
the frame (or some such).

You'll find out as soon as you go to test your code :-).
Bug for implementing GetComputedStyle is 42417.

Not setting depends as we don't need a full fix to enable this.

hmm, well, it seems this might be as simple as adpating this the implementation 
of GetBackgroundImage and using it to implement GetListStyleImage in 
nsComputedDOMStyle.cpp

Something like this, maybe:
nsComputedDOMStyle::GetListStyleImage(nsIFrame *aFrame,
                              nsIDOMCSSPrimitiveValue*& aValue)

{
  nsresult result=NS_OK;
   
   if(aFrame) {
     const nsStyleList* list=nsnull;
     GetStyleData(eStyleStruct_List,(const nsStyleStruct*&)list,aFrame);
     if(list) {
       nsROCSSPrimitiveValue* val=GetROCSSPrimitiveValue();
       if(val) {
         val->SetString(list->mListStyleImage);
         result=val->QueryInterface(NS_GET_IID(nsIDOMCSSPrimitiveValue),(void **
)&aValue);
       }
       else {
         result=NS_ERROR_OUT_OF_MEMORY;
       }
     }
   }
   return result;
 }

Would also need to add some glue in the nsComputedDOMStyle::GetPropertyCSSValue 
method. Obv. I don't fully understand what's being done but looking at the 
GetBackgroundImage method and the way it seems to work I figure the above code 
would end up getting us an "instance" of the struct:
struct StyleListImpl: public nsStyleList which is defined in content/base/src/
nsStyleContext.cpp and that ought to get the list-style-image URL

If I get time I'll work on this, but if you have a partial implementation of 
something already and can get to this first, law?

Well, I hacked on nsComputedDOMStyle.cpp to have GetPropertyValue for
list-style-image.

Bad news: that don't work.  It seems lots of xul content depends on inheriting
that style attribute in such a manner that if you change it, that URL gets used
to display random stuff all over the place (e.g., in separators on table column
headers).

So, Dave Hyatt volunteered to add a "-moz-window-icon" style attribute to a new
XUL style struct he is working on for bug 70809.  So we'll be going with that.
Adding link to bug 70809.
Depends on: 70809
No longer depends on: 70809
Depends on: 70809
No longer depends on: 70809
Depends on: 70904
Depends on: 70974
No longer depends on: 70904
nsComputedDOMStyle.cpp changes look good to me (well, it needs some cleaning up,
and it doesn't work if there's no frame for the element, but those problems are
from the code you copied so that's ok, I (or harishd) will do the cleaning up
later). r/a=jst, please land these changes ASAP.
should we check in a cleaned up version of that patch anyway, just to be more
CSS2 compliant?
The null check for the frame is unnecessary ( actually, I introduced this
redundency ). I'll clean it up. The patch looks good to me. r=harishd
Fix the autostring that has the name "behavior".  The variable name should be
windowIcon or something.

Please don't use the icons at http://www.crosswinds.net/~ggc/ for the reasons
I've described in bug 27254. I have some new ones attached to that bug and 
gcc said he would some new ones too.
[sorry. i wrote the wrong bug number in my previous comment. re-submitting]


Please don't use the throbber based M icons at 
http://www.crosswinds.net/~ggc/ for the reasons I've described in bug 43647.
I have some new ones attached to that bug and gcc said he would do some new
ones too.

The icons at http://www.crosswinds.net/~ggc/ are the only usable thing I've got
at this point.  Somebody will have to come up with windows .ico files using the
images posted in that other bug (32x32 and 16x16, 16K colors).  I'm not sure
about the color depth, though.

I like his splash screen, too.  That needs to be a windows .bmp (although I can
use IE to convert just about anything to a .bmp).  The code for bug 35866 has to
be tuned for the splash screen.
I'm gonna go with a vote for the icons at http://www.crosswinds.net/~ggc/ .
These seem to be the cleanest and most thought out designs I have seen yet, and
are very recognizable. They respect the past while modernizing it, just like
Mozilla itself. I also like the splash screens. Any other votes?
IMHO ggc's icons are the best we have at the moment. We should checkin the 
backend code and the icons that ggc has whipped up. Currently doing nothing and 
just arguing about anything and everything isnt that constructive in the end. 
It's not like it is hard to throw in a new set of icons down the track when 
there made. IMHO icons are a source of flair in an app and the current one (1) 
icon isnt serving its purpose at the moment.
I'm for http://www.crosswinds.net/~ggc/ and deeply believe it's time to
implement them (as we have nothing else realy ready).

BTW, isn't this vote thing SPAM galore? I'm glad asa is not in the CC: list ;-)
sr=hyatt

If you're going to use string class references in interfaces *don't* use the old
class nsString, use nsAReadableString or nsAWritableString, in this case you
should define the SetIcon() method like this:

  NS_IMETHOD SetIcon(const nsAReadableString& anIconSpec) = 0;

Please don't check this in w/o fixing this.

Also, iconSpec.AssignWithConversion( anIconSpec ); will loose information if
anIconSpec contains non-ASCII characters, wether or not that's an issue I don't
know.
This is a working patch for windows? Want help with Mac version?

re: Mac help

I guess so, but the consensus here was that this didn't really apply to the Mac
because it doesn't have per-window icons.

Is that not the case?
Mac does not have per-window icons of this kind. It does have window icons in the 
titlebar, but they are indented to be proxies for an on-disk file that the window 
"owns".

Right... taking Navigator as an example, on Mac OS the icon in the titlebar 
should match whatever the icon in the URL bar is. Dragging it is just like 
dragging the icon in the titlebar.

This behaviour - being able to drag this icon - would be non-standard on Windows, 
neverthless IE5 implements this on Windows and I have a bug on implementing it on 
Windows and MacOS; bug 36305.

I had assumed, for this reason, that we'd probably put the icon for a shortcut on 
Navigator's window, but I guess most people were just planning to use the 16x16 
icon for Mozilla (and/or each sub component).

To do the drag thing properly, it should only be possible to drag when the 
address in the URL bar matches where the user is (the fact that this isn't 
currently true is a bug) The proxy icon in the URL bar is supposed to disappear 
when the user starts typing. The one in the title would need to be disabled (not 
draggable). 

MacIE5 also does something really nice - it shows the correct icon for everything 
loaded through a file:// URL. So if one is looking at an HTML page you're 
designing in BBEdit (a Mac text editor) the icon is the one of a BBEdit document.

Beyond this - composer is document oriented - on MacOS this means it should allow 
its icon to be dragged when its document is saved, and disable it when it is not. 
News could make an icon representing the frontmost article's URL available (the 
fact there's no option to show the address bar in Mail and News is another bug). 
Mail compose windows are similarly documents. I don't know if we form URLs for 
messages in the user's inbox? On Windows, it might be nice to offer this 
functionality, but its non standard.

In short, its complex. Perhaps we should consider using the icon being used in 
the URL bar in the window title on Windows? After all, the button in the taskbar 
shows the document's name, does it not? This is why I was making a fuss about 
thing being skinnable - if you mirror the proxy icon in the addressbar then it'll 
change with each skin.

Assuming you decide to go with a lizard head or some other Moz component icon, 
we'll need a way to do proper document proxing on Mac OS.

sorry to jump in late, but isn't there a better way to resolve resource URLs
than this? Maybe something like converting it to an nsIFile, and then retrieving
the native path? 

This is more than just a cleanliness of code issue - we really need to minimize
the number of file path parsers. We had lots of security problems in 4.x because
one of the 97 file parsers in the codebase had some wierd loophole that someone
could exploit.
Ignoring the Mac issues for the moment...I've taken jst's suggestions and 
attached an updated patch (with all files).  I switched SetIcon to take an 
nsAReadableString& and have changed the Windows code to build the icon path 
name in unicode rather than ascii.

Re: Mac icons

I think I see what you're talking about.  That seems like a different problem 
(at least in some respects) because the icon is for the document, not the 
window/app.  So specifying the icon in the <window> may not make sense for 
Mac.  That will have to be sorted out later.
Alec: Yes, we really wanted it to be easy to take a resource: url and get a 
file path.  Unfortunately, I couldn't find any way to do it.  The "resource:" 
protocol handler does it under the covers, but there didn't seem to be any way 
to re-use that logic.

If somebody can figure out a better way, I'd be glad to do it differently.
Resetting milestone to get these non-critical bugs off the radar till later 
this week.
Target Milestone: mozilla0.8.1 → mozilla0.9
patch builds on linux ok (SetIcon stubbed out on linux),
current behavior unchanged.


law, with r and sr we would consider this for 0.8.1 checkin. 
-asa (on behalf of drivers)
Whiteboard: with r= and sr= would consider for 0.8.1 checkin
Of course we don't actually have any icons, so getting the code checked in won't
do anything.  But it's ready to go.
I don't see the advantage of increasing the risk in 0.8.1, esp given that we
don't have icons even designed.
OK, pulling off the 0.8.1 radar. Thanks.
Whiteboard: with r= and sr= would consider for 0.8.1 checkin
*** Bug 72650 has been marked as a duplicate of this bug. ***
Depends on: stopthewhining
Chris McAfee is working on the Linux implementation and I've had to explain some
details that probably should be recorded for posterity...

First, the code in the attached patch provides the basic window/widget code and
a Win32 implementation only.  After the patch is applied, nsXULWindows will
attempt to get the -moz-window-icon style property for the window and pass that
resource: url to nsIWidget::SetIcon.

Currently, the style system doesn't support -moz-window-icon and the property
value will also be null/empty.

In order to actually test my Win32 nsWindow::SetIcon implementation, I've
enhanced nsXULWindow::LoadIconFromXUL so that it, by default (if the style
property is not defined), use the resource url
"resource:///chrome/icons/default/<id>" (where <id> is the id= attribute on the
<window>).  If there is no id= attribute, then it pretends id="default").

Consequently, you can provide new icons (having applied this patch) by placing
*.ico files in the directory $(MOZ_DIST)/bin/chrome/icons/default.  This is kind
of broken, because some window share the same id= attribute value but shouldn't
have the same icon (e.g., Navigator and Composer).  But it should let you play
around with new icons (on Win32).

Remaining tasks are:

1. Check this patch in.
2. Complete Linux implementation of nsWindow::SetIcon.
3. Get real icons in both Win32 .ico format (16x16 and 32x32) and whatever the
Linux/GTK format is.
4. Tweak the build system to push the icon files to .../icons/default/ directory.
5. "Style" each window to specify the proper icon.  This can be done by adding
.css to set the -moz-window-icon property (requires #6, below), or, by setting
the id= attribute on the window to match the icon file name.
6. Extend the "xul" style property struct so that it properly handles
-moz-window-icon.  This is bug 70974.

We'll use this bug to track tasks 1 and 2.

I've opened bug 73712 to track items 3, 4, and 5.
No longer depends on: stopthewhining
Depends on: stopthewhining
I created bug 76211 to address the problem of not having an implementation of 
SetIcon on Linux/GTK.

The bulk of the work is done and the icons should work when the "depends on" 
bugs are fixed.
Depends on: 76211
Resetting target milestone to match blocking bugs.
Target Milestone: mozilla0.9 → mozilla0.9.1
nav triage team:

Reassign to vishy to drive new icons, marking p2
Assignee: law → vishy
Priority: -- → P2
I hate to point this out at this late stage, but according to my documentation
a) Windows 9x doesn't support LoadImageW
b) Windows NT doesn't support LR_LOADFROMFILE
c) Icons can't be loaded from skin jars
I expect this will be fixed by a forthcoming gif to ico converter.
See also bug 29856, "*nix only : Window Class the same for all mozilla windows"
nav triage: moving to Future.
Target Milestone: mozilla0.9.1 → Future
I don't believe we can't do icons!
I don't either.  Did law, mscott and others really do all this backend work so 
we could throw in the towel when it's time to choose the icons?  But bug 73712 
seems to be handling that now.  I don't see any reason for this bug to remain 
open (but correct me if I'm wrong) -- task 1 seems to have been completed, and 
task 2 is now bug 76211 -- so marking fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Summary: Apps in the suite should use different/distinct icons → Apps in the suite should be able to use different/distinct icons
This code will now work with everything except Windows NT v3.x.  I don't 
suspect that particular version is widely enough used to warrant finding a way 
to load the .ico files by some other means.

I'm resetting the target milestone.  Suggest this go in mozilla0.9.1.  Need 
review.
Status: RESOLVED → REOPENED
Keywords: mozilla1.0 → patch, review
Resolution: FIXED → ---
Target Milestone: Future → ---
Have we got any suitable icons to use yet? There is no point putting this into
0.9.1 if there is no benefit to the user.

Then again, any icons (no matter how ugly) would be better than the current
situation.
The benefit for the user is to take the risk hit
in the beta cycle instead of the rtm cycle.
It's something we're gonna need, i vote to get this in.
Ian Thomas: you asked if we have some suitable icons to use. Hell yes! Look at
the link provided on this bug: http://www.crosswinds.net/~ggc/ I love those icons!
I'd love it if we could use those icons, as you will see from my comments in bug
43647. However, that bug was closed because there was argument about whether
those icons were suitable. I suppose you can never resolve things like that with
an open source project, so we might as well check them in and let them be
changed later.
*** Bug 84563 has been marked as a duplicate of this bug. ***
Target Milestone: --- → mozilla0.9.3
Resetting target milestone.  This really, really should go in mozilla0.9.2.

The fix is simple and has been reviewed (by Naoki, although it was in the
bugscape counterpart to this bug).  Need super-review and approvale from drivers.
Whiteboard: fix in hand
Target Milestone: mozilla0.9.3 → mozilla0.9.2
sr=alecf
Whiteboard: fix in hand → fix in hand, have r/sr, need a.
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
Please don't use giovanni's newer icons - they are too detailed to be recongnizable.

Use his previous set, which are a masterpiece in design. 

See them here: http://www.crosswinds.net/~ggc/oldstuff/index.html
I agree, the new icons (while being the second best I've seen) are not as good
at the originals. The 32x32 version is alright, but the 16x16 version sucks.
Besides, there is only one icon, we need a set for each sub-application.

Removing needs a= from status whiteboard
Whiteboard: fix in hand, have r/sr, need a. → fix in hand, have r/sr/a.
Yes, the previous icons are better.  Besides, I didn't see a mail icon in the
new set.
Please take your icon advocacy to the n.p.m.ui newsgroup. Bugzilla is not the
place to express your aesthetic sensibilities.  With every "I like that icon
more/less" comment you add to this bug you make it less likely to get fixed and
verified in a reasonable timeframe.  (did you know that QA has to read through a
bug before it can be verified? take a moment and read through this bug. how long
did that take? now do that for the other 6000 Fixed bugs that need verifying.
see my point?).  If you would like to respond to my comment please do so via
email and stop polluting this bug. Thanks.
patch dated 05/25/01 checked in on behalf of law.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
QA Contact: sairuh → claudius
OK, I'm confused now. Is this patch the same as bug 47779 ?
If so, then are there now bits of code doing exactly the same thing?
No longer blocks: 43647
No longer depends on: stopthewhining
Blocks: stopthewhining
I think bug 47779 is talking about getting an correctly packaging icons for each
Mozilla component in the build (with reference to corresponding Netscape
specific icons in the Netscape commercial build). This bug on the other hand is
talking about adding the support so that Mozilla uses those icons correctly when
it runs. The last patch in this bug was to make it work correctly on some
Windows OS's esp. Win98. 
sufferin' succotash! I just read this whole damn bug and there's nothing for me to do.
(see Asa's 6-19 comments)
Code-level fix, marking VERIFIED Fixed.
Status: RESOLVED → VERIFIED
OK, the *ability* is fixed. I created bug 87450 to actually get it implemented ;)
Blocks: 87450
No longer depends on: 70974
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: