The new password notification bar needs an icon to the left of the notification text. There is blank space there now.
I'm not really sure if an icon adds value here or not. Alex?
Yeah, it needs an icon. Tell me what size and I'll attach a temporary one to this bug, and add it to our list of icons to have produced.
chrome://browser/skin/Info.png is currently being used in a few of the other notification bars, and is 16x16.
Assignee: nobody → dolske
Target Milestone: --- → Firefox 3 M11
Created attachment 300609 [details] [diff] [review] patch - v1 Use chrome://mozapps/skin/passwordmgr/key.png as the icon.
Assignee: dolske → reed
Status: NEW → ASSIGNED
Attachment #300609 - Flags: review?(dolske)
Comment on attachment 300609 [details] [diff] [review] patch - v1 I think you'll also need to update toolkit/themes/*/mozapps/jar.mn to include a reference to key.png.
Created attachment 300610 [details] [diff] [review] add to jar.mn - v1 (In reply to comment #6) > (From update of attachment 300609 [details] [diff] [review]) > I think you'll also need to update toolkit/themes/*/mozapps/jar.mn to include a > reference to key.png. Yep, I usually just do that when landing the new icons, but since you asked, here's a patch for winstripe and pinstripe. I'm including the addition to gnomestripe in another bug, so not changing it here.
Created attachment 300616 [details] [diff] [review] combined patch - v1 Combined patch... gnomestripe already has its new icon and jar.mn changes already done.
Attachment #300616 - Flags: review?(gavin.sharp) → review+
Comment on attachment 300616 [details] [diff] [review] combined patch - v1 Simple patch to make the password manager's notification bar use an icon (using temporary icon for pinstripe/winstripe and completed/final icon for gnomestripe).
Attachment #300616 - Flags: approval1.9b3?
Checking in toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js; /cvsroot/mozilla/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js,v <-- nsLoginManagerPrompter.js new revision: 1.19; previous revision: 1.18 done Checking in toolkit/themes/pinstripe/mozapps/jar.mn; /cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/jar.mn,v <-- jar.mn new revision: 1.26; previous revision: 1.25 done RCS file: /cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/passwordmgr/key.png,v done Checking in toolkit/themes/pinstripe/mozapps/passwordmgr/key.png; /cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/passwordmgr/key.png,v <-- key.png initial revision: 1.1 done Checking in toolkit/themes/winstripe/mozapps/jar.mn; /cvsroot/mozilla/toolkit/themes/winstripe/mozapps/jar.mn,v <-- jar.mn new revision: 1.23; previous revision: 1.22 done RCS file: /cvsroot/mozilla/toolkit/themes/winstripe/mozapps/passwordmgr/key.png,v done Checking in toolkit/themes/winstripe/mozapps/passwordmgr/key.png; /cvsroot/mozilla/toolkit/themes/winstripe/mozapps/passwordmgr/key.png,v <-- key.png initial revision: 1.1 done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3 beta4 → Firefox 3 beta3
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre. Also verified on Windows Vista nightly.
Status: RESOLVED → VERIFIED
One thing more, please. You need to allow the icon to be accessible through themes. Right now it is hard-coded. Although we themers can swap out the image with one we like, we might have a preexisting image we want to use for the purpose. Requiring a separate image can be wasteful. In general, every icon you put in fx should be defined in a theme-based css file so we themrs have access to it.
The save-password bar's icon is set the same way as every other notification bar icon, so there's a problem with that it should be taken up in a new bug (for the general issue). [That said, can't themes set |override| directives in chrome.manifest?]
(In reply to comment #12) > Although we themers can swap out the image with one we like, we might have a > preexisting image we want to use for the purpose. Requiring a separate image > can be wasteful. Not sure what you mean by "requiring a separate image". You can override multiple images with the same source image, right?
When the name and address of an image is hard-coded, all we can do in theme files is swap out an image while keeping the same name and address. When an image is specified in css, OTOH, we can indeed assign many functions to the same image. I'm not sure we can override things in chrome.manifest from themes. They are not extensions, after all. If that does work, and if we have to set overrides for every hard-coded image, our chrome.manifest files are going to get big, complex and confusing. They will act as a barrier to thorough theming. Now that you mention other notification bar icons, I can see that this is a more pervasive problem that I had thought. I'll file a separate bug.
(In reply to comment #15) > When the name and address of an image is hard-coded, all we can do in theme > files is swap out an image while keeping the same name and address. When an > image is specified in css, OTOH, we can indeed assign many functions to the > same image. I still don't understand. My understanding is that tour new image has a URI, something like chrome://yourtheme/skin/image.png. You can make the hard-coded URI used in Firefox code map to your new image's URI using chrome overrides. You can still use chrome://yourtheme/skin/password.png in other places, with CSS, etc. I might be missing something; indeed I am not a theme developer and don't have much experience creating them. It would be good to clear up any misunderstandings with a newsgroup post rather than going back and forth in different bugs.
I started a thread in the Mozillazine forums: http://forums.mozillazine.org/viewtopic.php?p=3241514#3241514 I will just say that themes use chrome.manifest to tell Firefox where to find the various directories. I have never seen an override code in a theme's chrome.manifest. Although extensions have wide latitude, I do not know if mere themes can override anything in chrome.manifest.
Looks like you filed bug 415400 on the general issue, thanks. [All: please use that bug for further comments.]
You need to log in before you can comment on or make changes to this bug.