content/build/nsContentDLF.cpp currently contains an array of supported Image Mime types. On your request, I file this as a bug on you. There should be some other way.
16 years ago
taking, I think I know how to fix this
Take a look at this patch: http://bugzilla.mozilla.org/attachment.cgi?id=61711&action=view Decoders need to register in some fashion with the imgILoader service. People outside like nsContentDLF should simply ask the imgILoader service if it supports a certain mimetype and an answer should be given back.
yeah, well... the issue is this, basically: Currently, people look for a component with the contract id of @mozilla.org/content-viewer-factory/view;1?type=<the mime type> to check if Gecko can handle documents for a specific type... So my plan is to make them use a category instead (what some code parts already do). So imagelib would at registration time add a category entry for each mime type it supports. So the question is: pavlov, is it ok with you to do that (to call AddCategoryEntry for each supported mime type when the imagelib component gets registered)?
before I forget - the ART decoder in the commercial tree will need a change similar to the MNGDecoder change in my patch that I'll attach
(forgot the important part - it needs to do it for the mimetypes image/x-art as well as image/x-jg)
Created attachment 113681 [details] [diff] [review] patch so, here's the patch. also makes users of gecko-content-viewers use do_GetService to get the DocumentLoaderFactory.
Comment on attachment 113681 [details] [diff] [review] patch Looks reasonable to me. r=jst You might be able to get email@example.com to help you out with the Netscape commercial changes when you land this.
Comment on attachment 113681 [details] [diff] [review] patch bz, do you have time to sr this? If not, just tell me, and I'll find another super-reviewer jst, thanks, I'll talk to him
Comment on attachment 113681 [details] [diff] [review] patch >Index: docshell/base/nsDocShell.cpp >+ nsCOMPtr<nsICategoryManager> catMan(do_GetService(NS_CATEGORYMANAGER_CONTRACTID)); >+ nsXPIDLCString contractId; >+ catMan->GetCategoryEntry("Gecko-Content-Viewers", "text/html", getter_Copies(contractId)); What if the do_GetService fails? > // Note that we're always passing in "view" for the contractid above > // and to the docLoaderFactory->CreateInstance() at the end of this method. > // nsLayoutDLF makes the determination if it should be a "view-source" This comment no longer seems relevant. Is it? >Index: layout/build/nsContentDLF.cpp > // add the MIME types layotu can handle to the handlers category. "layout". I know you didn't write that, but fix it. Is there a reason you leave no unregistration proc for nsContentDLF (but have one for nsImageModule) ? >+/* To get a component that implements nsIDocumentLoaderFactory Start the comment like: /** * To get ... (javadoc syntax) sr=me with those nits
>What if the do_GetService fails? Then you have a crash ;) Ok, I'll fix it >This comment no longer seems relevant. Is it? OK, I'll move the comment down to the mentioned CreateInstance line and adjust the wording. >"layout". I know you didn't write that, but fix it. ok >Is there a reason you leave no unregistration proc for nsContentDLF (but have >one for nsImageModule) ? The reason is that the current code doesn't have one either... and I'm lazy :) Do you want me to write one? I felt that the added code wouldn't be all that useful... would it ever get called? >Start the comment like:[...] >(javadoc syntax) sure, will do
> Do you want me to write one? Please do. Dunno if it ever gets called, but it's bad not to clean up after yourself....
ok, I'll have a new patch, with these additional changes: o) Make RegisterTypes only take two arguments, since that's all that's needed o) Fix a part of DocShell.cpp code that I missed earlier (also use do_GetService and requery category manager after reloading plugins) and with the issues mentioned by bz addressed. I'll attach it after I tested it.
Created attachment 114251 [details] [diff] [review] patch v2
This is blocked by bug 193031, because the plugins code (after my patch) relies on the aPersist argument of AddCategoryEntry to work. I'm not requesting new r/sr because the previous ones should still apply.
Created attachment 115252 [details] [diff] [review] replacement patch for modules/plugin/ ok, ignore the modules/plugin/ part of the previous patch(es), this one is better - it unregisters the plugin types on shutdown. Now, I think there still may be edge cases when mozilla crashes at the right time that the category entries will be written to the registry, however I don't think that's much of a problem. that should be fixed once the bug this one depends on gets addressed.
Comment on attachment 115252 [details] [diff] [review] replacement patch for modules/plugin/ this is the result of my merge with the patch you sent me, with a few changes. hope it's ok with you.
Comment on attachment 115252 [details] [diff] [review] replacement patch for modules/plugin/ clearing review request, I am going to attach a new patch per emailed comments.
Created attachment 115669 [details] [diff] [review] better replacement patch for moduels/plugin/
Comment on attachment 115669 [details] [diff] [review] better replacement patch for moduels/plugin/ r=peterl
Comment on attachment 115669 [details] [diff] [review] better replacement patch for moduels/plugin/ sr=jst
I just ntoiced a typo in my patch, for which I checked in this patch: --- modules/libpr0n/build/nsImageModule.cpp 27 Feb 2003 13:51:48 -0000 1.7 +++ modules/libpr0n/build/nsImageModule.cpp 27 Feb 2003 15:09:39 -0000 1.8 @@ -122,15 +122,15 @@ "image/pjpeg", "image/jpg", #endif #ifdef IMG_BUILD_bmp "image/x-icon", "image/bmp", #endif -#ifdef IMB_BUILD_png +#ifdef IMG_BUILD_png "image/png", "image/x-png", #endif #ifdef IMG_BUILD_xbm "image/x-xbitmap", "image/x-xbm", "image/xbm",
this caused a regression, see bug #195386 so if this goes on any branches (doesn't sound like it is planned to), we need to take the supplimentatl fix in that bug, too.