293459 - SVG crash when gdiplus.dll not present [@ nsSVGPathGeometryFrame::GetCoveredRegion]

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[PikeUK]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050508 Firefox/1.0+

Firefox crashes on the above URL when gdiplus.dll is not installed, it seems to
be related to manipulating the DOM in script, see reduced testcase in next comment.

Steps to reproduce:

1. Install Firefox on a system that does not have a copy of gdiplus.dll available.
2. Visit the URL (http://w3future.com/2005/svg/icosa/icosa.svg).

Talkback IDs:

TB5688551E
TB5688687H

Comment 1

[PikeUK]

Attachment: Reduced Testcase

Comment 2

[Hermann Schwab]

fixed some days ago: Bug 292222 Crash on some SVG with no gdi+ [@
nsStyleContext::GetStyleData]

This one is new:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB5688551E
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB5688687H

nsSVGPathGeometryFrame::GetCoveredRegion 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/svg/base/src/nsSVGPathGeometryFrame.cpp,
line 339]
nsSVGDefsFrame::DidModifySVGObservable 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/svg/base/src/nsSVGDefsFrame.cpp,
line 230]
nsFrameManager::ReParentStyleContext 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsFrameManager.cpp,
line 924]
nsCSSFrameConstructor::ContentRemoved 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 9787]
nsIPresShell::ReconstructStyleDataInternal 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp,
line 5509]
nsGenericElement::RemoveChild 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 3251]
nsGenericElement::InsertBefore 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 3013]
nsHTMLLabelElement::GetFirstFormControl 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsHTMLLabelElement.cpp,
line 382]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2111]
XPC_WN_GetterSetter 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1310]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1320]
js_Interpret 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 3611]
js_Execute 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1551]
JS_EvaluateUCScriptForPrincipals 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsapi.c, line 3786]
nsJSContext::EvaluateString 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1047]
nsGlobalWindow::RunTimeout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 5193]
nsGlobalWindow::ClearAllTimeouts 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 5562]
nsAppStartup::QueryInterface 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 123]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 60]
KERNEL32.DLL + 0x1b560 (0xbff8b560)
KERNEL32.DLL + 0x1b412 (0xbff8b412)
KERNEL32.DLL + 0x19dd5 (0xbff89dd5)

Comment 3

[tor]

Attachment: more bulletproofing

Comment 4

[Scooter Morris]

Comment on attachment 183043 [details] [diff] [review]
more bulletproofing

Looks good (obvious) to me.

Comment 5

[Asa Dotzler [:asa]]

Comment on attachment 183043 [details] [diff] [review]
more bulletproofing

a=asa

Comment 6

[tor]

Checked in.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Instead of bulletproofing every little bit of the SVG code, why not make it so
that you can't create any SVG frames at all if there's no working SVG renderer
available?

Comment 8

[tor]

I was interested in doing that, but there wasn't an existing API to set a frame
into an error state and prevent leaf nodes from being created.  bz has a patch
for that, but it isn't in yet.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why not just act as though SVG weren't enabled?

Comment 10

[tor]

SVG disabled prevents the elements from being constructed, not the frames.

Comment 11

[Boris Zbarsky [:bzbarsky]]

The leaf frame patch won't help here, will it?  That would just let you prevent
child frame construction for a particular frame, but there's nothing preventing
someone from sticking an <svg:gradient> as the child of an <html:table>.  At the
moment, we'll create an SVG frame in that case...  And the <html:table> is
certainly not in error in any way.

I think what David was proposing is that if there is no renderer
ConstructSVGFrame should just return null (so we fall back on constructing
frames by display type) or explicitly say there should be no frame (the way
ConstructXULFrame does for some cases).

Comment 12

[Hixie (not reading bugmail)]

According to the SVGWG, the following:

   <?xml-stylesheet ... >                   /* stylesheet: */
   <html:html ...>                          html { display: block; }
    <svg:rect ...> Text </svg:rect>         rect { border: solid; color: green;
   </html:html>                                    text-indent: 1em; }

...should render exactly the same as:

   <?xml-stylesheet ... >                   /* stylesheet: */
   <html:html ...>                          html { display: block; }
    <abc:defg ...> Text </abc:defg>         defg { border: solid; color: green;
   </html:html>                                    text-indent: 1em; }

(Assuming appropriate namespace declarations.)

So bz's suggestion that we should construct SVG elements' frames based on
display type if there is no appropriately placed <svg> ancestor is correct.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ian, we're talking about what to do if SVG is enabled but we don't have a
renderer for it.  The document in question is well-formed SVG....

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #10)
> SVG disabled prevents the elements from being constructed, not the frames.

Are you saying that disabling the SVG pref is not equivalent to a build without
SVG support?  I thought it should be.

Comment 15

[Hixie (not reading bugmail)]

Ah. Then what dbaron said.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I filed bug 294519 to follow up.