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
646 stars 96 forks source link

testString appended to body twice as svg text on doc.svg() #217

Closed BBX999 closed 11 months ago

BBX999 commented 2 years ago

I'm using 2.2.0 on macOS Monterey 12.0.1 and Chrome Version 103.0.5060.53 (Official Build) (x86_64)

I'm hoping this description is strange and specific enough that someone who works on the code may quickly be able to figure out. On my end I've narrowed it down to the call in my code to doc.svg(svgElement, { x, y, width, height}), when that is present, after the pdf downloads, I get two of these svg elements appended to the html body:

abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789!"$%&/()=?'\+*-_.:,;^}][{#~|<>

In searching this repo, I saw that "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ 0123456789!"$%&/()=?'+*-_.:,;^}][{#~|<>" shows up as a private variable called "testString" here:

https://github.com/yWorks/svg2pdf.js/blob/d6dd5f105df37a6a4593dcddca6d22216a1a570d/src/context/textmeasure.ts#L17

Upon a quick review of the code there, it seems this string is used to check if the canvas measurement is accurate enough, and perhaps the current appending of svg elements is intended behavior, as they are both set to "visibility: hidden", however, in my case they are creating a scrollbar and excess whitespace at the bottom of the page.

Upon further review I see:

https://github.com/yWorks/svg2pdf.js/blob/d6dd5f105df37a6a4593dcddca6d22216a1a570d/src/context/textmeasure.ts#L67

is where the svg is appended, so it seems intentional. Is it possible to have the svg removed/cleaned up at the end of the measurement process?

Thanks!

yGuy commented 2 years ago

Interesting... thanks for the report. Actually that element is not supposed to be there, anymore, after the export:

https://github.com/yWorks/svg2pdf.js/blob/d6dd5f105df37a6a4593dcddca6d22216a1a570d/src/svg2pdf.ts#L53 https://github.com/yWorks/svg2pdf.js/blob/d6dd5f105df37a6a4593dcddca6d22216a1a570d/src/context/textmeasure.ts#L147

BBX999 commented 2 years ago

Thanks for clarifying that it is meant to be cleaned up, so there may be a bug here.

One thing I note from the cleanup code is that it seems to indirectly target the svg element by getting it as the parent node of the text measuring element. Perhaps that assumption for some reason isn't holding true as expected, and perhaps it might be better to just save a reference to the svg element itself and remove it directly.