Browser Crashed When SVG Contains Arc

[henrikskar]

Hi there yWorks,

The browser crashes when I input an svg with arcs in svg2pdf. I'm running Chrome 55.0.2883.95 (64-bit). Here's a simple demo to illustrate: jsFiddle. Let me know if you have a fix for this.

Best regards, Henrik Skar

[mwasiluk]

👍

[yGuy]

That's true and is due to the "// TODO" in line 104. at least breaking the while loop would stop the browser from crashing. Converting svg arcs to pdf arcs is the right fix, here. Right now arcs simply aren't supported. I don't currently have a fix for this, but we would welcome pull requests, since we also don't currently have the time, nor the need, for fixing this properly.

[yGuy]

http://stackoverflow.com/a/30279817/351836

[TorsteinHonsi]

Emergency fix for the crash. The while loop goes eternal because the length is -1. It is prevented by defining a length for the arc operators:

      var length = "zZ".indexOf(type) >= 0 ? 0 :
          "hHvV".indexOf(type) >= 0  ? 1 :
          "mMlLtT".indexOf(type) >= 0  ? 2 :
          "sSqQ".indexOf(type) >= 0  ? 4 :
          "cC".indexOf(type) >= 0  ? 6 :
          "aA".indexOf(type) >= 0 ? 7 : -1; // <== Add length for arcs

[TorsteinHonsi]

@yGuy Here's a project that converts the arc path segment to bezier curves with a nice function API that would probably fit our need perfectly: https://github.com/colinmeinke/svg-arc-to-cubic-bezier.

It comes with a liberal license that requires the copyright be added in all copies. How would it be to include this script in the svg2pdf.js file? Should we ask the author for a special permission? It would also have to be downgraded from ES6.

[TorsteinHonsi]

If we can use svg-arc-to-cubic-bezier, here's how to implement the arcToBezier function:

          case "a":
          case "A":
            var last = pathSegList[pathSegList.length - 1];
            var curves = arcToBezier({
              px: last.x,
              py: last.y,
              cx: coords[i + 5],
              cy: coords[i + 6],
              rx: coords[i + 0],
              ry: coords[i + 1],
              xAxisRotation: coords[i + 2],
              largeArcFlag: coords[i + 3],
              sweepFlag: coords[i + 4]
            });
            curves.forEach(function (curve, i) {
              pathSeg = curve;
              pathSeg.pathSegTypeAsLetter = 'C';
              // Push all except the last one
              if (i < curves.length - 1) {
                pathSegList.push(pathSeg);
              }
            });
            break;

The result looks like this:

[yGuy]

I think, we'll rather pull in https://github.com/fontello/svgpath as another dependency (MIT) and reuse this (and more) functionality from the other project. Just like we depend on jspdf (the fork), svg2pdf.js will then also have the dependency on svgpath - this simplifies the code we currently have a lot and should also make it more robust - svgpath looks like a very complete and robust implementation to me.

[TorsteinHonsi]

But if we can get the author's permission to use the lightweight contents of svg-arc-to-cubic-bezier inside the svg2pdf library, wouldn't it be better to have a self-contained solution instead of another dependency?

[yGuy]

I'm not so sure whether getting the permission from someone who just copied the code from another project and applied a different license is something that would be legally advisable. svgPath is the original source of the sources in svg-arg-to-cubic. And the original sources look a lot better to me, to - they are commented, and are es5 compatible - just their usage of greek letters is a bit annoying to me.

I think we have written similar code for another project some years ago - I'll see whether taking that code would work, too, and then we could contribute that code directly to svg2pdf - copying over the code from svgPath and properly crediting them would work, too, but in the sense of npm where every line of code is a separate module, pulling that dependency in and then possibly baking it into svg2pdf seems like the easier and more robust solution.

[TorsteinHonsi]

Allright, I'll leave it up to you to decide. If you go for a dependency, we can make a bundle of pdfjs + rgbcolor + svg2pdf + others and serve it from our CDN, for the convenience of our users.

[TorsteinHonsi]

How do we proceed? Should we create a PR where we depend on svgPath, or do you want to do a more thorough rewrite? Or we can do both, just fix this issue for now, and you can do the rewrite over time?

[yGuy]

I am actually just working on it - I'll keep you updated or maybe will just publish 1.0.0 (it's incompatible from the file layout point of view - the api will be the same) later today.

[yGuy]

Please check the version I just published - note that the file now is in dist/svg2pdf.min.js and has rgbcolor and svgpath bundled.

[TorsteinHonsi]

Thanks, it seems to work perfectly!

What I do miss is license headers. In the source file they are a bit buried, and in the minified file they are completely removed. Since the built file now contains three different projects I think the license header should mention that, and each project's license conditions. Or perhaps the minified license section can just point to the LICENCE file, where also the dependencies are listed?

Also, a mention of the version in the license header would be nice, but not crucial.

[henrikskar]

Epic! Thanks, yGuy!