Closed Bug 18502 Opened 25 years ago Closed 23 years ago

[RFE] Ability to display Windows .BMP and .ICO images in Mozilla

Categories

(Core :: Graphics: ImageLib, enhancement, P5)

Product:
Core
Component:
Graphics: ImageLib
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.6

People

(Reporter: cpratt, Assigned: hyatt)

References

Details

(Keywords: testcase, Whiteboard: [imglib])

Attachments

(3 files, 10 obsolete files)

Bitmap image testcase
(deleted), image/bmp
Details
Inline bitmap image display testcase (dependent on previous att)
(deleted), text/html
Details
Hopefully final patch
(deleted), patch
pavlov
: review+
jband_mozilla
: superreview+
Details | Diff | Splinter Review 
Build ID: 19991110911 (M11)
Platform: Windows NT

To reproduce:
- Launch the browser
- Load the above URL
- Scroll down to test 2.1.7 - Bitmap (BMP)

Result: There is no image there.

Expected result: You should see the image.

This works correctly in IE but not in Nav 4.x.
Severity: normal → enhancement
Summary: Bitmap images not displayed in Mozilla → [Enh] .BMP images not displayed in Mozilla
To the best of my knowledge, this bug is invalid.

We have never supported, and have no plans to natively support, the Microsoft
Windows-specific .BMP format.

