The default bug view has changed. See this FAQ.

App icons on windows 7/XP for native apps appear to be showing a black background

VERIFIED FIXED in Firefox 14

Status

Firefox Graveyard
Web Apps
--
minor
VERIFIED FIXED
5 years ago
a year ago

People

(Reporter: jsmith, Unassigned)

Tracking

unspecified
Firefox 14
x86_64
Windows 7
Bug Flags:
in-moztrap +

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Created attachment 601789 [details]
Black Background Icons Win 7

Steps:

1. Install the latest build of the extension
2. Go to apps.mozillalabs.com/appdir
3. Install an app natively
   a. Examples with bug: Favimon, BarFight

Expected:

The shortcut icon for the native app should have a transparent background.

Actual:

A black background is shown instead of a transparent background. See attached screenshot.
(Reporter)

Comment 1

5 years ago
Also: I've seen this issue on Windows XP.
Summary: App icons on windows 7 for native apps appear to be showing a black background → App icons on windows 7/XP for native apps appear to be showing a black background
(Reporter)

Updated

5 years ago
Component: Extension → Web Apps
Product: Web Apps → Firefox
QA Contact: extension → webapps
(Reporter)

Updated

5 years ago
Blocks: 731054
The way I read nsICOEncoder.cpp, there is no support for transparency there - the "and" image is hard-coded such that every pixel is opaque.  It appears bug 687062 is adding support for decoding icons, but that isn't what we need here.

I'm not that familiar with the imglib stuff, so I'm adding bbondy to the CC list as he seems to be in the know about such things and might be able to offer some insights.
sorry - to clarify, I meant "It appears bug 687062 is adding support *for transparency* when decoding icons,
So, I *think* this could be addressed by:

* Initializing the BMP encoder with 32 bytes per pixel, meaning the encoder will give us the alpha channel.

* In the ICO encoder we'd drop the memcpy of the image data and instead loop over the pixels, sticking the rgb data in the "real" buffer and the alpha channel in the "and" buffer (although IIUC, the "and" buffer is a simple byte-mask whereas the alpha data is a byte value, so I guess we'd check for the alpha channel being < 128 when setting the "and" buffer)

* Adjust uses of BMPImageBufferSize to account for this (ie, BMPImageBufferSize would then be bigger due to the alpha bytes)

Does that sound sane?
> * Initializing the BMP encoder with 32 bytes per pixel, meaning the encoder will give us the alpha channel.

Yes you should use 32 bytes per pixel, we don't support encoding of alpha masks.

> * In the ICO encoder we'd drop the memcpy of the image data and 
> instead loop over the pixels, sticking the rgb data in the "real" 
> buffer and the alpha channel in the "and" buffer (although IIUC, the 
> "and" buffer is a simple byte-mask whereas the alpha data is a byte value, 
> so I guess we'd check for the alpha channel being < 128 when setting the "and" buffer)

You shouldn't need to deal this low level, you should have an image container and then encode it as an icon using the icon mime type (image/vnd.microsoft.icon) and its encoder options (format=bmp;bpp=32).  TimA I think has already did some work here you might want to ping him as well.

> mMimeTypeOfInputData.AssignLiteral("image/vnd.microsoft.icon"); 
> rv = imgtool->EncodeImage(container, mMimeTypeOfInputData, 
>     NS_LITERAL_STRING("format=bmp;bpp=32"), getter_AddRefs(iconStream));

You can find a more complete example in this patch or in the jump list code: 
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=110894&attachment=611183

> * Adjust uses of BMPImageBufferSize to account for this (ie,
> BMPImageBufferSize would then be bigger due to the alpha bytes)

ditto above, you shouldn't need to worry about these details.
(In reply to Brian R. Bondy [:bbondy] from comment #5)

Awesome, thanks!  The problem is indeed solved simply by specifying bpp=32 (we previously only specified the format but not the bpp=).  I guess Windows ignores the AND mask when a true 32bit bitmap is found.

Once WebappsInstaller.jsm is checked in, we just need to change:

      iconStream = imgTools.encodeImage(imgContainer.value,
                                        "image/vnd.microsoft.icon",
                                        "format=bmp");

to:

      iconStream = imgTools.encodeImage(imgContainer.value,
                                        "image/vnd.microsoft.icon",
                                        "format=bmp;bpp=32");

and this bug goes away!
Thanks a lot Mark for the thorough investigation. I'll incorporate this change in the patch over bug 731541.
Fixed in bug 731541
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
(Reporter)

Updated

5 years ago
Status: RESOLVED → VERIFIED
(Reporter)

Updated

5 years ago
No longer blocks: 731054
(Reporter)

Updated

5 years ago
Flags: in-moztrap?(jsmith)
(Reporter)

Comment 9

5 years ago
MozTrap +. Already covered by windows shortcut MozTrap test case.
Flags: in-moztrap?(jsmith) → in-moztrap+
(Reporter)

Updated

5 years ago
QA Contact: jsmith
(Assignee)

Updated

a year ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.