Last Comment Bug 318379 - Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file
: Firefox 1.5 should not crash when viewing an SVG file, or a page embedding th...
Status: VERIFIED FIXED
: crash
Product: Core
Classification: Components
Component: SVG (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- critical with 1 vote (vote)
: ---
Assigned To: tor
: Hixie (not reading bugmail)
Mentors:
(See attached SVG file)
: 332378 (view as bug list)
Depends on: 346673
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-30 11:27 PST by djn
Modified: 2006-08-23 15:38 PDT (History)
9 users (show)
mtschrep: blocking1.8.1-
dveditz: blocking1.8.0.7-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Viewing this SVG file crashes Firefox 1.5 (9.55 KB, image/svg+xml)
2005-11-30 11:28 PST, djn
no flags Details
fix GetCoveredRegion like Render (1.58 KB, patch)
2005-11-30 12:31 PST, tor
jwatt: review+
brendan: approval1.8.0.1-
Details | Diff | Review
patch for trunk (2.01 KB, patch)
2006-06-28 22:51 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details | Diff | Review
patch for 1.8.0.7 branch (2.39 KB, patch)
2006-08-04 06:22 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details | Diff | Review
diff -w version of patch (1.39 KB, patch)
2006-08-04 06:23 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details | Diff | Review

Description djn 2005-11-30 11:27:28 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

Firefox 1.5 crashes when an attempt to view the attached SVG file is made.

Reproducible: Always

Steps to Reproduce:
1. Save the attached SVG file to your machine.
2. Open SVG file in Firefox.
3. Observe crash.

Actual Results:  
Firefox crashes.

Expected Results:  
SVG file should be shown.

This SVG file views fine with the Adobe SVG Viewer 3 and 6 beta. The crash also occurs when the SVG file is embedded within an HTML page using <object> or <iframe>.
Comment 1 djn 2005-11-30 11:28:26 PST
Created attachment 204577 [details]
Viewing this SVG file crashes Firefox 1.5

Viewing this SVG file crashes Firefox 1.5.
Comment 2 djn 2005-11-30 11:39:38 PST
Removing the <defs> section from the file stops the crash from happening. Needless to say, this isn't terribly helpful, as we need that for additional visual effects when the Adobe SVG viewer is used to view the content.
Comment 3 djn 2005-11-30 11:43:29 PST
I've submitted some Talkbacks for this.
Comment 4 djn 2005-11-30 11:46:11 PST
Commenting out the <marker id='lineSeries1'...> definition in <defs> stops the crash. Unfortunately, this is critical to our application.
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-11-30 11:55:44 PST
Can you provide the submitted Talkback IDs?
Comment 6 djn 2005-11-30 11:58:10 PST
I'd be happy to, but how do I determine the Talkback IDs? They're not immediately apparent when submitting the report...
Comment 7 djn 2005-11-30 12:02:19 PST
I notice that the attached SVG file has a <path> element with the 'd' attribute set to an empty string (obviously not sensible). Commenting out that element eliminates the crash, so this looks like it could be poor error handling of an unexpectedly empty 'd' attribute on the <path> element.
Comment 8 djn 2005-11-30 12:04:46 PST
Yes -- if I provide path data in the (formerly empty) 'd' attribute on the last path element in the attached file, the crash is eliminated. That looks like I can easily avoid the crash now.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2005-11-30 12:14:37 PST
You need to find the talkback executable and launch it, it should display a list of IDs that can be copied into the bug. For Firefox on windows the default location is:

C:\Program Files\Mozilla Firefox\extensions\talkback@mozilla.org\components\talkback.exe
Comment 10 djn 2005-11-30 12:30:19 PST
Aha, thank you for that. The ids are:

TB12435786M, TB12434697M, TB12434574Q, TB12434380X, TB12434269H
Comment 11 tor 2005-11-30 12:31:39 PST
Created attachment 204587 [details] [diff] [review]
fix GetCoveredRegion like Render
Comment 12 Reed Loden [:reed] (use needinfo?) 2005-11-30 13:20:45 PST
(In reply to comment #11)
> Created an attachment (id=204587) [edit]
> fix GetCoveredRegion like Render

I'm not a reviewer myself, but I couldn't help noticing this.

If every if statement requires num, why not either:
1) if (!num) return region;
2) if (num) { ... other if statements ... }

