225164 - nsIconDecoder needs not be built anymore on win32

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

since Bug 222694 was fixed, nsIconDecoder.cpp needs not be built anymore on
win32 - nsICODecoder.cpp will be used instead for moz-icon urls.
hence, Makefile.in should be changed appropriately.

filing on bsmedberg on neil's request

Comment 1

[Benjamin Smedberg]

I need a little more detail in this bug ;)

1) do we still need to build all the other stuff in
http://lxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/icon/

1a) and on which platforms?

2) Why does nsICODecoder live in decoders/bmp?

Comment 2

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

1) yes, the other stuff is needed
1a) non-windows should continue to build the same file set as today. windows
should stop building nsIconDecoder.

2) "because that's where hyatt put it" :) but more seriously, it shares code
with the bmp decoder, it is a general decoder for .ico files, it works on all
platforms, it wouldn't make sense in the icon/ directory.

Comment 3

[Benjamin Smedberg]

Attachment: you test this

I don't know how to test this... what kind of regressions are we looking for
here? I don't even know what icon: uris do.  What happens if you disable the
BMP decoder... should we revert to building nsIconDecoder.cpp or do something
else?

--BDS

Comment 4

[Benjamin Smedberg]

.

Comment 5

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

> what kind of regressions are we looking for here?

basically, moz-icon: urls not working. e.g:
moz-icon://.htm
should show the icon of the program associated with .htm files (e.g. mozilla)
this is used in the helper app dialog & pref panels

> What happens if you disable the
>BMP decoder...

then that's your own fault :)

don't you need to remove icondecoder registration from nsIconModule too?

Comment 6

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

.

Comment 7

[[e-mail]]

Passing the parcel :-P

Comment 8

[Stephen Walker]

> basically, moz-icon: urls not working.
moz-icon: urls still work with the patch.

Comment 9

[Stephen Walker]

Attachment: patch

Comment 10

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

actually, gnome does not need this file either, it produces PNGs...

Comment 11

[tor]

Comment on attachment 152151 [details] [diff] [review]
patch

Since only os2 and mac need nsIconDecoder, you should make the
makefile change conditionally compile for only those platforms.

Comment 12

[Stephen Walker]

Attachment: patch

Comment 13

[Stephen Walker]

Attachment: patch

With one less tab...

Comment 14

[Stephen Walker]

-> fixed, timeless checked in the last patch with biesi's review+ carried over.