Bug 28025

Summary: Add .ico decoding LayoutTest
Product: WebKit Reporter: Peter Kasting <pkasting>
Component: ImagesAssignee: Peter Kasting <pkasting>
Status: RESOLVED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
.ico LayoutTest eric: review+, pkasting: commit-queue+

Description Peter Kasting 2009-08-05 13:13:48 PDT
To my knowledge there are no tests of .ico decoding.  I'll add one that tests a few different types of decoding (1-bit, 8-bit, 32-bit, and multiple directory entries).
Comment 1 Peter Kasting 2009-08-05 13:20:51 PDT
Created attachment 34167 [details]
.ico LayoutTest

Here's the test.  Note that the CG .ico decoder has a bug in how it decodes 1-bit images; I've filed this on Apple's bug tracker and commented on it in the test, so if the bug gets fixed it will hopefully be obvious that the expected PNG should change.

There are still other types of decoding that could be tested (PNG-in-.ico, negative tests of broken icons, a BMP decoder test that checks RLE-encoded data), but this is at least better than nothing.
Comment 2 Darin Adler 2009-08-05 13:26:42 PDT
(In reply to comment #1)
> I've filed this on Apple's bug tracker and commented on it in the
> test, so if the bug gets fixed it will hopefully be obvious that the expected
> PNG should change.

Would you be willing to share the Apple bug number?
Comment 3 Peter Kasting 2009-08-05 13:37:08 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > I've filed this on Apple's bug tracker
> 
> Would you be willing to share the Apple bug number?

Sure, it's ID 7114290.

I don't think it's very important to fix -- probably few if any icons in the wild are 1-bit.
Comment 4 Eric Seidel (no email) 2009-08-06 16:10:17 PDT
Comment on attachment 34167 [details]
.ico LayoutTest

LGTM.  Seems we should file a bug about the CG decoder.  http://radar.apple.com/ is probably the right place for that, even though I hate that site. :(
Comment 5 Peter Kasting 2009-08-06 16:11:59 PDT
(In reply to comment #4)
> Seems we should file a bug about the CG decoder. 
> http://radar.apple.com/ is probably the right place for that, even though I
> hate that site. :(

I am confused.  I already said above that I'd filed a bug for this.
Comment 6 Peter Kasting 2009-08-06 17:26:07 PDT
Fixed in r46871.