warpjs icon indicating copy to clipboard operation
warpjs copied to clipboard

Error handling failure with too few arguments in path command

Open bgmort opened this issue 4 years ago • 2 comments

There are two related issues here. One is a quick fix, and I'll send a pull request for it with tests. The second is more complicated, and I see that you've started a rewrite, so I'll just point it out for you to be aware of.

Apparently the A arc path's two flag arguments, which must be either 0 or 1, do not have to have any spaces after them. So, the following command: a 110 110 0 0 1 55 -7 could be written like this: a 110 110 0 0155-7. You'll get output like that from an optimizer like https://jakearchibald.github.io/svgomg/.

The current parser doesn't take that into account, and parses the latter command as having only 5 arguments. I couldn't find where it was described in the svg spec, but it's clearly widely supported as any SVG renderer I tried doesn't have a problem with it. <path d="M0 0a 100 100 0 00200-0z" /> would be a valid path to test.

Second, this would have been easier to diagnose, but the line of code that should spit out the error has a typo in it, so produces a reference error instead. I'll have a pull request ready to fix that shortly.

bgmort avatar Oct 23 '20 17:10 bgmort

@bgmort thanks for finding and fixing! There's a lot of corners I cut, particularly with parsing, that I look back on now and wonder what I was thinking. I've merged your PR and will put out a new release this weekend. Really appreciate that you took the time to surface the issue and put out a fix for it :)

I'm actually looking at using a proper parser for the path in the rewrite. Either by forking https://github.com/hughsk/svg-path-parser/blob/master/grammar.peg (which looks like it does parse arc commands correctly) or writing my own, not sure yet. But I'll ensure I support exactly what the spec defines in https://www.w3.org/TR/SVG/paths.html

benjamminf avatar Oct 23 '20 23:10 benjamminf

No problem, thank you for the library! I'm a developer that does my own graphic design sometimes, and was disappointed to find that Affinity Designer has no support for warping text, but I was able to solve my problem using warp.js.

bgmort avatar Oct 24 '20 00:10 bgmort