199771 - No error message if gdiplus.dll is not found in mozilla directory

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[James Graham]

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.

Comment 1

[Alex Fritze]

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?

Comment 2

[Jonathan Watt [:jwatt]]

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. 

Comment 3

[Jonathan Watt [:jwatt]]

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.

Comment 4

[Jonathan Watt [:jwatt]]

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.

Comment 5

[tor]

Attachment: snapshot

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.

Comment 6

[tor]

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

Comment 7

[timeless]

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.

Comment 8

[tor]

Attachment: use shim component on win32

Comment 9

[Darin Fisher]

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.

Comment 10

[tor]

Attachment: use single entry point to get renderer functions

Comment 11

[tor]

Attachment: update packaging, remove undef'd bits

Comment 12

[tor]

scooter, could you review the layout portions of this patch?

bsmedberg, could you look over the packaging files and build changes?

Comment 13

[Benjamin Smedberg]

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.

Comment 14

[Benjamin Smedberg]

See also bug 281471

Comment 15

[tor]

Attachment: adjust makefile per bsmedberg's comments

Comment 16

[Scooter Morris]

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 17

[tor]

Attachment: fix comment, add property file and header missing from last diff

Comment 18

[Brian Ryner (not reading)]

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.

Comment 19

[tor]

Attachment: include dom_svg.xpt in packaging

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

Comment 20

[tor]

Checked in.