Improper handling of path elements which start at x = 0

[GoogleCodeExporter]

The attached image file contains a single vertical red line starting at x = 0  
relative to the containing svg described using the path command. Upon import I 
would expect to see a vertical red line in the canvas but instead I see a 
multitude of errors that suggest that the bounding box for this line is NaN. 

The root cause of this bug seems to be in the svgedit.utilities.getPathBBox 
function. It appears to be trying to short circuit if it encounters a segment 
of a path with no x value. However the test it is using is the following: 

    if(!seg.x) continue;

Which of course will also fail if the seg.x == 0, which it does in the case of 
a vertical line drawn from the origin (in this case the origin is relative to 
the parent svg so it can be anywhere on the canvas). The upshot of this short 
circuit is that the resulting bbox that is returned from the method contains 
Infinity and -Infinity for its height and width values respectively.

Now I know there have been other issues with bounding boxes and browser 
inconsistencies so before I file a patch I wanted to check to see if anyone is 
aware of the intent of this check. If it really is checking for the value to be 
undefined then the check should be more strict, something along the lines of: 

  if(typeof seg.x === "undefined" )continue;

Thoughts?

Original issue reported on code.google.com by adamben...@gmail.com on 28 Mar 2012 at 5:10

[GoogleCodeExporter]

Forgot to attach file.

Original comment by adamben...@gmail.com on 28 Mar 2012 at 5:12

Attachments:

• verticalline.svg

[GoogleCodeExporter]

I'm sure it's just an oversight on the coder's part. Sometimes it feels 
convenient to do if(!x) and ignore the fact that we accept 0 as well.

However in this case the check is unnecessary. From the spec[1] getItem() will 
return any of path segment types, all of which should have the attributes x & y.

Fix in r2071.

[1] http://www.w3.org/TR/SVG/paths.html#__svg__SVGPathSegList__getItem

Original comment by asyazwan on 29 Mar 2012 at 2:47

• Changed state: Fixed

[GoogleCodeExporter]

Im sure you are right, unfortunately in JavaScript it is sometimes a little too 
easy to be very broad when it comes to conditionals, which of course cuts both 
ways. Thanks for taking a look at this so quickly!

Adam

Original comment by adamben...@gmail.com on 29 Mar 2012 at 3:18

[GoogleCodeExporter]

Revision: r2071 seems to break the path selection box for me. Chrome 17 Linux. 
Make a path and then select it. The path selection box has x, y, width & height 
as 0.

Original comment by mcdonald...@gmail.com on 30 Mar 2012 at 12:54

[GoogleCodeExporter]

Broken for my Chrome 17 too, but fine with FF 11.

Confirmed that this fix is the cause. Thanks for the report.

Original comment by asyazwan on 30 Mar 2012 at 2:16

• Changed state: Started
• Added labels: NeededFor-2.6

[GoogleCodeExporter]

There are actually cases where segment's x/y undefined. My ignorance bites 
again...
Applied adam's earlier suggestion and included a unit test for getPathBBox().

Fixed in r2072.

Original comment by asyazwan on 30 Mar 2012 at 3:55

• Changed state: Fixed