1267224 - Latest version (45.0.2) no longer supports BMP 24bpp images with BITFIELDS compression and 16 bpp

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[stuart.taylor]

Attachment: ScreenShot00000(6) - Copy.bmp

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160407164938

Steps to reproduce:

Open Firefox (version 45.0.2)
Drag attached file onto Firefox window


Actual results:

The following text is displayed:
The image “file:///D:/Temp/BrowserImageIssueScreenShot00000(6)%20-%20Copy.bmp” cannot be displayed because it contains errors.

I have a screenshot but can only upload one file.


Expected results:

Image loaded in version 33.1

I have a screenshot but can only upload one file.

Comment 1

[stuart.taylor]

I have just viewed the uploaded attachment and it renders in Firefox version 45.0.2. If you cannot reproduce the issue it might be because the file upload has modified the file? I have the original which does still fail.

Comment 2

[Liviu Cirdei]

Does it still reproduce if you use a clean profile (https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles) and Firefox Safe Mode (https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode)?

If yes, could you, please, attach a screen record showing the issue?

Comment 3

[Loic]

We need the original BMP.

Comment 4

[Masatoshi Kimura [:emk]]

Bugzilla will convert uncompressed bmp to png automatically. You will have to compress the bmp to a zip and attach the zip file.

Comment 5

[stuart.taylor]

Attachment: ScreenShot00000(6) - Copy.zip

I've attached a zip containing the bmp file

Comment 6

[stuart.taylor]

(In reply to Liviu Cirdei [:liviucirdei] from comment #2)
> Does it still reproduce if you use a clean profile
> (https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-
> firefox-profiles) and Firefox Safe Mode
> (https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-
> mode)?
> 
> If yes, could you, please, attach a screen record showing the issue?

Yes, I can still reproduce the issue with a clean profile and in safe mode. My company doesn't have any approved screen recording software so unable to provide recording. The steps are simple enought:
- Open firefox
- Drag image onto tab
I have provided a new zip file containing the original bmp. Hopefully you will be able to reproduce the issue with this file.

Comment 7

[Liviu Cirdei]

User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID 	20160510030240

Thanks for the info. I was able to reproduce the issue on Firefox 45.0.2 and latest Nightly (49.0a1).

I also found a regression range for this:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=92407172ab7ce2cdcbbd8e8d54fcc22c90abcb17&tochange=1c9516a8f8f5cc996e8d26b88769ed4c72923e32

Suspect: Bug 1204394 (part 2) - commit message: Add bmpsuite to the BMP reftests. r=seth.

Comment 8

[Masatoshi Kimura [:emk]]

By the way, Chrome did not open the attached .bmp either. IE and Edge could view the image.

Comment 9

[Timothy Nikkel (:tnikkel)]

We reject the file because it uses compression type BITFIELDS, and has BPP of 24. We only accept BITFIELDS with a BPP of 32 or 16.

The old code apparently handled this case. In the old code we had the same BPP check (only 16 and 32 accepted) for ALPHABITFIELDS type compression (but not BITFIELDS). We removed support for ALPHABITFIELDS in bug 1204394 as well.

The new check seems to be based on this page
http://www.fileformat.info/format/bmp/egff.htm
Although the language isn't 100% clear it seems to be saying that BITFIELDS is only valid with 16 or 32 BPP.

https://msdn.microsoft.com/en-us/library/aa452885.aspx says that for ALPHABITFIELDS only 16 and 32 BPP is valid.

So perhaps http://www.fileformat.info/format/bmp/egff.htm confused BITFIELDS and ALPHABITFIELDS, and BITFIELDS and BPP 24 is valid. We have at least one example of such a file in the wild.

Comment 10

[Nicholas Nethercote [inactive]]

(In reply to Timothy Nikkel (:tnikkel) from comment #9)
> 
> The new check seems to be based on this page
> http://www.fileformat.info/format/bmp/egff.htm
> Although the language isn't 100% clear it seems to be saying that BITFIELDS
> is only valid with 16 or 32 BPP.
> 
> https://msdn.microsoft.com/en-us/library/aa452885.aspx says that for
> ALPHABITFIELDS only 16 and 32 BPP is valid.
> 
> So perhaps http://www.fileformat.info/format/bmp/egff.htm confused BITFIELDS
> and ALPHABITFIELDS, and BITFIELDS and BPP 24 is valid. We have at least one
> example of such a file in the wild.

Arguments against supporting BITFIELDS+24BPP:

- As you say, http://www.fileformat.info/format/bmp/egff.htm isn't totally clear but sounds like BITFIELDS+24BPP isn't allowed.

- https://en.wikipedia.org/wiki/File:AllBMPformats.png from https://en.wikipedia.org/wiki/BMP_file_format clearly states that using BITFIELDS in combination with a BPP of 24 is illegal.

- BMPSuite has examples of BITFIELDS combined with 16BPP and 32BPP, but not 24BPP, and it's the most comprehensive source of weird BMP files I know.

- Chrome doesn't handle BITFIELDS+24BPP.

To counter that:

- IE and Edge handle BITFIELDS+24BPP.

- We have one example file that uses it.

It would be possible to support this. I don't have enough experience with imagelib to have a good sense of how lenient we are with accepting borderline cases like this.

Comment 11

[Marcia Knous [:marcia]]

ni on Timothy and Seth to find out what we want to do regarding this bug since we still have to make a call for 48/49 tracking. 

Won't fix for 46 and 47.

Comment 12

[Timothy Nikkel (:tnikkel)]

Since Chrome doesn't support these types of BMP I don't think the priority is high to support them ourselves.

Comment 13

[Marcia Knous [:marcia]]

Based on Comment 12, for now we won't track this for 48/49. Please feel free to re-nominate this issue if we think the priority changes.

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Part 1. Add support for 24bpp bitfield compressed BMPs, v1

I added support for 24bpp bitfield compressed images, not a lot of code required to support it. As far as the standard goes, it claims:

> If the bitmap contains 16 or 32 bits per pixel, then only a Compression value of 3 is supported

It doesn't say 24bpp cannot use a compression value of 3, only that 16bpp and 32bpp must use it. Very unclear language :).

However prior to bug 1204394, while we rendered attachment 8751136 [details], it was not because of 24bpp bitfield compression support. In attachment 8744870 [details], you can see the image as it should be rendered, and how it is rendered on Windows. If you compare this to an earlier version of Firefox, you will see a white band on the right hand side of the image instead of black.

This is because the image header is wrong -- it indicates bitfield compression, but it does not actually provide the bitmasks in the header. It should be uncompressed. The data offset corroborates this because it claims the image data starts where the bitmasks would be, the data located there is all zeroes, and not a bitmask at all.

Thus it appears Internet Explorer and other Windows apps (e.g. photo viewer) adjust for the mistaken header and ignore the bitfields compression. If I modify the header to add the bitmask, it also renders in IE (but not Chrome). I've included that kludge in my patch, but if we don't like doing that, then please r- so we can close the bug as WONTFIX :). Otherwise, I will craft some test bmps and create test cases.

Comment 15

[Nicholas Nethercote [inactive]]

Comment on attachment 8773266 [details] [diff] [review]
Part 1. Add support for 24bpp bitfield compressed BMPs, v1

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

I appreciate the effort you've gone to getting this image to work. But I'm not comfortable with the generosity of interpretation required to get it to work. BMP is a nasty enough format as it is, and IMO this change falls on the wrong side of the complexity vs. benefit dichotomy, especially given that Chrome doesn't read this.

Apologies for not being more definitive earlier.