Current supported file formats are PNG, JPEG, GIF and (sortof) XBM.
[changing to enhancement request]
Priority: P3 → P5
Agreed that this is an enhancement request. The DTD does not specify *which*
file formats to support ("widely recognized image formats include GIF, JPEG, and
PNG"); this bug is only to point out that IE (on Windows, not Mac OS) supports
.BMP files, but no version of Communicator does.
Status: NEW → ASSIGNED
Target Milestone: M20
Chris:

I'd entended to use making a bmp decoder component the
example for a cookbook of how to add decoder components.
Its about the easiest format around, and the image decoder
problems would be trivial compared to creating the decoder
as a component.
So for sure, later.
-p
Summary: [Enh] .BMP images not displayed in Mozilla → .BMP images not displayed in Mozilla
Note also related bug 27729, in which attempting to open a .bmp image crashes 
Seamonkey.
Target Milestone: M20 → Future
pnunn, have you written anything that would add .bmp support? I don't see any
reason why we shouldn't add it.

adding myself to cc list.

jce2:
oooooo, a volunteer!
BMP is the simplest of the simple formats to implement.

My schedule is filled right now with reload policy issues
and imgcache changes.

Enjoy.
-P
*** Bug 56436 has been marked as a duplicate of this bug. ***
Marking [IBENCH] to ease searching. (Shows up in i-bench test suite.)
Summary: .BMP images not displayed in Mozilla → [IBENCH] .BMP images not displayed in Mozilla
Blocks: 61481
QA Contact: elig → tpreston
I agree that .bmp file support would be pretty cool and pretty easy. Since 
99.9% of Netscape users are windows users, this would also be pretty smart. Not 
having bmp support has bothered me many times, and if IE has it, it probably is 
not good that Netscape doesn't. You, of course, can't use windows specific APIs 
to do so, but writing a full decoders for BMPs are a piece of cake. I have done 
it a million times in my sleep. BTW: Don't forget bmp RLE support.
Reassigning to Brian since he clearly has the experience to do this, wants it
implemented, and the current assignee has no intention of spending resources on 
this enhancement request.

pnunn: I hope that's ok with you.
Assignee: pnunn → boberb
Status: ASSIGNED → NEW
That's fine with me. Changing to M23 for now. I have never made an image decoder
for Mozilla before. Perhaps someone can point me to something that explains how.
Target Milestone: Future → M23
Status: NEW → ASSIGNED
reassign away! I've got a few other bugs you
could take off my hands....... ;>
-p
Target Milestone: M23 → mozilla1.1
Since the M19+ milestones will be removed, changed to mozilla1.1. Reporter:
Since the .BMP format is something that isn't a web standard, and displaying
such images is an enhancement request, I am changing the Summary to reflect
that. The old summary was the following:

"[IBENCH] .BMP images not displayed in Mozilla"

I hope this is to your liking. Feel free to change it back.

I am not planning to implement this immediately, but rather in a few months,
probably. If someone wants to tackle this before that time, then let me know. I
hope this takes some of the weight of pnunn's shoulders.
Summary: [IBENCH] .BMP images not displayed in Mozilla → [RFE] Ability to display Windows .BMP images in Mozilla
Blocks: 66962
Blocks: 32087
Whiteboard: [imglib]
didnt mscott just recently implement the ability for mozilla to view windows 
system icons? Could this be related?  something like a moz-icon: protocol....
Attached image Bitmap image testcase (deleted) — 

    
    

    
  
I hope this feature makes it in sometime. Even though people aren't supposed to
upload files like *.bmp and *.art to the web, many still do, and it would be a
shame if I had to close Netscape and start Internet Explorer in order to see the
picture one of these problematic web pages is describing.

    
    

    
  
Its good you added some testcases. Unfortunately, if Mozilla displays these 
bitmaps it will only be a start. If you look at the spec bitmaps, there are RLE 
encoded bitmaps, 24bit, 8bit, 4bit, monochrome, cursor files, icons, there are 
bitmaps that use the BitmapInfoHeader structure and ones that use the 
BitmapCoreHeader structure. Anyone who wants to develop a test suite of images 
for every bitmap possibility at multiple sizes for each case would be a great 
help.

If anyone wants to implement this, feel free as I haven't started it yet.

Changing os to all since Mozilla would display bitmaps on all oses if it was 
implemented.
Keywords: helpwanted
OS: Windows NT → All
Hardware: PC → All

    
    

    
  
Also notice: Although bitmaps are a bad format to use for the web, there are 
times when bitmaps come in handy. For instance, if someone wanted to put a 
bitmap on the internet so that none of the quality was lost in the image. 

There are also those bimbos who put bitmaps on web pages for whatever reason. 

Another case is if Mozilla was used to browse the hard drive as a file explorer 
(http://filezilla.mozdev.org/). For windows systems and linux systems, it would 
be nice if people could view bitmaps on their systems.

Bitmap is not a severely complicated and I don't see why it would hurt for 
Mozilla to support bitmaps.

The only thing I am worried about is putting bitmap support in might encourage 
people to use bitmaps in their web pages. But if they do, then what the heck - 
that is their problem.

Usually, when I see a bitmap in a webpage I send a letter to the webmaster 
telling him he should know better. As long as everyone does this, I don't see 
bitmaps ever taking over the web until the day when compression is not 
necessary - i.e. never.

    
    

    
  
"For instance, if someone wanted to put a bitmap on the internet so that none of
the quality was lost in the image."

24-bit PNG can accomplish this with much better compression. But sometimes
people like to share their Windows wallpaper...

BTW, just a thought... in IE, AOL ART image support is downloadable as a
separate component, so people who don't want it that badly don't have to
download it. Since the common accusation is that Netscape 6 is bloated, maybe
something similar to this should be considered for BMP image support.



    
    

    
  
On Windows, one could probably use the Win32 API to do this pretty easily. I'd
take a look, but alas too many bugs to fix. :(

But...one can probably take the sample plugin at /mozilla/modules/plugin/test
and fix it up so it works for BMP mime types pretty easily. If Mozilla ever gets
a plugin download area like Netscape has, it could also put the plugin there for
almost seemless download when a user a page that needs it.

    
    

    
  
Hmmmm.... I wonder if there are already Bitmap plugins for Netscape browsers. 
There is one thing, though. If we made it optional, then people would probably 
not know they can add it.

    
    

    
  
Peter: there is a function in the Win32 API called LoadBitmap or something. 
Windows does all the work for you. But, back in the days when I did a lot of 
Windows-specific programming (back in win3.11), there wansn't an API to load 
bitmaps. You had to do all the work yourself to convert the image from a DIB to 
a DDB (device dependant bitmap). I am currently working on Autoscroll/Panning 
bug 22775, and after that, I will write OS-independant non-plugin code for the 
image library. If someone has experience with image code but doesn't know how 
to write bitmap code and wants to help, just email me and I will explain how 
bitmaps work.

    
    

    
  
"If we made it optional, then people would probably not know they can add it."

In IE, I think you can do a full, recommended, minimal, or custom install, and
everything except minimal automatically has the ART image support (of course,
with IE the bitmap support is built in). This way users will get the ART support
unless they specifically don't want it. This is the same category of users who
think Netscape 6 is bloated, and the people who don't care will do a full or
recommended install and get the bitmap plugin. After all, Netscape 6 already
includes a lot of other stuff most people don't want, such as RealPlayer and AOL
Messenger...


    
    

    
  
*** Bug 85356 has been marked as a duplicate of this bug. ***

    
    

    
  
I would like to make a case against adding internal BMP support. Here are some 
of my reasons:

1. Adds no functionality. Can not do anything that PNG cannot do.

2. Is not necessary for compatibility. Is not a W3C Recommendataion. No serious 
web authors intentionally use it, or wish to use it.

3. Large file size makes it bad for the web. No compression at all except for 
4bpp and 8bpp varieties, and then only a rather poor RLE compression.

4. MSIE already supports it, so if Mozilla does too, it will become another de 
facto standard image format, and soon every web browser may need to support it 
in perpetuity. That isn't a decision that should be made lightly, even if it may 
seem to benefit Mozilla in the short run.

5. BMP is not as simple as it may seem, if you want to support all the flavors 
of it that are in use.

6. AFAIK, there is no official specification for the BMP format.

7. BMP may not be stable. Microsoft has defined new-fangled V4 and V5 bitmaps, 
which can be quite complex. It's not clear if they are supposed to be allowed in 
BMP files (see previous), but it seems plausible that some day they will be 
used.
----

I have written a BMP plugin for Windows web browsers: 
http://entropymine.com/jason/webbmp/

Like all plugins, it canNOT be used to add full support for that image type. The 
reason is very simple: no browser that I know of will ever invoke a plugin for 
an <img> element -- the web page author must use <embed>. (Well, theoretically 
one could use <object> for all images, but that's not practical at this time due 
to generally dismal browser support.)

I DO think it ought to be possible for the user to add full support for new 
image formats using plugins. (Ideally, *all* image formats should be implemented 
with plugins.) Efforts should be made in this direction, rather than adding 
internal BMP support.

I probably shouldn't mention that I also have a set of BMP test images: 
http://entropymine.com/jason/bmpsuite/

    
    

    
  
Of course BMP files aren't good for the web. If only I could get every idiot who
posts these blasted things all over the internet to understand that, we wouldn't
need support for this. However, as long as there are BMP files out there, and as
long as IE supports them, there's a strong argument for including at least basic
support for BMP images in Mozilla.

    
    

    
  
having .bmp  support has other effects too right? like the favicon stuff.  

    
    

    
  
If you're talking about what I think you're talking about, favicon.ico files are
a special kind of bitmap formatted for use as an icon. And I sincerely hope
there are no plans to support favicon.ico in Mozilla.

    
    

    
  
There is one other problem that you guys haven't thought of (which I mentioned 
earlier). It is that in order for Mozilla to become a viable File Explorer for 
Windows, it needs bitmap support. 

Secondly, .bmp support might not be good for external use (i.e. pages that 
others will be viewing), but for internal use - i.e. your own page that only 
you go to on your LAN - a bitmap is a viable option. I have had many times that 
I have needed this. Bitmaps are the image format of choice for Windows Users 
for internal use if they don't care about file size and/or if they don't know 
how to make any other image format. See http://meow.mozdev.org/.

Thirdly, there are standards for .bmp and they are in the Windows API 
Documentation. Support is only needed for bitmaps that are loaded from files 
(Known as file bitmaps), not memory bitmaps. V4 and V5 are structures used to 
represent bitmaps with added support for gifs, jpegs, and pngs in memory. I 
doubt that Windows will ever change the bitmap format to cater to these forms 
which don't work on every system. Nor do we have to worry about it for now.

Yes, there is a bug for favicon implemtation. It is bug 32087 and is assigned 
to me. Although .ico files aren't true bitmaps, they are very similiar to .bmp 
files, and can be done in the same implementation. Now, we should try to push 
people away from using icon files for their favorite icons and to use gifs, 
pngs, or jpegs instead. Therefore, we will be the ones with full favicon 
support and IE won't. Also, favicons won't have to be named favicon.ico and 
won't be searched for on the server. They will have to be linked in. Please see 
the bug.

As for the web getting covered with bitmaps, I don't believe that will ever be 
a problem. People know better than to use bitmaps, and bitmaps aren't defined 
in the standard, so people are ill-advised to use bitmaps in their web pages. 
Whenever I come across a commercial site containing a bitmap, I email the 
author telling them that they don't have the proper image format on their site. 
I assume that many other people do the same. Therefore, I don't think you have 
anything to worry about.

As for people with home pages, they have the right to use any image format they 
please, and they shouldn't have to use Internet Explorer to view the pages. 
Where do you think all the newbie web designers are going to go.

Implementing this will be more of a convenience to Windows user than anything 
else. I don't see any reason not to support bitmaps as not supporting bitmaps 
is something Netscape gets bashed about all the time from people who don't know 
better.

    
    

    
  
The Windows SDK is rather ambiguous about V4/V5 support. But the link from 
Jason's site (http://support.microsoft.com/support/kb/articles/q216/2/05.asp) 
shows that you aren't supposed to save in V4 and V5 format and instead are 
supposed to load pngs, jpegs, and gifs, (as stated in the SDK) into the V4/V5 
structures. Therefore, I do not feel there is any need for V4/V5 support.

Jason, although you don't agree about .bmp support in Mozilla, thank you for 
the test-suite. Can we count on your site being up indefinately?

I might extend your bitmap creation code when I start to implement this.

I would also like to say that I believe if Internet Explorer for the Mac 
doesn't support bitmaps and Mozilla does eventually, that would provied for 
quite a bit of irony. Can anyone verify whether Mac IE supports bitmap? 

    
    

    
  
Mac IE relies on QuickTime for much of its image support, IIRC. I believe - 
again, if I remember correctly - that QuickTime supports Windows bitmap files as 
of version 3.0, so Mac IE should in theory be able to deal with the format.

    
    

    
  
[Concerning my web pages about BMP things] I expect to keep them available 
indefinitely, assuming I don't get hit by a bus or something.

[IE requires Quicktime] is mostly a myth. The problem is that IE is overly 
willing to allow its own internal support for various formats to be overridden 
by other program such as Quicktime. (Except when using the <img> element, of 
course.) IE for Windows is quite capable of displaying BMP images without using 
Quicktime.

["People know better than to use bitmaps" on the web.]  Nonsense. Most people do 
not even understand the concept of file formats at all, let alone the difference 
between various *image* file formats. It's not uncommon to find BMP files on web 
sites renamed to end in .gif -- and then the owner asks why his "GIF files" 
don't work in Netscape. Fortunately, *because* they don't work in Netscape, he 
discovers his error and fixes it.

    
    

    
  
Win32 and UNIX IE of course have nothing to do with QuickTime. The Mac OS 
version of IE does, however, rely on it at least in part for image rendering. 
I've since confirmed with the QA lead on Mac IE that they do support BMP files, 
at least in 5.0 and later.

    
    

    
  
*** Bug 91777 has been marked as a duplicate of this bug. ***

    
    

    
  
Reading through all the comments was surprisingly useful.  
if i get the Quicktime  plugin will get some bmp support for Mozilla in windows.  
That's roughly half my problem solved.  

The reason people put Bitmaps on web pages is because the dont know any better 
and neither Frontpage nor Internet Explorer give them any warning that they 
shouldn't.  It does not convert them to another format (even an uncompressed gif 
would be better) like the way Netscape Composer does/did.  


    
    

    
  
What is going on with this bug?  There has been no activity.

I think that not supporting a simple format like bmp is a big problem.  Why
don't we change the "Target Milestone" to be Mozilla 1.0?  I don't think this
would be a huge burden.  Right now it is set to Mozilla 1.1 and I think that
this is a big problem especially with the penetration of favicons.

    
    

    
  
burton: Per comments, I will add a mozilla1.0 request. Note that favicons are
probably a completely separate discussion from this.
Keywords: mozilla1.0

    
    

    
  
Removing URL since it is a dead link. There is a simplified testcase for this 
bug.
URL: http://i-bench.zdnet.com/ibench/html4...
Keywords: testcase

    
    

    
  
If I read these comments correctly,  Brian would like to implement it,  but
doesn't have time for it at the moment.

In this case, I'd like to implement this feature.
I will probably not be able to do RLE compression, though.

    
    

    
  
OK, I've started work on this now, and some BMPs already display.

This patch contains the changes to the existing files.
It will probably not change.

I will attach a tarball of the added files, unpack it in the mozilla/ directory.
Please note that these files are work-in-progress, they do not work for all
uncompressed images, let alone compressed ones.
But some are displayed fine :)

