Bug 28073 – Cross-platform ICO decoder gets wrong bit depth for icons w/o color count

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Peter Kasting]

The ICO decoder ranks icons by quality, which is calculated first by size, then by bit depth.  To find the bit depth to use, we read the directory entry's wBitCount and bColorCount entries.  If the bit count is nonzero, it is used; otherwise, the bit depth is calculated from the color count.

Apparently, real-world icons exist which use wBitCount == 0, bColorCount == 0 to mean "256 colors".  The URL above is one such example; imdb.com/favicon.ico is another.  I already knew 0 meant "256" for bWidth and bHeight, but I didn't know that for bColorCount.  http://msdn.microsoft.com/en-us/library/ms997538.aspx implies this is legitimate (although it's not clear).

This is a simple one-liner.

[Peter Kasting]

Created attachment 34294 [details]
patch

OK, not quite one line, but pretty simple.

[Eric Seidel (no email)]

Comment on attachment 34294 [details]
patch

You and your wrapping!

Test case?

Our style is a bit lame, but this is technically stylistically wrong:
+        if (colorCount == 0)

r- for lack of test.

[Peter Kasting]

Created attachment 34342 [details]
patch w/test, no expected results

Here's the patch with the style violation fixed and a layout test added.  I don't seem to be able to generate expected pixel results on my Windows box.  Expected results should be identical for all platforms, though.  Not sure if there's some way to have the buildbot do this for me or something?

[Eric Seidel (no email)]

Comment on attachment 34342 [details]
patch w/test, no expected results

Fantastic!  Will need pixel test results but those can be added in separate commits.

[Peter Kasting]

Fixed in r47042.

[Eric Seidel (no email)]

Adding results:
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/images/icon-0colors-expected.checksum
	A	LayoutTests/fast/images/icon-0colors-expected.png
	A	LayoutTests/fast/images/icon-0colors-expected.txt
Committed r47070