User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4a) Gecko/20030322 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4a) Gecko/20030328 The svg/gdi+ builds hang at the splash screen if gdiplus.dll is missing from the mozilla binary directory. No feedback is given about why the build won't load. Reproducible: Always Steps to Reproduce: Expected Results: Error message e.g. "A required file, gdiplus.dll was not found in your mozilla directory. Instructions for obtaining this file can be found at http://www.mozilla.org/projects/svg/build.html [close] " Closing the dialog should cause Mozilla to exit cleanly.

How do we handle other required libraries that are missing? Maybe this is something to live with and we just have to make sure to always ship with gdiplus.dll?

beisi has suggested that when a required library is missing the requiring component fails to load, and thus do_CreateInstance/do_GetService will fail. Perhaps we should use NS_ABORT to prevent the hang rather than NS_ASSERTION at: http://lxr.mozilla.org/seamonkey/source/layout/svg/base/src/nsSVGOuterSVGFrame.cpp#319 until we do something better.

Actually Mozilla doesn't hang for me. If I remove gdiplus.dll from PATH then I get multiple alerts with the title "XPCOM:EventReceiver:mozilla.exe - Unable To Locate DLL" and a message along the lines of "The dynamic link library GDIPLUS.dll could not be found in the specified path...". After closing all of these, Mozilla exits, so I guess the behaviour as it stands is sort of close to how you wanted Mozills to behave James. However, I don't think this is how it should behave. It would be better if Mozilla simply behaved as if it doesn't support SVG if the renderer can't be found (and show an alert about the missing library on each failed attempt to render SVG). There is no reason why Mozilla shouldn't be able to start and load HTML, check email, etc. just because it can't render SVG. Currently I'm not sure why Mozilla is trying to find gdiplus.dll when it starts, but I'll try to track this down and see what can be done. I'll probably change the summary for this bug to reflect what I'll try to do.

I ran an svg enabled moz through dependency walker with gdiplus.dll removed and saved the output to http://jwatt.org/moz/mozilla-gdiplus.dwi. Hopefully the output will help me figure out why mozilla won't start when gdiplus.dll is missing.

Snapshot of making mozilla+svg deal gracefully with a missing gdiplus.dll. Pops up a localizable dialog indicating the missing library (once per session) and draws a red "X" where the SVG content would be. Notes: layout/build - temporary test hacks, sections can be removed xpcom - turns off error dialogs globally, need to talk to xpcom people about the appropriate sections to bracket for component registration. once that's done, the dialogs will need to be turned off again for a short window in nsSVGOuterSVGFrame.cpp layout/svg/base - change from bug 265367 will be used to simplify this and to prevent us from needlessly creating the child frames of an outer svg frame missing its renderer. packaging files - could use input from someone more familiar with packaging systems to tell me which manifest files I've missed.

Oh, and the libart renderer also needs to be componentized.

i'll have to check, but i believe the annoying please insert the disk in drive A is also affected by: + SetErrorMode(SEM_FAILCRITICALERRORS); and that the conclusion we reached was that somewhere early in each app, that should be set for the life span of the app and never changed. it may take me a week to find the right bug. blah, no point. i'm not wrong. http://support.microsoft.com/kb/q115828/ anyway, if you can find the right place to stick that error mode globally, that'd be wonderful :). if you can't, i'll just have to remember to squish the unset call when we do find that place.

This looks good, but it might make sense to have a single entry point into the renderer DLL that returns a pointer to a function table. Then you only have one call to GetProcAddress. See the implementation of NS_GetFrozenFunctions for example.

scooter, could you review the layout portions of this patch? bsmedberg, could you look over the packaging files and build changes?

You want FORCE_SHARED_LIB = 1 for libs that should be dynamic even in static builds. And you should remove LIBXUL_LIBRARY from code that goes into those sharedlibs.

See also bug 281471

Comment on attachment 173757 [details] [diff] [review] adjust makefile per bsmedberg's comments In nsSVGOuterSVGFrame::Paint you use drawLine to draw two lines (a red X) on the screen if you don't have a renderer. It would be useful to add a comment to tell the reader what's going on (and that its not using SVG to do the drawing)

Comment on attachment 173806 [details] [diff] [review] fix comment, add property file and header missing from last diff >--- xpinstall/packager/packages-unix 7 Feb 2005 13:22:35 -0000 1.300 >+++ xpinstall/packager/packages-unix 8 Feb 2005 22:53:37 -0000 >@@ -356,16 +356,20 @@ bin/res/mathml.css > bin/res/rdf/folder-closed.gif > bin/res/rdf/folder-open.gif > bin/res/rdf/loading.gif > bin/res/html/* > bin/res/fonts/* > bin/res/dtd/* > bin/res/builtin/platformHTMLBindings.xml > >+; svg >+bin/components/libgksvgcairo.so >+bin/res/svg.css >+bin/res/svg.properties Are there any xpt files that need to be included here? Same for other platforms / manifests. >--- layout/svg/base/src/nsSVGOuterSVGFrame.cpp 8 Feb 2005 00:59:51 -0000 1.38 >+++ layout/svg/base/src/nsSVGOuterSVGFrame.cpp 8 Feb 2005 22:53:38 -0000 > nsresult nsSVGOuterSVGFrame::Init() > { > #if (defined(MOZ_SVG_RENDERER_GDIPLUS) + \ > defined(MOZ_SVG_RENDERER_LIBART) + \ > defined(MOZ_SVG_RENDERER_CAIRO) > 1) > #error "Multiple SVG renderers. Please choose one manually." > #elif defined(MOZ_SVG_RENDERER_GDIPLUS) > mRenderer = do_CreateInstance(NS_SVG_RENDERER_GDIPLUS_CONTRACTID); >+ if (!mRenderer) >+ AlertMissingGDIPlus(); Is it really ok to do this during frame construction? Please get ok on that from a layout guy. Conditional sr=me with those points addressed.

bz says doing an alert from the frame Init() should be fine as long as it isn't modal (which it's not).

Checked in.