Oh, and NeTDeMoN, feel free to assign this bug on me, as I'm currently working
on it.

    
  
Keywords: patch, 
          
            review

    
    

    
  
> This patch contains the changes to the existing files.
> It will probably not change.

You are *THE MAN* !

> I will attach a tarball of the added files, unpack it in the 
> mozilla/  directory.  Please note that these files are work-in-progress, 
> they do not work for all uncompressed images, let alone compressed ones.
> But some are displayed fine :)

Was this patch against HEAD from CVS?  How should I apply the patch.

I will try testing it out with my application.  It pulls in bitmaps from a ton
of places.  The least I can do is give feedback.
  

    
    

    
  
You can apply the patch by entering "patch -p0 < filename_of_patchfile" in the
Mozilla Source Directory.

I haven't tested this under Linux yet, so some feedback how it works there is nice.

    
    

    
  
And about the tarball:
Create a directory moduled/libpr0n/decoders/bmp and untar the tarball there (tar
xzf filename.tar.gz)

    
    

    
  
When you get around to handling them, compressed bitmaps usually come in two
flavours, BI_RLE8 for 8-bit bitmaps and BI_RLE4 for 4-bit bitmaps. However there
also exists an undocumented bitmap format which is BI_RLE4 encoded but only uses
a two colour table. To indicate this the bit count is 1 instead of 4. I would
appreciate it if you could support this format as the extra work seems slight.

    
    

    
  
<biesi> Hm... W/o RLE, I'm probably finished by 0.9.5. With, hm, 0.9.6 is more 
likely

cool
Assignee: netdemonz → cbiesinger
Status: ASSIGNED → NEW
Keywords: mozilla0.9.5
Target Milestone: mozilla1.1 → mozilla0.9.6
Status: NEW → ASSIGNED
Keywords: mozilla0.9.5

    
    

    
  
OK, this version should display almost all uncompressed BMPs.
Known problems:
1) The first line is not decoded, a grey line appears instead (I don't know yet why)
2) Doesn't display OS/2 Bitmaps
3) It was only tested under Windows

To use it, apply "Patch: Part 1", then untar this attachment in the mozilla/
directory.

    
    

    
  
I don't think this is ready for review at the moment.
Keywords: review

    
    

    
  
If you want this to work for larger files, change line 243 in
modules/libpr0n/decoders/bmp/nsBMPDecoder.cpp from
	if (mPos == mBFH.dataoffset) {
to
	if (mPos >= mBFH.dataoffset) {



    
    

    
  
I must say that i'm opposed teaching moz to handle BMP. BMP is a really 
horrible format so IMHO we should not encourage people to use it.

I don't think the argument "not many people uses moz anyway" really holds 
because 1) if nobody will use moz then why are we developing it? and 2) lots of 
people are using moz if you count all browsers derived from the moz codebase.

The fact that only IE support BMP has kept it off the web, however if both NS 
and IE supports it there is a really big risk that developers will start using 
it more often.

If we contact authors and say "you shouldn't use BMP becuase it dosn't work in 
mozilla browsers such as NS" they are far more likely to listen then if we 
say "you shouldn't use BMP becuase it's a BadThing, but it will work in most 
browsers"...

    
    

    
  
I disagree with you.
1. It's the webmaster's decision what he/she puts on his homepage.
If this is a BMP, well, then so be it.

You could argue that GIF is a bad format, because of this Unisys patent they're
now enforcing. However, you don't suggest removing GIF support.
Why is it a bad format? Because it's lossless? Because there's no patent
whatsoever on it? Or because there's not really a compression, besides RLE?

You seem to think that many people would put BMPs on the web, if just Netscape
would support them. I seriously doubt that.

There is a wish that BMPs are supported. As a Netscape employee reported this
bug, apparently this company wants it.

Your reason for not wanting to include this is just: "This makes people put more
 BMPs on the web", correct?

BTW, this tarball is the latest and greatest version of the patch, fixes some
problems. The first line is not just a grey line anymore, but the correct
content. It works for every image in the BMP Testsuite by Jason Summers, except
the compressed ones, the 16bit and 32 bit ones, and the OS/2 one

    
    

    
  
