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

VERIFIED FIXED

Status

()

defect
--
critical
VERIFIED FIXED
14 years ago
13 years ago

People

(Reporter: djn, Assigned: tor)

Tracking

({crash})

1.8 Branch
x86
Windows XP
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

14 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

14 years ago
Viewing this SVG file crashes Firefox 1.5.
(Reporter)

Comment 2

14 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

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

Comment 4

14 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

14 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

14 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

14 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

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

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

Comment 11

14 years ago
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+
Keywords: crash
(Assignee)

Comment 14

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

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

Comment 18

13 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?
Posted patch patch for trunk (obsolete) — Splinter Review
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)
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.
Posted patch patch for 1.8.0.7 branch (obsolete) — Splinter Review
Attachment #232116 - Flags: review?(tor)
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

13 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)
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.