373675 - Icon resize doesn't support alpha-transparency

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[Wladimir Palant]

Extension icons often use alpha-transparency. However, if I upload the Adblock Plus icon (http://www.mozdev.org/source/browse/~checkout~/adblockplus/src/chrome/skin/classic/adblockplus.png) it will be resized (32x32 to 32x32 :) and the result looks broken. Apparently the alpha-transparent parts have been changed to opaque using a black background. If I save the same image with a white background without any transparency and upload it everything is fine.

Comment 1

[Fred Wenzel [:wenzel]]

I attempted to fix this in r2608. "Attempted" means it does now respect alpha transparencies, but resized images still don't look entirely beautiful and smooth. That, however, looks more like a problem with the gd library rather than part of this bug.

I also avoided resizing images that already have the destination size. Thus we don't make the quality worse by resizing 32x32 to the same size, as you mentioned above.

I mark it fixed for now, please let me know if you disagree.

Comment 2

[Wladimir Palant]

Yes, seems to work fine (and didn't reopen bug 372808, comments are still stripped from the file).

Comment 3

[Nils Maier [:nmaier]]

Nope, doesn't work for me (downthemall).
We're using png24 with semi-transparency.

Icon:
http://bugs.code.downthemall.net/trac/browser/trunk/chrome/skin/common/icon.png?format=raw

Working solution (for that icon at least) is to call
> imagesavealpha($sourceImage, true);
after |imagecreatefromString)

Comment 4

[Nils Maier [:nmaier]]

Attachment: enabling alpha for sourceImage

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 260012 [details] [diff] [review]
enabling alpha for sourceImage

We wanna write a test for this too?  Just a gold-master or A/B thing would work fine, I suspect.

Comment 7

[Nils Maier [:nmaier]]

Alright, it got a review...
So now somebody will have to check this in and update the production servers accordingly ;)

Comment 8

[Justin Scott [:fligtar]]

I was waiting for comment #6 before checking in.

Comment 9

[Nils Maier [:nmaier]]

Attachment: Same as above, but against current revision

Comment 10

[Reed Loden [:reed]]

Please address comment #6.

Comment 11

[Nils Maier [:nmaier]]

Attachment: Same as above, this time with a testcase

Ok, you're desperate for a test-case?
Here you are.
Doesn't include the test-data images. Will attach mine later.
Isn't that elegant.
And actually I don't know much about creating test cases for cake/AMO...
Actually it was quite a challenge to get the stuff up and running.

Comment 12

[Nils Maier [:nmaier]]

Attachment: test-data: base-icon.png

Comment 13

[Nils Maier [:nmaier]]

Attachment: test-data: resized-icon.png

Comment 14

[Justin Scott [:fligtar]]

Comment on attachment 304175 [details] [diff] [review]
Same as above, this time with a testcase

r=fligtar

Checked in r10620, along with a few other test fixes.

This is only on 3.2 branch, so it will be pushed live when that launches in March.

Thanks for the patch and testcase!