> The fact that only IE support BMP has kept it off the web, however if both NS 
> and IE supports it there is a really big risk that developers will start using 
> it more often.

No where in the HTML specification does it say "don't support BMP" "these are
the supported image formats".  BMP is a very popular format and people are using
it.  Therefore we *have* to support it.  

Of course if we want to take your non-pragmatic approach, we should also drop
GIF support because of the UNISYS patent.  I don't see that happening anytime soon. 

When a user double clicks on a BMP file and Mozilla can't handle it - it makes
Moz look bad.  Especially when IE can handle it just fine.

    
    

    
  
> Why is it a bad format? Because it's lossless? Because there's no patent
> whatsoever on it? Or because there's not really a compression, besides RLE?

Because it has lousy compression.

> > The fact that only IE support BMP has kept it off the web, however if both
> > NS and IE supports it there is a really big risk that developers will start
> > using it more often.

> No where in the HTML specification does it say "don't support BMP" "these are
> the supported image formats".  BMP is a very popular format and people are
> using it.  Therefore we *have* to support it.  

BMP is *not* a "popular format" other then on windows, and even there I 
wouldn't call it popular among people dealing with graphics. We don't *have* to 
support BMP, we've done just fine without it for several years.

> Of course if we want to take your non-pragmatic approach, we should also drop
> GIF support because of the UNISYS patent.  I don't see that happening anytime
> soon.

However GIF is a format we *have* to support since there is so many sites 
depending on it, if there wasn't I don't think we should have added support for 
it no.

IMHO there are the exact same arguments for adding support for the IE extension 
document.all, desition was taken some time ago that we should *not* support 
that but rather go for other better alternatives. And in that case there were 
much stronger arguments for support then there is for supporting BMP, more 
sites depending on it and no other cross-browser alternatives.

So, should we add support for that too?

Remember that once support for something is added in mozilla we are pretty much 
stuck with it for all eternity

    
    

    
  
End users will be disappointed if an image displays in MSIE and doesn't display
in Mozilla. I personally think it's more important to prevent situations where
pages don't work than to worry about being the superhero that squashed BMP
images on the web. I don't expect any commercial or otherwise highly public
website to start using BMP just because two browsers support it (in fact, if
they know what they're doing, they're still desigining their pages to work in
Netscape 4). The newbies are the ones who do this, and they won't listen to
complaints about their images not working in Mozilla any sooner than they'll
listen to complaints about BMP being a bad format for the web.

    
    

    
  
So shoule we add support for document.all and document.layers becuase "users 
will be dissapointed if a page that works in IE/NS4 doesn't work in mozilla" ?

And again: when adding something to the mozilla tree it's not just added to the 
mozilla browser, it's added to all browsers derived from mozilla, so yes, it 
makes a difference

    
    

    
  
> BMP is *not* a "popular format" other then on windows, and even there I 
> wouldn't call it popular among people dealing with graphics. We don't *have* >
> to support BMP, we've done just fine without it for several years.

I am sorry but Windows has 95% of the market.  This makes BMP a popular format! 
I could also make the analogy that "Air is *not* 'popular format' other then
with people that are living".  :)

> However GIF is a format we *have* to support since there is so many sites 
> depending on it, if there wasn't I don't think we should have added support 
> for it no.

Sorry.  I think that the UNISYS patent issue *clearly* outweighs your arguments
against BMP.  If we are going to drop image formats I believe it should be in
this order... gif then bmp.  

If you want to drop bmp then fine but you should first acknowledge that we
should drop gif.  (BTW I am not advocating doing either)

> Remember that once support for something is added in mozilla we are pretty 
> much stuck with it for all eternity

Let's not be dramatic.  No we don't have to support anything "for all eternity".
    If there is a reason to remove BMP in the future, we will do so.

BTW.  I am not a Windows supporter.  In fact I haven't used Windows for years. 
I only use Free Software.  I just don't want to give the impression that I am a
windows user/developer who think that BMP should be supported.

The reason that I need BMP support is that I am writing a P2P application using
Mozilla and some people are syndicating BMP files.  These usually come from
Windows platforms and I need to view them under Linux within Mozilla.  I think
this is a clear example of why we need BMP.

We need to be format agnostic.  Ideally Mozilla should be able to support
everything including png, gif, bmp and also multimedia formats.

My $.02.



    
    

    
  
Jonas: document.all and document.layers are DHTML. DHTML is not common enough or
important enough to the functionality of websites to worry about it. Authors who
understand DHTML also typically understand the cross-browser issues involved.

BMP images can be placed on the web by anyone, particularly newbies. If it
doesn't work, images are lost and websites are difficult or impossible to use.

And there is already a patch in progress. There's no good reason to start a
crusade against fixing this bug now that it's already almost fixed.

    
    

    
  
This latest patch is a diff against the latest CVS.
It adds BMP to some lists that I've previously forgotten.

Pavlov, please review it when you have time.


    
  
Keywords: review

    
    

    
  
missing from the 9/15 patch is change to "xpinstall/packager/packages-mac"
add something like:
  viewer:Components:bmpdecoder.shlb


    
    

    
  
This works neither on Linux nor on Mac.
Linux: No Image is displayed, Browser window is completely white
Mac: Titlebar displays "Image 1x1" or similar and consequently only displays one
pixel.

I'll try to fix this on Linux, but I've got no idea why it doesn't work.

    
    

    
  
Comment on attachment 49443 [details] [diff] [review]
Patch: Part 1, second revision

Pavlov gave me r=pavlov on IRC

        Attachment #49443 -
        Flags: review+

    
    

    
  
In the part1 of the patch in packages-unix, libbmp.so must be changed to libimgbmp.so

    
    

    
  
The REQUIRES Line is wrong, should be:
REQUIRES
= xpcom \
                  necko \
		  gfx \
                  gfx2 \
                  imglib2 \
                  string \
                  $(NULL)

And: I finally found the problem on Linux! It was: SetImageData should be called
with bpr as second argument, not width*3; so the next version of the patch will
work on Linux as well as on Windows.

    
    

    
  
Removing helpwanted and review keyword.
Keywords: helpwanted, 
          
            review

    
    

    
  
is this patch ready for review? :)

    
    

    
  
basic: No, at least not the version that is attached to this bug.

What's not working in the latest version (not attached to this bug) is this:
x) Support for compressed BMPs, including those using Bitfields
x) Support for OS/2 BMPs

And last time it was tested on Mac, it wasn't working even though it should have.
Linux support is not optimal.
I would like to fix at least the Linux Issue that I currently have before
checking in the first version; maybe also Bitfields.

        Attachment #48885 -
        Attachment is obsolete: true

        Attachment #48885 -
        Flags: needs-work+

    
    

    
  
OK, this fixes the following problems from the last tarball:
o) Doesn't work on Mac. It should work fine now.
o) Doesn't work on Linux.
Both Linux and Mac have problems opening a local file. Loading one from network
is fine, as is loading via <img src="">, at least on Linux (untested on mac).
o) REQUIRES Line is wrong. I corrected it.
o) Maybe some other small fixes that I forgot
To use it, a rather recent tree is needed because of a change to the image
decoder interface which happened yesterday.

    
    

    
  
Comment on attachment 49443 [details] [diff] [review]
Patch: Part 1, second revision

needs work; mac packages file needs updating as well as mac build  list

        Attachment #49443 -
        Flags: needs-work+

    
    

    
  
OK, the patch I just attached corrects the filename in
xpinstall/packager/packages-unix and the mac files.


    
    

    
  
Comment on attachment 53573 [details] [diff] [review]
Patch: Part 1, third revision

Will attach a new version, which makes it possible to directly open local files.

Also, will change the image/bmp part of the Accept: Header to image/bmp;q=0.1 (to indicate that we'd rather want another format if possible).

Pavlov: Since your last review, Mac build was also fixed; is your r= still valid? If no, could you please re-review?

    
    

    
  
yeah, r=pavlov

    
    

    
  
Comment on attachment 53742 [details] [diff] [review]
Patch: Part 1, fourth (final?) revision

Pavlov told me r=pavlov on IRC

        Attachment #53742 -
        Flags: review+

    
  
Blocks: 55623

    
    

    
  
Excellent work here, Christian.  If you don't mind, I'm going to take this bug, 
get it compiling with the current trunk, and help shepherd this patch into the 
tree.  I'm going to be leveraging this code to produce an ICO decoder.

Assignee: cbiesinger → hyatt
Status: ASSIGNED → NEW

    
    

    
  
Comment on attachment 53742 [details] [diff] [review]
Patch: Part 1, fourth (final?) revision

Obsoleting the last patch.  A new one is coming that compiles on the trunk.

        Attachment #53742 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        I'll see your BMP and raise you an ICO. (obsolete) (deleted)
        — 
          — Splinter Review
      

    


  

    
  
Status: NEW → ASSIGNED
Summary: [RFE] Ability to display Windows .BMP images in Mozilla → [RFE] Ability to display Windows .BMP and .ICO images in Mozilla

    
    

    
  
Comment on attachment 53327 [details]
Latest tarball of new files. untar into modules/libpr0n/decoders/bmp

Hyatt's patch includes the new files

        Attachment #53327 -
        Attachment is obsolete: true

    
    

    
  
Note, I will fix the ENDIAN problems on the sniffing code.  I just need to 
ensure that the 2-byte PRUint16 is converted using the LITTLE_ENDIAN macros.

Ready for r/sr.


    
    

    
  
a few comments:
you are added the bmp stuff to the mac build list.. do you have a mac project 
file?  if not, you will  break the tree... just comment it back out :)
in the http accept header string, you should move icons to after bmp and with a 
lower q level than them.  0.1 for both bmp and icon should be fine.
we should check, at some point, to make sure that only checking for 01 for 
icons is good enough sniffing..
fix the first 2 and r=pavlov

    
  

        Attachment #55905 -
        Flags: review+

    
    

    
  
Comment on attachment 55905 [details] [diff] [review]
I'll see your BMP and raise you an ICO.

sr=ben@netscape.com

        Attachment #55905 -
        Flags: superreview+

    
    

    
  
[jeez, I wish we could segregate the advocacy debate]

+    Close(); /* XXX Is this necessary? No. Delete it. */

Fix this or fix the comment please.

+NS_IMETHODIMP nsBMPDecoder::Flush()
+{
+    PR_LOG(gBMPLog, PR_LOG_DEBUG, ("nsBMPDecoder::Flush()\n"));
+    // Pavlov said this: This function is called at the end, and here I
+    // can process whatever was left over from previous calls to Write*
+    return NS_ERROR_NOT_IMPLEMENTED;
+}

