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)
SeaMonkey
UI Design
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.2
People
(Reporter: bugzilla, Assigned: vishy)
References
()
Details
(Whiteboard: fix in hand, have r/sr/a.)
Attachments
(7 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
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.
Comment 1•24 years ago
|
||
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
Comment 2•24 years ago
|
||
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
Reporter | ||
Comment 3•24 years ago
|
||
pav sez he has this working in gfx2, and kerz says he's working on it.
Comment 4•24 years ago
|
||
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
Updated•24 years ago
|
Status: NEW → ASSIGNED
Comment 5•24 years ago
|
||
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
Comment 7•24 years ago
|
||
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-
Comment 10•24 years ago
|
||
*** Bug 47779 has been marked as a duplicate of this bug. ***
Comment 11•24 years ago
|
||
*** Bug 65292 has been marked as a duplicate of this bug. ***
Comment 13•24 years ago
|
||
Assignee | ||
Comment 14•24 years ago
|
||
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 → ---
Comment 15•24 years ago
|
||
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.
Comment 16•24 years ago
|
||
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
Comment 17•24 years ago
|
||
Comment 18•24 years ago
|
||
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).
Comment 19•24 years ago
|
||
+ docShellElement->GetAttribute(NS_ConvertASCIItoUCS2("icon"), windowIcon);
please use the new string foo. NS_LITERAL_STRING("icon") i think. otherwise it
looks good.
Comment 20•24 years ago
|
||
Comment 21•24 years ago
|
||
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.
Comment 22•24 years ago
|
||
>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 :-).
Comment 23•24 years ago
|
||
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.
Comment 24•24 years ago
|
||
cc'ing hyatt and hixie for their comments.
Comment 25•24 years ago
|
||
Changing keyword to "nsbeta1+." It was set to "nsbeta1-" accidentally.
Comment 26•24 years ago
|
||
I did an icon set which includes icons for Navigator, Mail & News and Composer.
Here's the link: http://www.crosswinds.net/~ggc/
Comment 27•24 years ago
|
||
gcavallanti@gmx.it, they are really cool!!! You can make also a complete theme
based on this design! I would use it by default...
Comment 28•24 years ago
|
||
Giovanni's icons are VERY good. They should appear in the NEXT nightlies!!! ;-)
Updated•24 years ago
|
Comment 29•24 years ago
|
||
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...
Comment 30•24 years ago
|
||
ugh, the imagerequest,etc are all going away...
Comment 31•24 years ago
|
||
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?
Comment 32•24 years ago
|
||
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?
Comment 33•24 years ago
|
||
> 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.
Comment 34•24 years ago
|
||
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.
Comment 35•24 years ago
|
||
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.
Comment 36•24 years ago
|
||
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.
Comment 37•24 years ago
|
||
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
Comment 38•24 years ago
|
||
Mass moving most of mozilla0.9 bugs to mozilla0.8.1
Target Milestone: mozilla0.9 → mozilla0.8.1
Comment 39•24 years ago
|
||
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...
Comment 40•24 years ago
|
||
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 :-).
Comment 41•24 years ago
|
||
Bug for implementing GetComputedStyle is 42417.
Not setting depends as we don't need a full fix to enable this.
Comment 42•24 years ago
|
||
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?
Comment 43•24 years ago
|
||
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.
Comment 45•24 years ago
|
||
Updated•24 years ago
|
Comment 46•24 years ago
|
||
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.
Comment 47•24 years ago
|
||
should we check in a cleaned up version of that patch anyway, just to be more
CSS2 compliant?
Comment 48•24 years ago
|
||
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
Comment 49•24 years ago
|
||
Comment 50•24 years ago
|
||
Fix the autostring that has the name "behavior". The variable name should be
windowIcon or something.
Comment 51•24 years ago
|
||
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.
Comment 52•24 years ago
|
||
[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.
Comment 53•24 years ago
|
||
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.
Comment 54•24 years ago
|
||
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?
Comment 55•24 years ago
|
||
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.
Comment 56•24 years ago
|
||
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 ;-)
Comment 57•24 years ago
|
||
sr=hyatt
Comment 58•24 years ago
|
||
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.
Comment 59•24 years ago
|
||
Comment 60•24 years ago
|
||
This is a working patch for windows? Want help with Mac version?
Comment 61•24 years ago
|
||
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?
Comment 62•24 years ago
|
||
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".
Comment 63•24 years ago
|
||
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.
Comment 64•24 years ago
|
||
Comment 65•24 years ago
|
||
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.
Comment 66•24 years ago
|
||
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.
Comment 67•24 years ago
|
||
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.
Comment 68•24 years ago
|
||
Resetting milestone to get these non-critical bugs off the radar till later
this week.
Target Milestone: mozilla0.8.1 → mozilla0.9
Comment 69•24 years ago
|
||
patch builds on linux ok (SetIcon stubbed out on linux),
current behavior unchanged.
Comment 70•24 years ago
|
||
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
Comment 71•24 years ago
|
||
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.
Comment 72•24 years ago
|
||
I don't see the advantage of increasing the risk in 0.8.1, esp given that we
don't have icons even designed.
Comment 73•24 years ago
|
||
OK, pulling off the 0.8.1 radar. Thanks.
Whiteboard: with r= and sr= would consider for 0.8.1 checkin
Comment 74•24 years ago
|
||
*** Bug 72650 has been marked as a duplicate of this bug. ***
Depends on: stopthewhining
Comment 75•24 years ago
|
||
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
Comment 76•24 years ago
|
||
Comment 77•24 years ago
|
||
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
Comment 78•24 years ago
|
||
Resetting target milestone to match blocking bugs.
Target Milestone: mozilla0.9 → mozilla0.9.1
Comment 79•24 years ago
|
||
nav triage team:
Reassign to vishy to drive new icons, marking p2
Assignee: law → vishy
Priority: -- → P2
Comment 80•24 years ago
|
||
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.
Comment 81•24 years ago
|
||
See also bug 29856, "*nix only : Window Class the same for all mozilla windows"
Assignee | ||
Comment 82•24 years ago
|
||
nav triage: moving to Future.
Target Milestone: mozilla0.9.1 → Future
Comment 83•24 years ago
|
||
I don't believe we can't do icons!
Reporter | ||
Comment 84•24 years ago
|
||
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: 24 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
Comment 85•24 years ago
|
||
Comment 86•24 years ago
|
||
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
Resolution: FIXED → ---
Target Milestone: Future → ---
Comment 87•24 years ago
|
||
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.
Comment 88•24 years ago
|
||
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.
Comment 89•24 years ago
|
||
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!
Comment 90•24 years ago
|
||
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.
Comment 91•24 years ago
|
||
*** Bug 84563 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•24 years ago
|
Target Milestone: --- → mozilla0.9.3
Comment 92•24 years ago
|
||
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
Comment 93•23 years ago
|
||
sr=alecf
Assignee | ||
Updated•23 years ago
|
Whiteboard: fix in hand → fix in hand, have r/sr, need a.
Comment 94•23 years ago
|
||
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
Comment 95•23 years ago
|
||
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
Comment 96•23 years ago
|
||
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.
Comment 97•23 years ago
|
||
Yes, the previous icons are better. Besides, I didn't see a mail icon in the
new set.
Comment 98•23 years ago
|
||
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.
Assignee | ||
Comment 99•23 years ago
|
||
patch dated 05/25/01 checked in on behalf of law.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED
Updated•23 years ago
|
QA Contact: sairuh → claudius
Comment 100•23 years ago
|
||
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?
Updated•23 years ago
|
No longer depends on: stopthewhining
Updated•23 years ago
|
Blocks: stopthewhining
Assignee | ||
Comment 101•23 years ago
|
||
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.
Comment 102•23 years ago
|
||
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
Comment 103•23 years ago
|
||
OK, the *ability* is fixed. I created bug 87450 to actually get it implemented ;)
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•