Closed
Bug 41333
Opened 25 years ago
Closed 22 years ago
Adding a new image type is way too complicated (architecture problem)
Categories
(Core :: Graphics: ImageLib, defect, P3)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: tor, Assigned: pavlov)
References
Details
(Keywords: arch, helpwanted, Whiteboard: [imglib])
Attachments
(3 obsolete files)
This is more of a general mozilla architecture problem, but mostly
related to libimg. Feel free to add more people to the CC list as
appropriate.
I'm working on adding a new image type to mozilla, and it turns out
that this is much more involved than just creating a new image decoder
component. So far the chunks I need to modify are core libimg, layout,
necko, and mailnews. These areas should just call into libimg to figure
out if a particular mimetype is supported instead of duplicating the
enumeration.
Tim:
I didn't go into it in the last email, but
the changes in mime sniffing in imglib(and the
other changes in necko, layout, & mailnews) should not
be needed.
The problem is in the mapping from the server sent
content type and necko's interpreting the content type
as belonging to an imagelib component. As long as the
image decoder component is placed in the bin/components
directory (or whatever is the correct place for the
specific platform) it should be able to find the correct
component.
-P
Here's an inventory of the places in mozilla that hardcode the image formats
supported by mozilla:
docshell/base/nsDSURIContentListener.cpp
nsDSURIContentListener::CanHandleContent() has a hardcoded list
of mimetypes that is willing to be the primary handler for.
embedding/browser/gtk/src/GtkMozEmbedChrome.cpp
GtkMozEmbedChrome::IsPreferred() ditto.
embedding/browser/webBrowser/nsWBURIContentListener.cpp
nsWBURIContentListener::IsPreferred() ditto.
xpfe/browser/src/nsBrowserInstance.cpp
nsBrowserInstance::IsPreferred() ditto, plus known image types
(plus image/tiff?) are listed in a nsModuleComponentInfo list.
Not sure what that list is for.
modules/libimg/src/if.cpp
sniffout_mimetype() identifies streams based on magic cookies.
tests are hardcoded for the currently known formats for libimg.
modules/libimg/src/scale.cpp
il_emit_row() has a couple odd checks for image mimetypes.
layout/build/nsLayoutDLF.cpp
nsLayoutDLF::CreateInstance() has a hardcoded list of known image
mimetypes (gImageTypes) that it checks before trying to create a
document around a channel.
netwerk/mime/src/nsXMLMIMEDataSource.cpp
nsXMLMIMEDataSource::InitFromHack() has a big mapping list of
mimetypes and filename extensions hardcoded.
layout/html/base/src/nsObjectFrame.cpp
nsObjectFrame::IsSupportedImage() has a hardcoded list of supported
image mimetypes and filename extensions that it uses to determine if
it can handle a particular object.
mailnews/mime/src/mimei.cpp
mime_find_class() has a hardcoded list of mimetypes that mailnews
is capable of displaying internally inline.
Thanks, tor. This will make the near future much easier.
Some of the references, like mime sniffer in imglib, don't
exclude other formats from passing in. But do verify some
of the very basic formats.
Others may be more problematic. From what I learned at last
meeting, we may _temporarily_ need to add the mime type to
one include file. And there is some code, unfinished, that
handles a dialog for adding new file extension mapping to mime
content types.
Would you be interested in looking at that code? mscott@netscape
mentioned to me yesterday. You could ping him about it.
-p
tor:
My current theory is that the only file we *should* have
to change is
netwerk/mime/src/nsXMLMIMEDataSource.cpp; nsXMLMIMEDataSource::InitFromHack()
This is the hack that replaces the dialog that should let us
input new file extension to mime content-type mapping.
I say should, because, well ...sometimes theres a distance between
theory and implementation.
Lets put the MNG stuff here for now.
-P
ok.ok. TWO files.
netwerk/mime/src/nsXMLMIMEDataSource.cpp; nsXMLMIMEDataSource::InitFromHack()
AND
netwerk/mime/public/nsMimeTypes.h
-P ;-)
Comment 6•24 years ago
|
||
Changing the OS platform to Linux as this was tested with M16 distribution
to exist on Linux.
-raghu@eng.sun.com
OS: Solaris → Linux
Blocks: 44868
Comment 9•24 years ago
|
||
Nominating for Moz1.0. A comment in bug 18574 suggests that this is the cause of
MNGs not working in chrome; it's listed as a blocker for 18574 (and thus is a
major part of the reason we are still using gifs in the chrome) and besides,
getting a sensible architecture in place for developers to build on sounds like
something most people agree should be a goal for 1.0.
Keywords: mozilla1.0
Updated•24 years ago
|
QA Contact: elig → tpreston
Comment 10•24 years ago
|
||
Added Arch keyword, and changed Platform/OS to All/All, since this isn't
platform specific.
Keywords: nsbeta3 → helpwanted
Whiteboard: [nsbeta3-]
Reporter | ||
Comment 11•24 years ago
|
||
*** Bug 64745 has been marked as a duplicate of this bug. ***
Comment 12•24 years ago
|
||
Is this going to be fixed by the libimg rewrite? If not, we should sort it, as
MNG in chrome would be good for 1.0.
Gerv
Comment 13•24 years ago
|
||
All pnunn bugs reassigned to Pav, who is taking over
the imglib.
Assignee: pnunn → pavlov
Status: ASSIGNED → NEW
Comment 15•23 years ago
|
||
I'm currently trying to add a new Image decoder.
In this progress, I found that this bug does not seem to be fixed.
Comment 16•23 years ago
|
||
Adding bug 104906 and bug 97324 as dependency
Comment 17•23 years ago
|
||
stuart:
What about using nsICategoryManager for the various decoders? Using a category
like "ImageDecoders" and mime-types as entries, this would easily allow
enumeration of image decoders and an easy way for getting one (the value could
be the Contract ID).
http://lxr.mozilla.org/mozilla/source/xpcom/components/nsICategoryManager.idl#25
Comment 18•23 years ago
|
||
this patch eliminates some lists of image mime types. pavlov, could you review?
Assignee | ||
Comment 19•23 years ago
|
||
Comment on attachment 72463 [details] [diff] [review]
partial patch
You should be able to use:
NS_LITERAL_CSTRING("@mozilla.org/image/decoder;2?type=") + aContentType
to avoid a copy. do this and r=pavlov
Attachment #72463 -
Flags: review+
Comment 20•23 years ago
|
||
it wasn't quite as easy as it sounded
Attachment #72463 -
Attachment is obsolete: true
Comment 21•23 years ago
|
||
Comment on attachment 72657 [details] [diff] [review]
patch with stuart's comments
marking stuart's r=
Attachment #72657 -
Flags: review+
Attachment #72657 -
Flags: superreview+
Reporter | ||
Comment 22•23 years ago
|
||
Comment on attachment 72657 [details] [diff] [review]
patch with stuart's comments
sr=tor with the string change suggested by dbaron
Comment 23•23 years ago
|
||
attaching the final patch here for reference
Attachment #72657 -
Attachment is obsolete: true
Comment 24•23 years ago
|
||
Comment on attachment 73031 [details] [diff] [review]
final patch
marking stuart's r= and tor's sr=
Attachment #73031 -
Flags: superreview+
Attachment #73031 -
Flags: review+
Comment 25•23 years ago
|
||
Comment on attachment 73031 [details] [diff] [review]
final patch
a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73031 -
Flags: approval+
Comment 26•23 years ago
|
||
Comment on attachment 73031 [details] [diff] [review]
final patch
checked in
Attachment #73031 -
Attachment is obsolete: true
Comment 27•22 years ago
|
||
OK, if someone is interested in this:
If you want to add an image decoder, these are the places you have to change:
o) uriloader/exthandler/nsExternalHelperAppService.cpp around line 124
This not strictly necessary; it is for mapping file extensions to mime types.
(Note that the OS will be asked if no entry for a given extension is in this
list). If this fails, the file content is sniffed, see below.
o) content/build/nsContentDLF.cpp around line 132
This is necessary so that "full-page" images work, ie. those loaded directly,
not as part of the page. bug 97324 is filed to remove this need.
o) modules/libpr0n/src/imgLoader.cpp - in the function starting at line 691
This is necessary so that content sniffing works, when no mimetype is available.
If it is a local file, Mozilla first tries to get the type from the extension
(basically, at least) (see my first point), then it checks the file contents.
For remote files, the content check is done first, if that fails extension is
used. bug 104906 is filed to change this, by moving the content sniffing into
each decoder, or something like that.
Thanks to bz for explaining some of the above to me, I hope I got it right here :)
Comment 28•22 years ago
|
||
This bug looks like it was fixed a while ago. Can it be resolved?
Comment 30•21 years ago
|
||
bug 231886 filed for first part of comment 27.
No longer depends on: 231886
You need to log in
before you can comment on or make changes to this bug.
Description
•