I just like to keep stuff optimized to the greatest extent when I code, and this was just bugging me. :P
Comment 13 Jonathan Watt [:jwatt] 2005-11-30 13:21:40 PST
Comment on attachment 204587 [details] [diff] [review]
fix GetCoveredRegion like Render

r=me
Comment 14 tor 2005-12-02 16:01:09 PST
Checked in on trunk.
Comment 15 Stephen Donner [:stephend] - PTO; back on 5/28 2005-12-03 10:14:12 PST
Verified FIXED using trunk SeaMonkey build 2005-12-03-07 on Windows XP with https://bugzilla.mozilla.org/attachment.cgi?id=204577&action=view as my testcase.
Comment 16 Brendan Eich [:brendan] 2005-12-19 14:52:12 PST
Comment on attachment 204587 [details] [diff] [review]
fix GetCoveredRegion like Render

Reed's right, this patch should be minimized.  Please do so and renominate.

/be
Comment 17 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-04-01 03:31:48 PST
*** Bug 332378 has been marked as a duplicate of this bug. ***
Comment 18 Mike Schroepfer 2006-06-28 19:13:01 PDT
Anyone up for a 1.8.1 patch with be's comments addressed?
Comment 19 Mike Shaver (:shaver -- probably not reading bugmail closely) 2006-06-28 21:34:14 PDT
Is this still not fixed on the 1.8.0.x branch either?
Comment 20 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-06-28 22:51:49 PDT
Created attachment 227507 [details] [diff] [review]
patch for trunk

I've only tested that it compiles, but I'm basically only doing what is suggested in comment 12.
Comment 21 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-06-28 22:54:32 PDT
Comment on attachment 227507 [details] [diff] [review]
patch for trunk

oops, no, that's a different function. Apparently, there are more places that could be optimised, I guess.
Comment 22 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-06-28 23:55:52 PDT
So apparently this was already checked into the 1.8.1 tree:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/svg/base/src/nsSVGPathGeometryFrame.cpp&rev=&root=/cvsroot&rev=MOZILLA_1_8_BRANCH#350
as part of bug 339220.
If this is wanted for the 1.8.0.x branch, I guess someone should ask for a blocking 1.8.0.6 flag.
A patch should be easy to make, I think.
Comment 23 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-08-04 06:22:34 PDT
Created attachment 232116 [details] [diff] [review]
patch for 1.8.0.7 branch
Comment 24 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-08-04 06:23:16 PDT
Created attachment 232117 [details] [diff] [review]
diff -w version of patch
Comment 25 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-08-04 06:31:48 PDT
The patch compiles, but I can't check if the patch fixes the crash on 1.8.0.x branch, since I don't have a build that actually supports svg, but it should fix the crash.
Comment 26 tor 2006-08-04 06:50:35 PDT
This latter patch looks similar to bug 346673, which has been checked in on the 1.8 branch.  Could you check if there is still a problem there?
Comment 27 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-08-04 07:01:58 PDT
(In reply to comment #26)
> This latter patch looks similar to bug 346673, which has been checked in on the
> 1.8 branch.  Could you check if there is still a problem there?

You mean the patch in that bug would also fix this bug?
That bug is still a problem in the 1.8.0.x tree, because it hasn't been checked in there yet.
Comment 28 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-08-04 07:35:23 PDT
Comment on attachment 232116 [details] [diff] [review]
patch for 1.8.0.7 branch

So I guess the patch in bug 346673 will fix it on the 1.8.0.x branch.
Comment 29 Daniel Veditz [:dveditz] 2006-08-10 12:09:02 PDT
To be fixed by bug 346673

Note You need to log in before you can comment on or make changes to this bug.