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)
: Jet Villegas (:jet)
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 | Splinter Review
patch for trunk (2.01 KB, patch)
2006-06-28 22:51 PDT, Martijn Wargers [:mwargers]
no flags Details | Diff | Splinter Review
patch for 1.8.0.7 branch (2.39 KB, patch)
2006-08-04 06:22 PDT, Martijn Wargers [:mwargers]
no flags Details | Diff | Splinter Review
diff -w version of patch (1.39 KB, patch)
2006-08-04 06:23 PDT, Martijn Wargers [:mwargers]
no flags Details | Diff | Splinter Review

Description User image 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 User image 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 User image 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 User image djn 2005-11-30 11:43:29 PST
I've submitted some Talkbacks for this.
Comment 4 User image 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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2005-11-30 11:55:44 PST
Can you provide the submitted Talkback IDs?
Comment 6 User image 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 User image 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 User image 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 User image :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 User image djn 2005-11-30 12:30:19 PST
Aha, thank you for that. The ids are:

TB12435786M, TB12434697M, TB12434574Q, TB12434380X, TB12434269H
Comment 11 User image tor 2005-11-30 12:31:39 PST
Created attachment 204587 [details] [diff] [review]
fix GetCoveredRegion like Render
Comment 12 User image 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 User image 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 User image tor 2005-12-02 16:01:09 PST
Checked in on trunk.
Comment 15 User image Stephen Donner [:stephend] 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 User image 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 User image Martijn Wargers [:mwargers] 2006-04-01 03:31:48 PST
*** Bug 332378 has been marked as a duplicate of this bug. ***
Comment 18 User image Mike Schroepfer 2006-06-28 19:13:01 PDT
Anyone up for a 1.8.1 patch with be's comments addressed?
Comment 19 User image 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 User image Martijn Wargers [:mwargers] 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 User image Martijn Wargers [:mwargers] 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 User image Martijn Wargers [:mwargers] 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 User image Martijn Wargers [:mwargers] 2006-08-04 06:22:34 PDT
Created attachment 232116 [details] [diff] [review]
patch for 1.8.0.7 branch
Comment 24 User image Martijn Wargers [:mwargers] 2006-08-04 06:23:16 PDT
Created attachment 232117 [details] [diff] [review]
diff -w version of patch
Comment 25 User image Martijn Wargers [:mwargers] 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 User image 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 User image Martijn Wargers [:mwargers] 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 User image Martijn Wargers [:mwargers] 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 User image 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.