Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file

VERIFIED FIXED

Status

()

Core
SVG
--
critical
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: djn, Assigned: tor)

Tracking

({crash})

1.8 Branch
x86
Windows XP
crash
Points:
---
Bug Flags:
blocking1.8.1 -
blocking1.8.0.7 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

12 years ago
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>.
(Reporter)

Comment 1

12 years ago
Created attachment 204577 [details]
Viewing this SVG file crashes Firefox 1.5

Viewing this SVG file crashes Firefox 1.5.
(Reporter)

Comment 2

12 years ago
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.
(Reporter)

Comment 3

12 years ago
I've submitted some Talkbacks for this.
(Reporter)

Comment 4

12 years ago
Commenting out the <marker id='lineSeries1'...> definition in <defs> stops the crash. Unfortunately, this is critical to our application.
Can you provide the submitted Talkback IDs?
(Reporter)

Comment 6

12 years ago
I'd be happy to, but how do I determine the Talkback IDs? They're not immediately apparent when submitting the report...
(Reporter)

Comment 7

12 years ago
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.
(Reporter)

Comment 8

12 years ago
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.
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
Assignee: nobody → general
Component: General → SVG
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → 1.8 Branch
(Reporter)

Comment 10

12 years ago
Aha, thank you for that. The ids are:

TB12435786M, TB12434697M, TB12434574Q, TB12434380X, TB12434269H
(Assignee)

Comment 11

12 years ago
Created attachment 204587 [details] [diff] [review]
fix GetCoveredRegion like Render
Assignee: general → tor
Status: UNCONFIRMED → ASSIGNED
Attachment #204587 - Flags: review?(jonathan.watt)
Attachment #204587 - Flags: approval1.8.0.1?
(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 on attachment 204587 [details] [diff] [review]
fix GetCoveredRegion like Render

r=me
Attachment #204587 - Flags: review?(jonathan.watt) → review+

Updated

12 years ago
Keywords: crash
(Assignee)

Comment 14

12 years ago
Checked in on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
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

Updated

12 years ago
Attachment #204587 - Flags: approval1.8.0.1? → approval1.8.0.1-
*** Bug 332378 has been marked as a duplicate of this bug. ***
Flags: blocking1.8.1?

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+

Comment 18

11 years ago
Anyone up for a 1.8.1 patch with be's comments addressed?
Flags: blocking1.8.1+ → blocking1.8.1-
Is this still not fixed on the 1.8.0.x branch either?
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.
Attachment #227507 - Flags: review?(tor)
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.
Attachment #227507 - Flags: review?(tor)

Updated

11 years ago
Attachment #227507 - Attachment is obsolete: true
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.
Created attachment 232116 [details] [diff] [review]
patch for 1.8.0.7 branch
Attachment #232116 - Flags: review?(tor)
Created attachment 232117 [details] [diff] [review]
diff -w version of patch
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.
Flags: blocking1.8.0.7?
(Assignee)

Comment 26

11 years ago
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?
(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 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.
Attachment #232116 - Attachment is obsolete: true
Attachment #232116 - Flags: review?(tor)

Updated

11 years ago
Depends on: 346673
To be fixed by bug 346673
Flags: blocking1.8.0.7? → blocking1.8.0.7+
Flags: blocking1.8.0.7+ → blocking1.8.0.7-
You need to log in before you can comment on or make changes to this bug.