A SVGPathSegCurvetoCubicSmoothAbs says its pathSegTypeAsLetter is Q and a SVGPathSegCurvetoQuadraticAbs says its pathSegTypeAsLetter is S.

RESOLVED FIXED

Status

()

Core
SVG
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Mitten.O, Assigned: longsonr)

Tracking

unspecified
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fi; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fi; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3

When a path is used to create its segments and they are inserted into it, the xml representation is correct (as per firebug), but the pathSegList.getItem(0).pathSegTypeAsLetter returns wrong values for SVGPathSegCurvetoQuadraticAbs:ses and SVGPathSegCurvetoCubicSmoothAbs:ses return wrong letters.


Reproducible: Always

Steps to Reproduce:
1.Have any page open. Even a simple like:
<html>
<head>
<title>Letters fail</title>

</head>

<body>
	Sorry, this isn't one of those visually impressive problems.
</body>

</html>

Will suffice.
2. Execute the following code:
var svgns="http://www.w3.org/2000/svg";

var path1=document.createElementNS(svgns,"path");
var sseg=path1.createSVGPathSegCurvetoCubicSmoothAbs(50,40,20,60);
path1.pathSegList.appendItem(sseg);

var path2=document.createElementNS(svgns,"path");
sseg=path2.createSVGPathSegCurvetoQuadraticAbs(50,40,20,60);
path2.pathSegList.appendItem(sseg);


alert("Should be S:"+path1.pathSegList.getItem(0).pathSegTypeAsLetter);
alert("Should be Q:"+path2.pathSegList.getItem(0).pathSegTypeAsLetter);


3.The alerts should state the problem. For Google Chrome and Opera show the correct ones, so I'm pretty sure I'm not the one who's mixed up the letters.
Actual Results:  
Well, the given letters are not the same as they are in the dom's view of the xml (seen from firebug) and the specification. This is shown by the alert boxes.

Expected Results:  
The SVGPathSegCurvetoQuadraticAbs should have "Q" as its pathSegTypeAsLetter and the SVGPathSegCurvetoCubicSmoothAbs "S".
(Assignee)

Updated

9 years ago
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general
(Assignee)

Comment 1

9 years ago
Created attachment 343222 [details] [diff] [review]
patch
Assignee: nobody → longsonr
Attachment #343222 - Flags: superreview?(roc)
Attachment #343222 - Flags: review?(roc)
(Assignee)

Updated

9 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

9 years ago
OS: Windows XP → All
Hardware: PC → All
Wouldn't it be simpler for GetSegTypeAsLetter to just return a char?
(Assignee)

Comment 3

9 years ago
Created attachment 345116 [details] [diff] [review]
updated patch


I think it would be better still just to implement the virtual functions directly rather than having one virtual function call another virtual function. This reduces the class size by a virtual function.
Attachment #343222 - Attachment is obsolete: true
Attachment #345116 - Flags: superreview?(roc)
Attachment #345116 - Flags: review?(roc)
Attachment #343222 - Flags: superreview?(roc)
Attachment #343222 - Flags: review?(roc)
nsSVGPathSeg::GetPathSegType(PRUint16 *)
 {
-  *aPathSegType = GetSegType();
-  return NS_OK;
+  NS_ERROR("Should be overridden");
+  return NS_ERROR_UNEXPECTED;

Can't we just make these pure virtual?
(Assignee)

Comment 5

9 years ago
Created attachment 345250 [details] [diff] [review]
address review comment
Attachment #345116 - Attachment is obsolete: true
Attachment #345250 - Flags: superreview?(roc)
Attachment #345250 - Flags: review?(roc)
Attachment #345116 - Flags: superreview?(roc)
Attachment #345116 - Flags: review?(roc)
(Assignee)

Comment 6

9 years ago
Mitten.O, would it be possible for you to expand your testcase so that it covers all possible path segment letters please?
Attachment #345250 - Flags: superreview?(roc)
Attachment #345250 - Flags: superreview+
Attachment #345250 - Flags: review?(roc)
Attachment #345250 - Flags: review+
(Assignee)

Comment 7

9 years ago
checked in http://hg.mozilla.org/mozilla-central/rev/762df4556c17
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

9 years ago
Created attachment 346054 [details]
PathSegLettersTestcase

This tests all the letters (I found), not just the ones that were wrong.
I am glad you fixed this.
(Assignee)

Comment 9

9 years ago
Created attachment 346065 [details] [diff] [review]
testcase converted to mochitest
(Assignee)

Updated

9 years ago
Flags: in-testsuite+
(Assignee)

Comment 10

9 years ago
checked in mochitest http://hg.mozilla.org/mozilla-central/rev/cb9d21b64b17
You need to log in before you can comment on or make changes to this bug.