Closed
Bug 418877
Opened 17 years ago
Closed 17 years ago
Error/Warning/Information/Question icons
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 3
People
(Reporter: micmon, Assigned: ispence)
References
Details
Attachments
(3 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
beltzner
:
approval1.9+
|
Details |
(deleted),
patch
|
Gavin
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
It has been decided that for the Error/Warning/Information/Question (M3) set of icons we will use GTK stock icons:
Error: gtk-dialog-error
Information: gtk-dialog-info
Question: gtk-dialog-question
Warning: gtk-dialog-warning
We need to find all the places where we currently use a buildin icon and replace them with the right GTK icons. Mostly:
- Error/Warning pages/fav icons
- Notifications
- Error console
Reporter | ||
Comment 1•17 years ago
|
||
This shows a few places that need to be fixed, but there are many more... Hopefully someone can find a way to reliably find them all.
Assignee | ||
Comment 2•17 years ago
|
||
School is closed thanks to 4-8 inches of snow, so I'll give this a shot today
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•17 years ago
|
||
Let's cover the whole error console here. The old general/console/console-toolbar.png included error/warning/info/clear, which can all be used from gtk: gtk-dialog-error, gtk-dialog-warning, gtk-dialog-information, gtk-clear. The "main" icon is the only remaining icon on this bitmap (I don't think we need teh disabled state).
Assignee | ||
Comment 4•17 years ago
|
||
I include the winstripe results because in the case of "console.css", we need
to copy it from winstripe first.
gnometripe/global/findBar.css
gnomestripe/mozapps/update/updates.css
gnomestripe/browser/safebrowsing/browser-protection.css
winstripe/global/console/console.css
winstripe/mozapps/update/update.css
winstripe/mozapps/xpinstall/xpinstallConfirm.css
winstripe/global/config.css
Those files were found using error icons, info icons, blacklist icons, warning
icons
The only issue right now is that the "Page Load Error" icon that you see in the
location bar seems to be hardcoded (really hardcodes. src="data:image/png"
hardcoded)
Updated•17 years ago
|
Assignee: nobody → ispence
Status: ASSIGNED → NEW
Comment 5•17 years ago
|
||
Didn't you say earlier that the default gtk theme only produces those icons in dialog size? In that case would it be better to find more suitable stock icons for the 16x16 case, like gtk-info?
Updated•17 years ago
|
Flags: blocking-firefox3?
Reporter | ||
Comment 6•17 years ago
|
||
(In reply to comment #5)
> Didn't you say earlier that the default gtk theme only produces those icons in
> dialog size?
This is correct. However, i have tested how those icons look when scaled down and found them to still look good enough. Alex Faaborg agreed with that. Note that icon themes like gnome-icon-theme or tango-icon-theme provide pixel-perfect versions for all the sizes.
Assignee: ispence → nobody
Updated•17 years ago
|
Assignee: nobody → ispence
Assignee | ||
Comment 7•17 years ago
|
||
ok, so before I go further, would it be reasonable to assume that the icon attached could be referenced via chrome://global/skin/console/console-large.png, or should I assume it will overwrite chrome://global/skin/console/console-toolbar.png?
Also, am I correct in assuming that we are just scaling down icons and no longer creating an entirely new set like previously thought?
Reporter | ||
Comment 8•17 years ago
|
||
(In reply to comment #7)
> ok, so before I go further, would it be reasonable to assume that the icon
> attached could be referenced via
> chrome://global/skin/console/console-large.png, or should I assume it will
> overwrite chrome://global/skin/console/console-toolbar.png?
Doesn't really matter... we can either just replace the current image or add the console icon with the new name and remove the old image.
> Also, am I correct in assuming that we are just scaling down icons and no
> longer creating an entirely new set like previously thought?
Yes. We would never be able to create a set of error/warning/etc icons that fit nicely with EVERY theme out there.
Note that only users that do not have any desktop icon theme set (fluxbox users for example) will ever see the scaled icons and I don't think they will really notice. Users of DEs like GNOME, Xfce, KDE4 etc will get all the required sizes from their theme.
Assignee | ||
Comment 9•17 years ago
|
||
What I am wondering most about is if I do "moz-icon://stock/gtk-dialog-error?size=menu" on a system without a gnome theme, will I get back a scaled down icon, or a full sized icon that I will have to scale? Unforunately, I have a gnome theme so I've been unable to find a case where I have a large version of an icon but not a small version (even with the nonstandard icons)
Reporter | ||
Comment 10•17 years ago
|
||
I tested this... quit FF3. Select the default gnome theme as your icon theme, then do:
# mv /usr/share/icons/gnome /usr/share/icons/gnome-bak
Now start FF3, you should get the GTK fallback icons. You can test moz-icon://stock/gtk-dialog-error?size=menu from there.
Assignee | ||
Comment 11•17 years ago
|
||
Excellent. Turns out I get exactly what I want
Assignee | ||
Comment 12•17 years ago
|
||
*Thank you*
(Hate it when I forget to thank someone)
Assignee | ||
Comment 13•17 years ago
|
||
This adds the correct icons to the error console, the about:config warning, the updates dialog, the find toolbar, and various extension notification bars.
The favicon for that error page is hardcoded into mozilla/docshell/resources/content/netError.xhtml, so I can't change it based on theme
Attachment #305249 -
Flags: superreview?(roc)
Attachment #305249 -
Flags: review?(roc)
Reporter | ||
Comment 14•17 years ago
|
||
Nice! But what about this hardcoded icon? The mac theme will run into the same problem I suppose...
Comment 15•17 years ago
|
||
Comment on attachment 305249 [details] [diff] [review]
Adds the use of Error, warning
roc isn't a valid reviewer of this.
Can we just override the chrome URL for extensions/question.png instead of doing the #ifdef everywhere?
Attachment #305249 -
Flags: superreview?(roc)
Attachment #305249 -
Flags: review?(roc)
Attachment #305249 -
Flags: review?(rflint)
Assignee | ||
Comment 16•17 years ago
|
||
I don't have the knowledge of the chrome to do that
Comment 17•17 years ago
|
||
I think it would be good to use gtk-execute on the error console's Evaluate button.
Assignee | ||
Comment 18•17 years ago
|
||
So I looked up how to do chrome overrides and I'm getting an error because it does not consider moz-icon://XXX as a local file (Possibly a bug I need to file, but will wait until tomorrow to see any replies)
Assignee | ||
Comment 19•17 years ago
|
||
Reed, since the chrome override doesn't seem like it's going to be possible, would you prefer I just had something like
#ifdef MOZ_WIDGET_GTK2
var notification_icon = "moz-icon://stock/gtk-dialog-info?size=menu";
#else
var notification_icon = "chrome://mozapps/skin/extensions/question.png";
#endif
somewhere appropriate towards the top of the code?
Assignee | ||
Comment 20•17 years ago
|
||
Just a little bump to get Reed's feedback on Comment #19
Comment 21•17 years ago
|
||
This will not block the final release of Firefox 3.
Flags: blocking-firefox3? → blocking-firefox3-
Comment 22•17 years ago
|
||
Mano / Gavin / Ryan: Do you know if there's a better way than what comment #19 says?
Comment 23•17 years ago
|
||
Here is a hopefully complete list of the occurrences of the icons.
I list where a given icon is located, and where it is used.
Error:
chrome/classic/skin/classic/global/console/bullet-error.png
chrome/classic/skin/classic/global/console/console.css
chrome/classic/skin/classic/global/console/console-toolbar.png
chrome/classic/skin/classic/global/console/console.css
chrome/classic/skin/classic/global/icons/Error.png
Dind't find where it is used, if at all.
chrome/classic/skin/classic/global/icons/notfound.png
chrome/classic/skin/classic/mozapps/update/updates.css
Information:
chrome/classic/skin/classic/global/console/bullet-question.png
chrome/classic/skin/classic/global/console/console.css
chrome/classic/skin/classic/global/console/console-toolbar.png
chrome/classic/skin/classic/global/console/console.css
Question:
chrome/classic/skin/classic/mozapps/extensions/question.png
chrome/toolkit/content/mozapps/extensions/extensions.js
chrome/classic/skin/classic/global/console/console.css
chrome/classic/skin/classic/global/icons/Question.png
Dind't find where it is used, if at all.
Warning:
chrome/classic/skin/classic/global/console/bullet-warning.png
chrome/classic/skin/classic/global/console/console.css
chrome/classic/skin/classic/global/console/console-toolbar.png
chrome/classic/skin/classic/global/console/console.css
chrome/classic/skin/classic/mozapps/update/extensionalert.png
Dind't find where it is used, if at all.
chrome/classic/skin/classic/global/icons/warning-large.png
chrome/pippki/content/pippki/exceptionDialog.xul
chrome/classic/skin/classic/global/config.css
chrome/classic/skin/classic/mozapps/update/warning.gif
Dind't find where it is used, if at all.
chrome/classic/skin/classic/global/icons/Warning.png
chrome/classic/skin/classic/mozapps/update/updates.css
--
To get the list, I first used:
#!/bin/sh
for i in `find . -name "*.jar"`; do
unzip -d "${i%.jar}" "${i}"
rm "${i}"
done
mkdir images
for i in `find . \
-name "*.bmp" -or -name "*.bmP" -or -name "*.bMp" -or -name "*.bMP" -or \
-name "*.Bmp" -or -name "*.BmP" -or -name "*.BMp" -or -name "*.BMP" -or \
-name "*.gif" -or -name "*.giF" -or -name "*.gIf" -or -name "*.gIF" -or \
-name "*.Gif" -or -name "*.GiF" -or -name "*.GIf" -or -name "*.GIF" -or \
-name "*.ico" -or -name "*.icO" -or -name "*.iCo" -or -name "*.iCO" -or \
-name "*.Ico" -or -name "*.IcO" -or -name "*.ICo" -or -name "*.ICO" -or \
-name "*.jpg" -or -name "*.jpG" -or -name "*.jPg" -or -name "*.jPG" -or \
-name "*.Jpg" -or -name "*.JpG" -or -name "*.JPg" -or -name "*.JPG" -or \
-name "*.png" -or -name "*.pnG" -or -name "*.pNg" -or -name "*.pNG" -or \
-name "*.Png" -or -name "*.PnG" -or -name "*.PNg" -or -name "*.PNG" -or \
-name "*.xbm" -or -name "*.xbM" -or -name "*.xBm" -or -name "*.xBM" -or \
-name "*.Xbm" -or -name "*.XbM" -or -name "*.XBm" -or -name "*.XBM" -or \
-name "*.xpm" -or -name "*.xpM" -or -name "*.xPm" -or -name "*.xPM" -or \
-name "*.Xpm" -or -name "*.XpM" -or -name "*.XPm" -or -name "*.XPM" -or \
-name "*.jpeg" -or -name "*.jpeG" -or -name "*.jpEg" -or -name "*.jpEG" -or \
-name "*.jPeg" -or -name "*.jPeG" -or -name "*.jPEg" -or -name "*.jPEG" -or \
-name "*.Jpeg" -or -name "*.JpeG" -or -name "*.JpEg" -or -name "*.JpEG" -or \
-name "*.JPeg" -or -name "*.JPeG" -or -name "*.JPEg" -or -name "*.JPEG"`; do
name=`basename "${i}"`
while [ -e "images/${name}" ]; do
name="_${name}"
done
cp "${i}" "images/${name}"
done
I view all the images, and the found in the sources where those images where used.
Hope this helps.
Comment 24•17 years ago
|
||
(In reply to comment #23)
> chrome/classic/skin/classic/mozapps/update/extensionalert.png
> Dind't find where it is used, if at all.
http://mxr.mozilla.org/mozilla/source/toolkit/themes/winstripe/mozapps/update/update.css#75
Comment 25•17 years ago
|
||
(In reply to comment #24)
> (In reply to comment #23)
>
> > chrome/classic/skin/classic/mozapps/update/extensionalert.png
> > Dind't find where it is used, if at all.
>
> http://mxr.mozilla.org/mozilla/source/toolkit/themes/winstripe/mozapps/update/update.css#75
>
OK, but must do we care about winstripe?
Comment 26•17 years ago
|
||
(In reply to comment #25)
> OK, but must do we care about winstripe?
No, but this is different from the other 3 icons you mentioned in that it is used somewhere, so needs platform specific tests if it is to be removed from the tar.bz2. Error.png and Question.png got updated yesterday but there's no code to use them yet. I think warning.gif can be removed.
Comment 27•17 years ago
|
||
(In reply to comment #26)
> (In reply to comment #25)
>
> > OK, but must do we care about winstripe?
>
> No, but this is different from the other 3 icons you mentioned in that it is
> used somewhere, so needs platform specific tests if it is to be removed from
> the tar.bz2. Error.png and Question.png got updated yesterday but there's no
> code to use them yet. I think warning.gif can be removed.
>
Ah, I see.
Reporter | ||
Comment 28•17 years ago
|
||
Hmm... this bug was marked blocking bug 418868, which has now been resolved? The Error/Warning/Information/Question are currently the most visible non-tango spots in the Linux builds, will this still make it into 3.0?
Comment 30•17 years ago
|
||
Can we split the changes to the error console off to a separate bug, using the Windows XP icons here really sticks out from all of the other tango icons.
Blocks: 425582
Assignee | ||
Updated•17 years ago
|
Attachment #305249 -
Flags: review?(rflint)
Comment 31•17 years ago
|
||
Comment on attachment 305249 [details] [diff] [review]
Adds the use of Error, warning
However, until you get a new patch ready for just the question stuff, I'll leave this r?Ryan. :)
Attachment #305249 -
Flags: review?(rflint)
Assignee | ||
Comment 32•17 years ago
|
||
I just assumed a review- since it really shouldn't go in with those #ifdefs
For those interested, split off for comment 30 is bug 427717
Updated•17 years ago
|
Attachment #304991 -
Flags: approval1.9?
Comment 33•17 years ago
|
||
Comment on attachment 304991 [details]
Console icon for error console
I don't understand what this has to do with this bug. Can someone please explain (and renom for approval?)
Attachment #304991 -
Flags: approval1.9? → approval1.9-
Comment 34•17 years ago
|
||
Comment on attachment 304991 [details]
Console icon for error console
Icon needed for bug 427717.
Attachment #304991 -
Flags: approval1.9- → approval1.9?
Comment 35•17 years ago
|
||
Comment on attachment 304991 [details]
Console icon for error console
Great, put it on that bug and ask for approval there.
Attachment #304991 -
Flags: approval1.9? → approval1.9-
Comment 36•17 years ago
|
||
Why? This bug needs the icon, too... I don't see a point in having multiple copies of the icon that might just become confusing later. The other bug clearly points to this bug for the icon and includes proper dependencies.
Comment 37•17 years ago
|
||
Comment on attachment 304991 [details]
Console icon for error console
whatever
Attachment #304991 -
Flags: approval1.9- → approval1.9+
Comment 38•17 years ago
|
||
Comment on attachment 305249 [details] [diff] [review]
Adds the use of Error, warning
Gavin, can you take a look at this, please?
Attachment #305249 -
Flags: review?(gavin.sharp)
Updated•17 years ago
|
Attachment #305249 -
Flags: review?(mano)
Comment 39•17 years ago
|
||
Updated patch removing the console changes that were done in another, simplying the config change, removing obsolete safebrowsing change, remove unneeded update change, and add another ifdef for extensions.js
Attachment #305249 -
Attachment is obsolete: true
Attachment #320089 -
Flags: review?(gavin.sharp)
Attachment #305249 -
Flags: review?(rflint)
Attachment #305249 -
Flags: review?(mano)
Attachment #305249 -
Flags: review?(gavin.sharp)
Comment 40•17 years ago
|
||
Comment on attachment 320089 [details] [diff] [review]
Updated patch
I assume that you've made sure that every single one of these works correctly and looks OK?
Attachment #320089 -
Flags: review?(gavin.sharp) → review+
Comment 41•17 years ago
|
||
(In reply to comment #40)
> (From update of attachment 320089 [details] [diff] [review])
> I assume that you've made sure that every single one of these works correctly
> and looks OK?
I've gone through each icon replacement in the patch and made sure that the replacement icon is the correct one and the correct size.
Status: NEW → ASSIGNED
Comment 42•17 years ago
|
||
Comment on attachment 320089 [details] [diff] [review]
Updated patch
Replaces lots of Windows icons with correct Linux Tango variants.
Attachment #320089 -
Flags: approval1.9?
Comment 43•17 years ago
|
||
Comment on attachment 320089 [details] [diff] [review]
Updated patch
Go Go GTK theme!
Attachment #320089 -
Flags: approval1.9? → approval1.9+
Comment 44•17 years ago
|
||
Checking in toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v <-- extensions.js
new revision: 1.176; previous revision: 1.175
done
Checking in toolkit/themes/gnomestripe/global/findBar.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/global/findBar.css,v <-- findBar.css
new revision: 1.5; previous revision: 1.4
done
Checking in toolkit/themes/gnomestripe/mozapps/update/updates.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/update/updates.css,v <-- updates.css
new revision: 1.3; previous revision: 1.2
done
Checking in toolkit/themes/winstripe/global/config.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/config.css,v <-- config.css
new revision: 1.6; previous revision: 1.5
done
Checking in toolkit/themes/winstripe/global/jar.mn;
/cvsroot/mozilla/toolkit/themes/winstripe/global/jar.mn,v <-- jar.mn
new revision: 1.59; previous revision: 1.58
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3
Comment 45•17 years ago
|
||
This patch broke config.css for Vista; you forgot to update the aero config.css entry in jar.mn. ;)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 46•17 years ago
|
||
Attachment #320255 -
Flags: review?(dao)
Comment 47•17 years ago
|
||
Comment on attachment 320255 [details] [diff] [review]
Fix bustage of config.css on Vista
Kai filed bug 433069.
Attachment #320255 -
Attachment is obsolete: true
Attachment #320255 -
Flags: review?(dao)
Updated•17 years ago
|
Attachment #320255 -
Attachment is obsolete: false
Updated•17 years ago
|
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Attachment #320255 -
Attachment is obsolete: true
Comment 48•17 years ago
|
||
Two mid-air collisions in a row, hehe...
You need to log in
before you can comment on or make changes to this bug.
Description
•