Closed
Bug 8031
Opened 25 years ago
Closed 23 years ago
[FIX] XBM images not [yet] displayed
Categories
(Core :: Graphics: ImageLib, enhancement, P1)
Core
Graphics: ImageLib
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.
XBM format will be used as an example for users adding
decoders in mozilla/extensions....later.
Updated•25 years ago
|
Target Milestone: M13 → M15
Comment 3•25 years ago
|
||
M13 is beta. I dont think this is happening for beta.
Reporter | ||
Comment 5•25 years ago
|
||
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.]
Reporter | ||
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 6•25 years ago
|
||
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...
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
Comment 10•24 years ago
|
||
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 ago → 24 years ago
Resolution: --- → WONTFIX
Comment 11•24 years ago
|
||
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
Comment 12•24 years ago
|
||
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?
Comment 13•24 years ago
|
||
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
Comment 14•24 years ago
|
||
*** Bug 62311 has been marked as a duplicate of this bug. ***
Comment 16•24 years ago
|
||
*** Bug 70646 has been marked as a duplicate of this bug. ***
Comment 17•23 years ago
|
||
Reopening to assign to biesi
Status: VERIFIED → REOPENED
Resolution: WONTFIX → ---
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Keywords: helpwanted → mozilla0.9.8
Priority: P3 → P1
Target Milestone: --- → mozilla0.9.8
Comment 19•23 years ago
|
||
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!)
Assignee | ||
Comment 20•23 years ago
|
||
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).
Assignee | ||
Comment 21•23 years ago
|
||
Current Status:
The XBM Decoder is working in my tree, I will clean it up a bit and attach it to
this bug.
Assignee | ||
Comment 22•23 years ago
|
||
Assignee | ||
Comment 23•23 years ago
|
||
Create a directory modules/libpr0n/decoders/xbm and untar this file there
Assignee | ||
Updated•23 years ago
|
Attachment #63086 -
Attachment mime type: application/octet-stream → application/x-gtar
Assignee | ||
Comment 24•23 years ago
|
||
Pavlov, could you review the two attachments? tia
Assignee | ||
Comment 25•23 years ago
|
||
The .tar.gz contains a wrong Makefile.in file. This one is the correct one.
(the file didn't work on Windows)
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.8 → mozilla1.0
Assignee | ||
Updated•23 years ago
|
Summary: XBM images not [yet] displayed → [FIX] XBM images not [yet] displayed
Assignee | ||
Comment 27•23 years ago
|
||
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.
Comment 28•23 years 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?
Comment 29•23 years ago
|
||
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.
Comment 30•23 years ago
|
||
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...
Comment 31•23 years ago
|
||
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.
Comment 32•23 years ago
|
||
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 ;-)
Comment 33•23 years ago
|
||
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.
Comment 35•23 years ago
|
||
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.
Comment 36•23 years ago
|
||
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.
Comment 37•23 years ago
|
||
topembed- since it isn't blocking an embeddor, but marked embed since it would
be nice to have
Comment 38•23 years ago
|
||
just replacing http://slip/projects/marvin/imaging/img-xbm/img-xbm.html with a
public available site
Assignee | ||
Comment 39•23 years ago
|
||
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
Assignee | ||
Comment 40•23 years ago
|
||
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
Assignee | ||
Updated•23 years ago
|
Attachment #63086 -
Attachment is obsolete: true
Attachment #63315 -
Attachment is obsolete: true
Assignee | ||
Comment 41•23 years ago
|
||
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.
Assignee | ||
Comment 42•23 years ago
|
||
/me hides
same file, one line below: return; -> return NS_OK;
Comment 43•23 years ago
|
||
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!
Assignee | ||
Comment 44•23 years ago
|
||
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.
Assignee | ||
Comment 45•23 years ago
|
||
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
Assignee | ||
Comment 46•23 years ago
|
||
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
Assignee | ||
Comment 47•23 years ago
|
||
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+
Assignee | ||
Comment 48•23 years ago
|
||
New .tar.gz of the new files; not using nsCString + iterators anymore.
Attachment #79792 -
Attachment is obsolete: true
Assignee | ||
Comment 49•23 years ago
|
||
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+
Assignee | ||
Comment 50•23 years ago
|
||
new tarball of the files for modules/libpr0n/decoders/xbm
Attachment #80587 -
Attachment is obsolete: true
Comment 51•23 years ago
|
||
> 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.
Assignee | ||
Comment 52•23 years ago
|
||
>> 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.
Comment 53•23 years ago
|
||
Adjusting KWs
Topembed nomination: INternal Reference:
http://bugscape.netscape.com/show_bug.cgi?id=12494
Assignee | ||
Comment 54•23 years ago
|
||
addressed tor's comments
Attachment #81324 -
Attachment is obsolete: true
Comment 55•23 years ago
|
||
Untested (uncompiled, even) patch which changes the parsing somewhat
and tries to keep the image even if we run into problems in the file.
Assignee | ||
Comment 56•23 years ago
|
||
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
Assignee | ||
Comment 57•23 years ago
|
||
the changes as patch; result is the same as attachment 83493 [details]
Assignee | ||
Comment 58•23 years ago
|
||
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
Comment 59•23 years ago
|
||
topembed+ since this impacts embedding customers, according to Mike's comment #53.
Comment 60•23 years ago
|
||
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 61•23 years ago
|
||
Comment on attachment 80218 [details] [diff] [review]
Fix Part 1 version 3
sr=tor
Attachment #80218 -
Flags: superreview+
Assignee | ||
Comment 62•23 years ago
|
||
Comment on attachment 83493 [details]
yet another tarball
changing the sr to r
Attachment #83493 -
Flags: superreview+ → review+
Assignee | ||
Updated•23 years ago
|
Attachment #80218 -
Flags: superreview+ → review+
Comment 63•23 years ago
|
||
Comment 64•23 years ago
|
||
This is needed after attachment 84800 [details] (or updated #) is checked in.
Assignee | ||
Comment 65•23 years ago
|
||
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 66•23 years ago
|
||
Comment on attachment 83493 [details]
yet another tarball
sr=dveditz based on tor's OK
Attachment #83493 -
Flags: superreview+
Comment 67•23 years ago
|
||
Comment on attachment 80218 [details] [diff] [review]
Fix Part 1 version 3
sr=dveditz
Attachment #80218 -
Flags: superreview+
Comment 68•23 years ago
|
||
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
Assignee | ||
Comment 69•23 years ago
|
||
Bug 148967 filed.
Checked in this one to trunk.
Finally marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED
Comment 70•23 years ago
|
||
Teri - Can you pls verify this on the trunk? Thanks!
Updated•23 years ago
|
Keywords: mozilla1.0.1
Comment 71•23 years ago
|
||
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.
Assignee | ||
Comment 73•23 years ago
|
||
heikki, yes, it was - the rest of the function doesn't check for null either, so
I didn't do it here.
Comment 74•23 years ago
|
||
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
Comment 75•22 years ago
|
||
please land on the 1.0.1 branch. once there remove the "mozilla1.0.1+" keyword
and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
Comment 76•22 years ago
|
||
the issue brendan raises is a good one. it can be treated independently of this
bug however (when in rome...).
Assignee | ||
Comment 77•22 years ago
|
||
ok, I filed bug 150142 for checking for malloc failure in that function
Comment 79•22 years ago
|
||
This is fixed in Win XP branch 2002061108 but not in Mac OS branch build
2002061105 or linux branch 2002061110
Assignee | ||
Comment 80•22 years ago
|
||
tpreston, that surprises me. can you try deleting component.reg and check again?
Assignee | ||
Comment 81•22 years ago
|
||
hmmm. you're right. libimgxbm.so is missing (on linux at least).
investigating.
Assignee | ||
Comment 82•22 years ago
|
||
ok, I missed a file when I was checking in apparently... can you try again when
newer builds are available? thanks
Comment 83•22 years ago
|
||
Okay, now verified on Mac OS branch build 2002061805 and linux branch build
2002061807
Keywords: fixed1.0.1 → verified1.0.1
Assignee | ||
Comment 84•22 years ago
|
||
*** Bug 138966 has been marked as a duplicate of this bug. ***
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.
Description
•