yWorks / svg2pdf.js

A javascript-only SVG to PDF conversion utility that runs in the browser. Brought to you by yWorks - the diagramming experts
MIT License
654 stars 101 forks source link

Issue in PDF having Highcharts (multiple series) chart generated with svg2pdf.js #71

Closed VKB2612 closed 5 years ago

VKB2612 commented 5 years ago

In case of chart having multiple series (a line series and a column series), if column series is disabled and PDF is generated, the resulting PDF throws error when tried to open using Acrobat Reader. The same PDF does not show error if opened in browser though. Also note that the same issue does not occur if line series is disabled and output PDF has only column series.

svg2pdf.js - v1.3.3 jspdf - v2.0.0

Reproduce the error:

Refer JSFiddle: https://jsfiddle.net/xL2q75vr/1/ Steps to reproduce the behavior:

  1. Diable column series
  2. Click on Download button
  3. Open the PDF in Acrobat error -> (empty chart or Acrobat error)

Other details

HackbrettXXX commented 5 years ago

This is an issue with clip-paths on invisible groups.

VKB2612 commented 5 years ago

Hi, Thanks for the reply. Any advice on how to go further about it?

HackbrettXXX commented 5 years ago

I'm currently working in a fix.

yGuy commented 5 years ago

A workaround would obviously be for the end-user (highcharts) not to produce clip-paths on invisible groups. I don't believe they are necessary in any way. Why would you put a clip on something that is not visible?

Denyllon commented 5 years ago

@yGuy I'm not agree with your sentence. We might ask as well, why any attributes are set on hidden elements? Because it doesn't make sense to remove all attributes after hiding a DOM element, and restoring the same attributes when making that element visible again. They would have to be stored somehow/somewhere, so it won't be correct approach. The problem is definitely with that library, and its interpretation of SVG structure.

yGuy commented 5 years ago

I'm confused. What do you want? This is an open source project. You could have fixed it yourself or you could have helped fixing it. You could have sanitized the input as per my suggestion. Arguing that this is a problem in the library gets you nowhere. Sanitizing the input would have helped. We fixed the problem for HighCharts in a record-breaking time, yesterday, long before your rant for nothing and this is what we get as a "thank you"? Well, thanks for nothing! It's statements like these that make me reconsider publishing the library as an open source project on GitHub.

Denyllon commented 5 years ago

@yGuy Apologize, but I've just answer to your question, and didn't want to disappoint anyone, so don't understand your resentment at all. However, as you said above, the problem is already fixed (by commit on this library), so most apparently that was the the most accurate place to fix it.

We fixed the problem for HighCharts in a record-breaking time

Actually, the problem wasn't only with Highcharts, but with all SVG elements with clip-path exported by this library, so could you explain me why would we remove the clip-path attribute on every hide series action in Highcharts?

Kind regards!

VKB2612 commented 5 years ago

Hi, Thank you all for the support. I really appreciate the fastness with which the solution was provided. I tested the changes again and it works perfectly fine in all the browsers. Thanks again.

Regards