The default bug view has changed. See this FAQ.

imgITools fails to decodeImageData for GIF images

RESOLVED FIXED in mozilla10

Status

()

Core
ImageLib
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: nossralf, Assigned: nossralf)

Tracking

Trunk
mozilla10
x86
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

9 years ago
When trying to use the image tools to decode a GIF image, the decodeImageData function throws NS_ERROR_NOT_IMPLEMENTED.

The reason is imgTools::DecodeImageData always [1] calls the Flush method for the image decoder, which in turn isn't implemented by nsGIFDecoder2 [2]. All other image decoders return NS_OK for Flush, even if it's a no-op.

The last patch for bug 399925 changed the gif decoder's Flush method to return NS_OK, but it was backed out, so this problem still exists.

[1] http://mxr.mozilla.org/mozilla/source/modules/libpr0n/src/imgTools.cpp#238
[2] http://mxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp#177

I also notice there is no unit test to cover image/gif for imgITools, and I would gladly write one if the powers that be wish me to do so.

Comment 1

9 years ago
Writing a unit test for GIF sounds like a great idea to me.
(Assignee)

Comment 2

9 years ago
Created attachment 304581 [details]
gif test image, 32x32, 1809 bytes

A possible GIF test image: converted image2jpg32x32.png to GIF using Gimp, named it image4.gif.
(Assignee)

Comment 3

9 years ago
Created attachment 304583 [details] [diff] [review]
unit test for mime type image/gif

Tentative unit test addition for test_imgtools.js.

Updated

6 years ago
Attachment #304583 - Flags: review?(joe)
Comment on attachment 304583 [details] [diff] [review]
unit test for mime type image/gif

Review of attachment 304583 [details] [diff] [review]:
-----------------------------------------------------------------

Looks lovely. This patch might need some massaging to commit though - at least the gif (the other attachment) is going to have to be hg added.
Attachment #304583 - Flags: review?(joe) → review+
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → nossralf+bugzilla
Created attachment 567281 [details] [diff] [review]
Rebased to tip + with binary added
Attachment #304581 - Attachment is obsolete: true
Attachment #304583 - Attachment is obsolete: true
In my queue with a few other bits that are being sent to try first and then onto inbound :-)

https://tbpl.mozilla.org/?tree=Try&rev=488d4a5f274a
Keywords: checkin-needed
Failed xpcshell due to:
TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | FAILED in test #10 -- test decoding a GIF: ReferenceError: TESTDIR is not defined

This:
> imgFile = do_get_file(TESTDIR + imgName);
should presumably be:
> imgFile = do_get_file(imgName);
Created attachment 567287 [details] [diff] [review]
Rebased + binary + remove TESTDIR param
Attachment #567281 - Attachment is obsolete: true
https://tbpl.mozilla.org/?tree=Try&rev=c81b8943dad6
https://hg.mozilla.org/integration/mozilla-inbound/rev/2081d8fab27d
Status: NEW → ASSIGNED
Flags: in-testsuite+
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/2081d8fab27d
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.