Don't convert an icon when it's unnecessary

VERIFIED FIXED

Status

Mozilla Labs
Prism
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: nossralf, Assigned: nossralf)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Created attachment 307110 [details] [diff] [review]
createNativeIcon only if necessary

The current behavior of install-shortcut.js is to always create a native icon by converting the format using image-tools, even if the supplied icon has a mime type that matches the native one. Since getIcon() sets up a proper stream for all cases (favicon found, app bundle has icon, use default icon when first two fail), skipping the conversion when mime types match is easy.

(As a side-effect, this fixes a really weird crasher for me, whereby Prism would crash if the default icon was used, due to some oddity with closing the output stream in ImageUtils.createNativeIcon. There are no guarantees that this will never happen even if this suggested fix is applied, but I still think the logic of only converting when there's a mime type mismatch motivates making this slight change.)

[Note: I use hg diff which means apply with "patch -p1".]
(Assignee)

Updated

9 years ago
Assignee: nobody → nossralf
(Assignee)

Updated

9 years ago
Attachment #307110 - Flags: review?(mark.finkle)
Attachment #307110 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 1

9 years ago
I just noticed the line |var storageStream = ImageUtils.createStorageStream();| should probably be moved inside the if-clause as well.
Yes, that makes sense. Done.

However, it won't hurt 0.9 to not have this part fixed.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.