424877 - remove FAMFAMFAM / Silk icons from themes

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P1)

Description

[Mike Beltzner [:beltzner, not reading bugmail]]

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

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

The solution for this beta will be to use the winstripe icons, which aren't FAMFAMFAM. a=beltzner for copying those files over.

Comment 2

[Dave Townsend [:mossop]]

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

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

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.

Comment 4

[Shawn Wilsher :sdwilsh]

download manager icons have been gone for a while now.

Comment 5

[Reed Loden [:reed]]

This got unfixed by the new proto landing in bug 427555.

Comment 6

[Kevin Gerich]

Ugh, sorry. Will back out when the tree opens. Do I need approval to back my changes out?

Comment 7

[Alex Faaborg [:faaborg] (Firefox UX)]

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

Comment 8

[Reed Loden [:reed]]

http://mxr.mozilla.org/seamonkey/source/browser/themes/gnomestripe/browser/places/wrench.png, too, I guess?

Comment 9

[Reed Loden [:reed]]

(In reply to comment #6)
> Ugh, sorry. Will back out when the tree opens. Do I need approval to back my
> changes out?

No.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

>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.

Comment 11

[mcdavis941 (sporadically reading bugmail)]

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

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

I thought we used these:

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

http://mxr.mozilla.org/seamonkey/source/toolkit/themes/gnomestripe/global/dirListing/

http://mxr.mozilla.org/seamonkey/source/toolkit/themes/winstripe/global/dirListing/

Comment 13

[Mike Connor [:mconnor]]

Alex, can you drive replacements for all of the ones we missed?

Comment 14

[mcdavis941 (sporadically reading bugmail)]

(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.

Comment 15

[mcdavis941 (sporadically reading bugmail)]

Attachment: 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.

Comment 16

[Alex Faaborg [:faaborg] (Firefox UX)]

>Alex, can you drive replacements for all of the ones we missed?

yep

Comment 17

[Michael Monreal [:micmon]]

(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.

Comment 18

[Dão Gottwald [:dao]]

(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.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

(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?

Comment 20

[Reed Loden [:reed]]

(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.

Comment 21

[Dão Gottwald [:dao]]

Attachment: use updated Windows icons in file listings

Comment 22

[Boris Zbarsky [:bzbarsky]]

I won't be able to get to this until June.  If this is needed before that, please ask someone else.

Comment 23

[Alex Faaborg [:faaborg] (Firefox UX)]

Is up.png encoded as a data URL as well?

Comment 24

[mcdavis941 (sporadically reading bugmail)]

(In reply to comment #23)
> Is up.png encoded as a data URL as well?
> 

Not that I'm aware of.

Comment 25

[mcdavis941 (sporadically reading bugmail)]

Attachment: 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.

Comment 26

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 314375 [details] [diff] [review]
use updated Windows icons in file listings

rs=biesi

Comment 27

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 314375 [details] [diff] [review]
use updated Windows icons in file listings

a1.9=beltzner

Comment 28

[:Gavin Sharp [email: [e-mail]]]

mozilla/netwerk/streamconv/converters/nsIndexedToHTML.cpp 	1.91