Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SVG Mime Type #443

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add SVG Mime Type #443

wants to merge 1 commit into from

Conversation

TheMDev
Copy link

@TheMDev TheMDev commented Jan 22, 2025

When attempting to add SVG images to connectors and cables, they failed to show up even though the base64 content was correct in the HTML and SVG outputs.
Checking https://www.w3.org/TR/SVG11/intro.html reveals the mime type should be image/svg+xml, not image/svg.
As the code already handles mime subtype replacements for jpeg and tiff, adding svg here made for a simple fix.

@kvid
Copy link
Collaborator

kvid commented Feb 8, 2025

Thank you for detecting and suggesting a fix for this bug. I tried your fix and I can verify that the HTML output looked correct, but I noticed that the SVG image for a connector is still not shown in the PNG output. I understand that such a problem is outside the scope of this PR, but I wonder if this is also observed by others. JPG, PNG, and GIF images in connectors seems to work fine, but a TIF image was rejected by Graphviz. It would be nice to create a test use case with all image types expected to work, and perhaps a separate issue for unexpected outputs for such a test use case.

The automatic checks are currently failing due to #441, that now should be fixed for PRs with dev as target, but I probably need to cherry-pick a commit into master to also fix PRs with master as target.

@kvid
Copy link
Collaborator

kvid commented Feb 15, 2025

The commit mentioned above is now cherry-picked into master, but the automatc checks seem not to be re-executed automatically. Maybe we need to rebase this PR branch on top of the latest master (there should not be any conflicts) to re-execute the latest versions of these automatc checks?

@TheMDev - do you want to try doing this yourself, or should I do it?

@TheMDev
Copy link
Author

TheMDev commented Feb 17, 2025

I'm sorry for not getting back to you sooner. I cannot replicate your issue where an SVG image does not appear in a PNG output, but I did notice that the PNG output does not follow the same scaling as the HTML and SVG outputs. It seems this is because it uses a different render function that would likely require a rewrite. A more straightforward solution is to convert the SVG output to PNG at export. If you can provide more detailed information on the issue you noticed, I would be more than happy to look into it in this PR, but the scaling issue warrants a separate issue with more discussion on how to resolve it.

Ill rebase my branch onto master here shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants