Closed Bug 8031 Opened 25 years ago Closed 23 years ago

[FIX] XBM images not [yet] displayed

Categories

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

enhancement

Tracking

()

VERIFIED FIXED
mozilla1.1alpha

People

(Reporter: elig, Assigned: Biesinger)

References

()

Details

(Keywords: topembed+, Whiteboard: suntrak-n6 [adt2 RTM])

Attachments

(4 files, 10 obsolete files)

(deleted), patch
Biesinger
: review+
dveditz
: superreview+
Details | Diff | Splinter Review
(deleted), application/octet-stream
Biesinger
: review+
dveditz
: superreview+
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), application/xml
Details
* TITLE/SUMMARY XBM images not [yet] displayed (functionality not yet implemented; writing up for tracking purposes.) * STEPS TO REPRODUCE 0) Launch Apprunner [checked 6.9.99 Win32, Linux & Mac OS builds] 1) View: a. http://slip/projects/marvin/imaging/img-xbm/img-xbm.html b. http://slip/projects/marvin/imaging/img-xbm/fancyclock.xbm * RESULT - What happened a. Nothing displayed b. Nothing displayed - What was expected a. The web page displayed should contain a black & white fancy bitmap image of a clock, followed below by a black and white bitmap image of a penguin. (This is currently broken in 4.6, BTW, as checked on Mac OS.) b. A 1-bit clock bitmap. [XBM will be supported natively --- well, via an added library that hasn't yet been added in --- this should work before 5.0 ships. Bug noted for tracking purposes; the functionality is simply not yet implemented.] * CONFIGURATIONS TESTED - [Mac] Power Mac 8500/120 (233 Mhz 604e), 64 MB RAM (VM on; 1 MB of VM used), 1024x768 (Thousands of Colors), Mac OS 8.6 - [Win32] Vectra VL (233 Mhz P2), 96 MB RAM, 800x600 (True Color), NT 4.0 SP3. - [Linux] Vectra VL (266 Mhz P2), 96 MB RAM.
Target Milestone: M12
XBM format will be used as an example for users adding decoders in mozilla/extensions....later.
Status: NEW → ASSIGNED
Target Milestone: M12 → M13
later, later, later.
Target Milestone: M13 → M15
M13 is beta. I dont think this is happening for beta.
*** Bug 19120 has been marked as a duplicate of this bug. ***
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → LATER
This was resolved as LATER without comment. Does that mean that XBM format support will not in fact be needed by any 5.0 components? [e-mailed pnunn to request confirmation.]
Status: RESOLVED → VERIFIED
pnunn confirmed by e-mail that XBM won't be supported in 5.0. Need to remember to revise test plans and nuke test cases as appropriate...
*** Bug 54432 has been marked as a duplicate of this bug. ***
reopen. later went out of style.
Severity: minor → enhancement
Status: VERIFIED → REOPENED
Keywords: helpwanted
Resolution: LATER → ---
Whiteboard: suntrak-n6
Target Milestone: M15 → ---
QA: can you publish the test urls for mozilla.org? thanks.
Status: REOPENED → NEW
QA Contact: elig → tpreston
XBM is no longer used when creating web pages and is not a w3c recommended format. Let them die in peace...
Status: NEW → RESOLVED
Closed: 25 years ago24 years ago
Resolution: --- → WONTFIX
timeless, if you want xbm support, go for it. Make a decoder component. Seems you might already have xbm files yourself for test....but if you don't, google search them. -p
To my knowledge PNG is the only W3C format. I believe we have MozClassic code around. And there are pages that use them (see bug #19120). Do we want to specifically not put in XBM?
The imglib decoder component design was intented to make putting in new image format decoders easy. There are some glitches where other modules are hard coding image format lists. With some work, these small barriers should be gone fairly soon. The point is, if someone wants XBM, or their own private secret 007 image format, they should be able to drop in their component, add their mimetype/file extension mapping to the list and **presto**! View their secret 007 images. I haven't yet run into anyone who wants XBM image support enough to actually do the work. -P
*** Bug 62311 has been marked as a duplicate of this bug. ***
Verified
Status: RESOLVED → VERIFIED
*** Bug 70646 has been marked as a duplicate of this bug. ***
Reopening to assign to biesi
Status: VERIFIED → REOPENED
Resolution: WONTFIX → ---
Found a volunteer!
Assignee: pnunn → cbiesinger
Status: REOPENED → NEW
Status: NEW → ASSIGNED
Keywords: helpwantedmozilla0.9.8
Priority: P3 → P1
Target Milestone: --- → mozilla0.9.8
Just wanted to note that I ran into this problem on OpenVMS. The reason I was trying to open an XBM file was that it's part of the Front-End tests in Cut And Paste. The test name is "Copy-img". (running on a build from 20011205) If this isn't going to be fixed, and as some of the comments suggest that not many people will use this, then maybe the Mozilla Front-End test suite should be modified. (Just a thought :-) Added Terri Preston to the Cc just for this reason. (Hope that's OK!)
jfarrell@arrayinc.com: I am working on this bug. I expect to have it done for 0.9.8 (as the Target Milestone field indicates).
Current Status: The XBM Decoder is working in my tree, I will clean it up a bit and attach it to this bug.
Attached file Fix Part 2: .tar.gz of new files (obsolete) (deleted) —
Create a directory modules/libpr0n/decoders/xbm and untar this file there
Attachment #63086 - Attachment mime type: application/octet-stream → application/x-gtar
Pavlov, could you review the two attachments? tia
Keywords: patch, review
Attached patch Correct Makefile.in (obsolete) (deleted) — Splinter Review
The .tar.gz contains a wrong Makefile.in file. This one is the correct one. (the file didn't work on Windows)
Target Milestone: mozilla0.9.8 → mozilla1.0
Summary: XBM images not [yet] displayed → [FIX] XBM images not [yet] displayed
status?
Keywords: mozilla0.9.9
The latest news I have is: pavlov doesn't think XBM support is wanted/necessary/a good thing, but he'd think about it. That was a few weeks ago.
4 votes for this 8 cc's besides pavlov most of the comments here seem to imply that people want this. tor@acm.org mention that he would prefer that it not be implemented. there is a patch If XBM is not wanted, why is there so much interest?
why not fix it at this point? after all, someone's already done all the work, so it's not like it'll take much of anyone's time.
Since Mozilla supports the Windows bitmap format, it sure seems logical that it should support the X11 bitmap format (if we do not implement XBM - why do we need Windows BMP support (it isn't a W3C recommendation either) ? =:-) ... :) I would say: Implement it - it won't hurt and makes us a little bit more compatible to NS4.x ... and at least the module can be used as example source for further image decoder work...
I'd like to voice my support for adding this patch, and have voted on the bug mostly because a patch exists. I've come across a site recently that displays data in XBM format: http://www.2spi.com/ -- go to a product page and notice the lack of prices in Mozilla. So there's at least one real-world case of someone using XBM, regardless of whether it makes sense to us. Let's add this bad boy.
There is a perl script "counter.pl". It displays the access numbers for a page as a xbm image. The counter is far from being exact. But people get an idea of the popularity of a page. Many people use it for their pages. Nearly all browsers, except mozilla, can render the xbm images :-( There are a lot of things which are not w3c recommended, but which are a *must* for a real working browser. XBM rendering is one of them ;-)
I would like to add to the above comment that such use is not limited to perl. (My personal preference - DCL on OpenVMS - but there are others) Page access counts, displaying parameters without recourse to SSI, etc.
from what I hear is that IE supports this too...
Keywords: 4xp
Yes, it does. As does Opera, according to my tests. We really, really, really ought to add this patch for Moz1.0. I'm seriously considering a nomination for topembed just to get it on enough radars to have the patch reviewed and approved. Hopefully someone will take those steps before I do.
From this morning's devsupport e-mail: >I am the site developer of the webpage I am trying to access. >I'm using a standard cgi hit counter provided by the domain host, >EarthLink, and it works fine with both IE and Opera and not at all >with Netscape. Said hit counter is spitting out XBM images. Earthlink has a lot of customers. We have a fix ready for review and inclusion, and seven votes for the bugs. I'm nominating topembed and nsbeta1.
Keywords: nsbeta1, topembed
topembed- since it isn't blocking an embeddor, but marked embed since it would be nice to have
Keywords: topembedembed, topembed-
Attached patch Fix Part 1 version 2 (obsolete) (deleted) — Splinter Review
un-bitrotted patch includes patches for mac build files - I will take care of getting a project file too before this is checked in
Attachment #63085 - Attachment is obsolete: true
Attached file Fix part 2 version 2 (obsolete) (deleted) —
new version of the tarball with the new files; only changes are the inclusion of the corrected Makefile.in (attachment 63315 [details] [diff] [review]) and the renaming of Makefile.win->makefile.win
Attachment #63086 - Attachment is obsolete: true
Attachment #63315 - Attachment is obsolete: true
arg, I really should compile patches before attaching them... In imgLoader.cpp: 618 aContentType = nsCRT::strndup("image/x-xbitmap", 15); A * is missing before aContentType, I'll of course add it.
/me hides same file, one line below: return; -> return NS_OK;
Comment on attachment 79791 [details] [diff] [review] Fix Part 1 version 2 In makefile.win: -DIRS = ppm gif png jpeg icon\win icon bmp mng +DIRS = ppm gif jpeg icon\win icon bmp mng xbm This looks as if you're losing png support!
er, oops, looks like I didn't change this back when I was testing using the MNG decoder for PNGs instead of the PNG one. I will of course change this back.
brade, please create a mac project file for the XBM decoder - the patch here should already include the needed changes to the mac build files
Attached patch Fix Part 1 version 3 (deleted) — Splinter Review
fixes the three issues mentioned above - compiles now and doesn't remove png decoder when building on windows using nmake
Attachment #79791 - Attachment is obsolete: true
Blocks: 138966
Comment on attachment 79792 [details] Fix part 2 version 2 I'll rewrite this to not use mozilla's strings & iterators, for speed and efficiency of the code and for being able to read it :)
Attachment #79792 - Flags: needs-work+
Attached file Fix Part 2 version 3 (obsolete) (deleted) —
New .tar.gz of the new files; not using nsCString + iterators anymore.
Attachment #79792 - Attachment is obsolete: true
Comment on attachment 80587 [details] Fix Part 2 version 3 should've tested this on large images earlier... because on such images only the first few lines are displayed
Attachment #80587 - Flags: needs-work+
Attached file hopefully final version (obsolete) (deleted) —
new tarball of the files for modules/libpr0n/decoders/xbm
Attachment #80587 - Attachment is obsolete: true
> const int kDefineLen = sizeof(kDefineText) - 1; strlen(kDefineText); > define1Pos += kDefineLen; > define1Pos = strchr(define1Pos, ' '); > if (!define1Pos) > return NS_ERROR_FAILURE; > > mWidth = atoi(define1Pos); if (sscanf(define1Pos, "#define %*s %d", &mWidth) != 1) return NS_ERROR_FAILURE; > const PRUint32 count = dataStart - mBuf; > memmove(mRow, dataStart, count); > mBufSize -= count; Was this supposed to be memmove(mBuf, dataStart, count)? While on the subject, you seem to be parsing by repeatedly shifting the string with memmove. For the size of the files it's probably not worth doing all that work - just buffer the whole thing and track your current position. > // Assuming values are always hexadecimal > PRUint32 pixel = strtoul(start, nsnull, 16); strtoul(start, nsnull, 0) would eliminate that assumption. > mCurRow = 1; zero-base, please.
>> const int kDefineLen = sizeof(kDefineText) - 1; >strlen(kDefineText); Hm, you are sure the strlen will be evaluated at compile time? Though, with your next comment that seems to be unnecessary anyway. >> const PRUint32 count = dataStart - mBuf; >> memmove(mRow, dataStart, count); >> mBufSize -= count; >Was this supposed to be memmove(mBuf, dataStart, count)? Eh, oops, indeed - I wonder why this worked? >While on the subject, you seem to be parsing by repeatedly >shifting the string with memmove. For the size of the files >it's probably not worth doing all that work - just buffer the >whole thing and track your current position. OK... might be indeed a better idea. I'll attach a new patch with your suggestions.
Adjusting KWs Topembed nomination: INternal Reference: http://bugscape.netscape.com/show_bug.cgi?id=12494
Keywords: embed, topembed-topembed
Attached file another try (obsolete) (deleted) —
addressed tor's comments
Attachment #81324 - Attachment is obsolete: true
Attached patch suggested alterations to biesi's patch (obsolete) (deleted) — Splinter Review
Untested (uncompiled, even) patch which changes the parsing somewhat and tries to keep the image even if we run into problems in the file.
Attached file yet another tarball (deleted) —
version with tor's patch applied & fixed I'll attach a patch against my previous tarball too
Attachment #82823 - Attachment is obsolete: true
Attachment #82835 - Attachment is obsolete: true
the changes as patch; result is the same as attachment 83493 [details]
tm->1.1alpha since this will probably not get approval for 1.0, even if it gets review and super-review in time
Target Milestone: mozilla1.0 → mozilla1.1alpha
topembed+ since this impacts embedding customers, according to Mike's comment #53.
Keywords: topembedtopembed+
Comment on attachment 83493 [details] yet another tarball return for sscanf failure should be NS_OK (partial header possible). Change that and sr=tor.
Attachment #83493 - Flags: superreview+
Comment on attachment 80218 [details] [diff] [review] Fix Part 1 version 3 sr=tor
Attachment #80218 - Flags: superreview+
Comment on attachment 83493 [details] yet another tarball changing the sr to r
Attachment #83493 - Flags: superreview+ → review+
Attachment #80218 - Flags: superreview+ → review+
Attached patch patch to fix mac build list (obsolete) (deleted) — Splinter Review
This is needed after attachment 84800 [details] (or updated #) is checked in.
Comment on attachment 84801 [details] [diff] [review] patch to fix mac build list marking obsolete since attachment 80218 [details] [diff] [review] already contains this
Attachment #84801 - Attachment is obsolete: true
Comment on attachment 83493 [details] yet another tarball sr=dveditz based on tor's OK
Attachment #83493 - Flags: superreview+
Comment on attachment 80218 [details] [diff] [review] Fix Part 1 version 3 sr=dveditz
Attachment #80218 - Flags: superreview+
Cool -- does http://gaia.ecs.csus.edu/~dsmith/csc121/lecture_notes/wk14/doodlepad/doodlepad.html work in Mozilla with this patch? If not, file a sequel bug or fix in a revised patch here. /be
Bug 148967 filed. Checked in this one to trunk. Finally marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago23 years ago
Resolution: --- → FIXED
Teri - Can you pls verify this on the trunk? Thanks!
Blocks: 143047
Keywords: adt1.0.1
Whiteboard: suntrak-n6 → suntrak-n6 [adt2 RTM]
Keywords: mozilla1.0.1
Verified fixed win XP trunk build 2002060408, Mac OS X trunk build 2002060403 and linux trunk build 2002060404
Status: RESOLVED → VERIFIED
Comment on attachment 80218 [details] [diff] [review] Fix Part 1 version 3 >+ *aContentType = nsCRT::strndup("image/x-xbitmap", 15); >+ return NS_OK; Was this checked in like this? I think you'd want to check for out of memory condition and return error if so.
heikki, yes, it was - the rest of the function doesn't check for null either, so I didn't do it here.
Checking for malloc failure is still a good thing in some subsystems (the JS engine, which embeds in tiny systems that run out of memory often, must cope in all known cases -- and thanks to scole@planetweb.com, it does), but I bet Mozilla falls over left and right. Still, worth the effort? Be consistent, if you fix this. /be
please land on the 1.0.1 branch. once there remove the "mozilla1.0.1+" keyword and add the "fixed1.0.1" keyword.
the issue brendan raises is a good one. it can be treated independently of this bug however (when in rome...).
ok, I filed bug 150142 for checking for malloc failure in that function
checked in on branch
Keywords: adt1.0.1
This is fixed in Win XP branch 2002061108 but not in Mac OS branch build 2002061105 or linux branch 2002061110
tpreston, that surprises me. can you try deleting component.reg and check again?
hmmm. you're right. libimgxbm.so is missing (on linux at least). investigating.
ok, I missed a file when I was checking in apparently... can you try again when newer builds are available? thanks
Okay, now verified on Mac OS branch build 2002061805 and linux branch build 2002061807
No longer blocks: 138966
*** Bug 138966 has been marked as a duplicate of this bug. ***
Blocks: 167211
Attachment #84800 - Attachment mime type: text/plain → application/xml
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: