Bug 28025 - Add .ico decoding LayoutTest
Summary: Add .ico decoding LayoutTest
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Peter Kasting
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-05 13:13 PDT by Peter Kasting
Modified: 2009-08-06 17:26 PDT (History)
1 user (show)

See Also:


Attachments
.ico LayoutTest (30.52 KB, patch)
2009-08-05 13:20 PDT, Peter Kasting
eric: review+
pkasting: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.