Closed Bug 459953 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: SVG, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: Mitten.O, Assigned: longsonr)

Details

Attachments

(3 files, 2 obsolete files)

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".
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → longsonr
Attachment #343222 - Flags: superreview?(roc)
Attachment #343222 - Flags: review?(roc)
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Wouldn't it be simpler for GetSegTypeAsLetter to just return a char?
Attached patch updated patch (obsolete) — Splinter Review
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?
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)
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+
checked in http://hg.mozilla.org/mozilla-central/rev/762df4556c17
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Attached file PathSegLettersTestcase
This tests all the letters (I found), not just the ones that were wrong.
I am glad you fixed this.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.