Should return NS_OK, no? Why make the caller think there was an error? (and 
how the *current* caller actually deal with the error is no excuse). 

+#ifdef XP_MAC
+    *aDecoded++ = 0; // Mac needs this padding byte
+#endif

Is this a Mac issue or an Endian issue? Ah, I see you are exlicitly making 
triplets for non-Mac and quads for Mac. I trust this follows imglib rules.

Making SetPixel function calls in the main loop is butt-ugly. *please* use a 
macro (or inline function if you trust the compiler that far) to get these 
expanded inline.

Also, you are *far* better off having separate inner loops rather than switching 
on (the unchanging) mBIH.bpp for each and every pixel. Doing that switch once 
per row is probably acceptable. 

Does someone want to talk about the testing that has been done on this code? On 
which platforms? On what variety of image sizes and pixel depth? This is just 
not going to get the level of real world testing from daily builds that the 
other formats get. We need to catch the problems early.



    
    

    
  
Comment on attachment 55905 [details] [diff] [review]
I'll see your BMP and raise you an ICO.

--sr
kick me if this is rude, but I think you need to fix some stuff.

        Attachment #55905 -
        Flags: superreview+

    
  

        Attachment #55905 -
        Flags: needs-work+

    
    

    
  
Comment on attachment 55905 [details] [diff] [review]
I'll see your BMP and raise you an ICO.

--r=pavlov..  lets see a new patch with the things that jband suggested...  some linux testing would be good too.

        Attachment #55905 -
        Flags: review+

    
    

    
  


  

    
    

    
  
Just tested ICO support on Linux, seems to work fine.

    
    

    
  
http://www.ordsvy.gov.uk/wallpapers/archive2000.htm

I loaded a bunch of huge remote bitmaps from this site.  They worked fine and
rendered incrementally.

In addition I've loaded about 50-60 bitmaps from my local machine without trouble.

I've loaded 10-15 ico files remotely and they load fine.

Ready for r/sr, pav and jband, on my revised patch.

    
    

    
  
I take it back.  I do see problems with some of the BMPs on the above URL.  The 
colors appear to be wrong.  I will debug further.


    
    

    
  
(arg, hyatt could have added me to cc when he took this bug)

This code was tested on Windows, Linux and the Mac.
It works for all the images in the bmpsuite, except the RLE and Bitfield ones
(and in this version, OS/2 ones).
The bmpsuite is available from http://entropymine.com/jason/bmpsuite/.

    
    

    
  
Sorry about that, Christian.  I assumed assignees became cc'ed automatically.  
Anyway, I'm pleased to report that the BMP I thought was malfunctioning is 
fine.  biesi and I spoke on IRC, and he wants to clean up the BMP code a bit 
more before we land, so I'll wait for his revised patch before we do another 
round of r/sr.


    
    

    
  
Comment on attachment 56066 [details] [diff] [review]
Diff of the revised BMP/ICO decoders with jband's changes made.

I don't see any changes to build/mac/build_scripts/MozillaBuildList.pm so the Mac bmp project won't build.  The line in that file for building the Mac project should be uncommented (it is already in the tree).

Do we need a separate project for ICOs?

Please correct these comments before checking in:
+ * Additionally, this patch was only tested on Windows and Linux.
+ * It won't work on Mac because Mac Project files are missing at the moment */

        Attachment #56066 -
        Flags: needs-work+

    
    

    
  
brade:
I think we don't need a new project for bmp; however, the file nsICODecoder.cpp
must be added to the BMP project file. (it's in the same directory as
nsBMPDecoder.cpp)

    
    

    
  
OK, brade kindly added nsICODecoder.cpp to the Mac Project file (pavlov: yes, it
was already there); so this should build on the Mac.

    
    

    
  
Brade, attachment 56066 [details] [diff] [review] is just a partial diff, since none of the other files
changed.  If you look at the previous diff, it includes changes to add the BMP
Mac project file to the build.  

Christian, do you want us to go ahead and get r/sr again and land this thing, or
do you want to make more changes to the BMP code first?


    
    

    
  
hyatt: I'm working on an updated patch right now
I hope to have it done in <= 2 hours

    
    

    
  
As Guardian of the Accept: header, your accept changes are bogus. You've put
image/x-icon in there with no q value, so it has the default q of 1.0, and
image/bmp with the same q as */*, meaning you might as well not have it at all.

Given that there's approximately zero cases in which we'd prefer BMP to some
other format, there is no gain to be had here, as they can be incorporated into
*/*. Making this change would bloat every GET request we make by 31 bytes. We
are already getting complaints that our GETs are too big. Please leave the
Accept: header as it is now.

Also, just a reminder: if any of the files with the MPL on are completely new
and not derivatives, could they be tri-licensed, please?

Gerv



    
    

    
  
<rereads>

Please? :-)

Gerv

    
    

    
  
So just remove the changes to the pref completely?  

    
    

    
  
OK, this is a complete diff (ie. includes changed files).

I think it addresses all the comments which hyatt's earlier patches didn't (also
yours, Gerv; I removed the change to that header).

Note that I haven't changed nsICODecoder.cpp; I know too little about the ICO
format.

This patch also adds support for OS/2 BMPs.

    
    

    
  
r/sr, folks? 


    
    

    
  
