remove FAMFAMFAM / Silk icons from themes

RESOLVED FIXED in Firefox 3

Status

()

P1
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: beltzner, Assigned: dao)

Tracking

Trunk
Firefox 3
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

We removed attribution of the Silk icons from FAMFAMFAM back in Beta 3, but still have them in the tree and they're being called by some of the themes.

The ones that remain are:

http://mxr.mozilla.org/seamonkey/source/toolkit/themes/pinstripe/global/dirListing/remote.png

and

http://mxr.mozilla.org/seamonkey/source/toolkit/themes/pinstripe/mozapps/passwordmgr/key.png
Flags: blocking-firefox3+
Priority: -- → P1
The solution for this beta will be to use the winstripe icons, which aren't FAMFAMFAM. a=beltzner for copying those files over.
Replaced those with the winstripe equivalents. Don't know if there are any others around?

Checking in global/dirListing/remote.png;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/dirListing/remote.png,v  <--  remote.png
new revision: 1.2; previous revision: 1.1
done
Checking in mozapps/passwordmgr/key.png;
/cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/passwordmgr/key.png,v  <--  key.png
new revision: 1.2; previous revision: 1.1
done
Resolving this FIXED for now, but could the people I'm cc'ing here (dao, sdwilsh, dietrich, mconnor, dolske) quickly check to make sure that any FAMFAMFAM icons they added have been removed/replaced by now? If not, please re-open.

Alex is confident we've got them all, but it would be nice to double check.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
download manager icons have been gone for a while now.
This got unfixed by the new proto landing in bug 427555.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 6

11 years ago
Ugh, sorry. Will back out when the tree opens. Do I need approval to back my changes out?
Note for windows, wrench is famfamfam, it really shouldn't still appear in the product, but setting this to depend on 424286

http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/wrench.png
Depends on: 424286
(In reply to comment #6)
> Ugh, sorry. Will back out when the tree opens. Do I need approval to back my
> changes out?

No.
>too, I guess?

Yeah, these are in no way related to the icons we plan to use, was just a placeholder that is surprisingly still there this late in the game.
It looks like dirlisting/remote.png and dirlisting/local.png are represented as
data: URIs in nsIndexedToHTML.cpp:

http://tinyurl.com/4x35c8
http://tinyurl.com/5xu7h5
Alex, can you drive replacements for all of the ones we missed?
Assignee: dtownsend → faaborg
Status: REOPENED → NEW
(In reply to comment #12)
> I thought we used these:
> 

I'm not totally sure, someone else would need to check it.  I'm mentioning it just in case.  I don't even know if that code is used, but if you take that data URI (the remote one) and paste into the location bar it will render as the old remote.png icon.
Created attachment 314288 [details]
screenshot showing old remote.png for FTP listing

Screenshot from the 20080407 nightly, with the old remote.png icon as favicon for an FTP listing, even though the new remote.png is available at chrome://global/skin/dirListing/remote.png.

This is from a new profile, so there's no issue with favicons coming from history.
>Alex, can you drive replacements for all of the ones we missed?

yep
(In reply to comment #15)
> Screenshot from the 20080407 nightly, with the old remote.png icon as favicon
> for an FTP listing, even though the new remote.png is available at
> chrome://global/skin/dirListing/remote.png.

Yeah the same problem exists for file:/// URIs where the wrong icon is also displayd (the winstripe folder icon on linux at least). IIRC those icons are hardcoded still, hopefully the themed ones can be used soon.
(Assignee)

Comment 18

11 years ago
(In reply to comment #14)
> I'm not totally sure, someone else would need to check it.  I'm mentioning it
> just in case.  I don't even know if that code is used

Yes, it is.
Assignee: faaborg → dao
Target Milestone: Firefox 3 beta5 → Firefox 3
(In reply to comment #17)
> Yeah the same problem exists for file:/// URIs where the wrong icon is also
> displayd (the winstripe folder icon on linux at least). IIRC those icons are
> hardcoded still, hopefully the themed ones can be used soon.

Hardcoded where? What do we need to do to get them themed? Is there a bug on that?

(In reply to comment #19)
> Hardcoded where? What do we need to do to get them themed? Is there a bug on
> that?

Bug 388553.
(Assignee)

Comment 21

11 years ago
Created attachment 314375 [details] [diff] [review]
use updated Windows icons in file listings
Attachment #314375 - Flags: superreview?(bzbarsky)
Attachment #314375 - Flags: review?(bzbarsky)
I won't be able to get to this until June.  If this is needed before that, please ask someone else.
(Assignee)

Updated

11 years ago
Attachment #314375 - Flags: superreview?(darin.moz)
Attachment #314375 - Flags: superreview?(bzbarsky)
Attachment #314375 - Flags: review?(darin.moz)
Attachment #314375 - Flags: review?(bzbarsky)
(Assignee)

Updated

11 years ago
Attachment #314375 - Flags: superreview?(darin.moz)
Attachment #314375 - Flags: review?(darin.moz)
Attachment #314375 - Flags: review?(cbiesinger)
Attachment #314375 - Flags: review?(bzbarsky)
Is up.png encoded as a data URL as well?
(In reply to comment #23)
> Is up.png encoded as a data URL as well?
> 

Not that I'm aware of.

Created attachment 314478 [details]
mxr search results for data:image/

(In reply to comment #24)
> (In reply to comment #23)
> > Is up.png encoded as a data URL as well?
> > 
> 
> Not that I'm aware of.
> 
And especially not in the section of code where those other two were.

Attached is a set of search results from MXR, searching for "data:image/", edited to remove results like unit tests that obviously aren't in-product.  It doesn't show any other place where a data URL is used to represent one of the FamFamFam icons.

Updated

11 years ago
Depends on: 427935
Comment on attachment 314375 [details] [diff] [review]
use updated Windows icons in file listings

rs=biesi
Attachment #314375 - Flags: review?(cbiesinger) → review+
(Assignee)

Updated

11 years ago
Attachment #314375 - Flags: review?(bzbarsky)
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
(Assignee)

Updated

11 years ago
Attachment #314375 - Flags: approval1.9?
Comment on attachment 314375 [details] [diff] [review]
use updated Windows icons in file listings

a1.9=beltzner
Attachment #314375 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Whiteboard: [has patch][has reviews]
mozilla/netwerk/streamconv/converters/nsIndexedToHTML.cpp 	1.91
Status: NEW → RESOLVED
Last Resolved: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews]
You need to log in before you can comment on or make changes to this bug.