Some initial thoughts on looking over the patch:

  * ICO decoder - the gtk alpha optimizations need SetAlpha() to be
    called before SetData() on the corresponding row.  Behavior is
    unspecified otherwise.

  * eyeballing the patch there appear to be large chunks of copied
    code in the two decoders.  Maybe they could be pulled into some
    utility functions for sanity's sake when fixing bugs?  Or if the
    formats are really almost identical maybe just one decoder with
    a little more smarts might be cleaner.

  * magic constants floating in the code - please make up appropriately
    named #defines to help others in understanding the code.  A few 
    examples:

+    if (mPos >= 6 + (mCurrIcon*sizeof(mDirEntryArray)) &&
+        mPos < 6 + ((mCurrIcon+1)*sizeof(mDirEntryArray))) {
+    if (mPos == 22+mCurrIcon*sizeof(mDirEntryArray)) {
+  if (mPos == mImageOffset + 40) {


    
    

    
  
* ICO decoder - the gtk alpha optimizations need SetAlpha() to be
    called before SetData() on the corresponding row.  Behavior is
    unspecified otherwise.

The icons seem to be properly working on Linux though.  

    
  

        Attachment #56066 -
        Attachment is obsolete: true

    
  

        Attachment #55905 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 56173 [details] [diff] [review]
Hopefully final patch

sr=jband
I'm not happy with all the magic numbers either. But, I'm not so sure
a bunch of #defines would make it more readable and I imagine it would increase 
the chance for coding error.

        Attachment #56173 -
        Flags: superreview+

    
    

    
  
Comment on attachment 56173 [details] [diff] [review]
Hopefully final patch

r=pavlov
I'm with jband on the magic numbers.. not sure defines would help..
tor: why would SetAlpha have to be called before SetData?  That is pretty bogus.  I don't believe that we can or should impose that restriction on current or future decoders.

        Attachment #56173 -
        Flags: review+

    
    

    
  
pav: we need the alpha data to be valid before nsImageGTK::ImageUpdated()
called.

    
    

    
  
--sr: I would like at least the latter two concerns (duplicate code,
magic constants) addressed before checkin, as we don't want 
unmaintainable code going into the tree.

hyatt: if ImageUpdated is called before the alpha data is set things
will often work out fine because there will be uninitialized garbage
in the alpha channel.  If you get unlucky you'll get a zeroed chunk
of memory and the image will be optimized away.  This was happening
occasionally on the UI gifs until the ordering in that decoder was
changed.

    
    

    
  
Last time I tested this patch on Mac, I was unable to open local *.bmp files.  
Has this issue been addressed yet?  I'd prefer that this patch not be checked in 
until that issue is also addressed.


    
    

    
  
brade: This issue should be fixed now; however, it wasn't tested on the mac yet.

    
    

    
  
OK this smells of a fillibuster here... please folks let this get checked in 
and post bugs on the remaining issues...


    
    

    
  
I don't think it's a filibuster if someone brings new information before others
and proposes a fix that's easily made (why wait?  sequel bugs sometimes don't
get fixed).  Is that not the case?

/be

    
    

    
  
Yep. My bad. Better to have it work right the first time.

    
    

    
  
I filed bug 108271 for supporting compressed BMPs and those using bitfields.

    
    

    
  
I have added the magic numbers.  I'm going to hold off on code sharing and land
this now and file a separate bug for the alpha problem.  

The reason for this is that I'm going to rewrite the ICO decoder to be
non-incremental, and then I'll ensure that alpha info is set up before pixel
info.  By the time I do that, the ICO code and BMP code will overlap only
minimally, so I'm not going to waste time worrying about code overlap now.  This
bug is 108318.

I am also not going to enable this code on the Macintosh until it has been
tested per brade's request.  I will not be checking in the modifications to the
build list, nor will I be checking in a Mac project file.

    
    

    
  
Fixed.

Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
Bug 108320 covers enabling the BMP and ICO decoders on the Mac.


    
    

    
  
I was able to view .ico files in todays mac build. Seems to work fine!

    
    

    
  
I'm seeing black garbage on the top and left edges of both inline BMP images in
attachment 36229 [details]. The effect seems amplified on the stretched one. Feel free to
split off another bug for that if you think it's necessary.

However, both tests in that testcase pass for me otherwise, 2001110703 W98. The
format of that testcase is 256 color bitmap, saved in MS Paint. As soon as I
find a BMP image that doesn't load into Adobe Photoshop I will check it in the
browser, since that seems like a potential common weakness.

Always nice to see RFEs get fixed.

    
    

    
  
After looking more closely, I think the black garbage may be connected with the
image placeholder not being properly cleared. The black garbage goes away if the
pictures are loaded from cache.

    
    

    
  
Okay, this is fixed on Win XP build 2001120303 and Linux build 2001120308 and I
am able to view these files on Mac OS 92001-12-03-11-trunk and X
2001-12-03-08-trunk but only if I drag and drop the files, trying to open
directly (File->Open file) the files are greyed out, I'll mark nanny this
verified and open a new bug
Status: RESOLVED → VERIFIED

    
    

    
  
For new people here:

This bug is fixed. Mozilla can now display .bmp and .ico files. Although most
cases of bitmaps are displayed correctly, there are a few that won't display
correctly.

As of now, all active bitmap and ico bugs are: bug 110835, bug 108271, bug
110848, bug 108318 and bug 113345. Please correct me if I forgot any. Any bitmap
problems not covered in these bugs should have a new bug added for them.


    
    

    
  
remove self

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: