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

Handle external images for SVGs in privacy plugin #7650

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

nejch
Copy link
Contributor

@nejch nejch commented Oct 29, 2024

Some users (like us) might have quirks around serving SVG images/logos, so we include them from remote locations, e.g. for licensing reasons etc.

Since we can't include them locally but some users would like to have offline builds, this PR adds support for loading remote images from SVGs in the privacy plugin. WDYT? See also some references below 🙇

🛠️ with ❤️ at Siemens

@nejch nejch marked this pull request as draft October 29, 2024 10:01
@nejch nejch force-pushed the feat/privacy-image-svg branch from cf63558 to c79ac7c Compare October 29, 2024 10:09
@nejch nejch marked this pull request as ready for review October 29, 2024 10:10
@squidfunk
Copy link
Owner

Thanks for the PR! Could you please provide a minimal reproduction, so we can understand what this PR is trying to solve exactly? Also, if no trouble, could you please include the other cases we already support, so we can estimate whether this is something we're happy to support? As mentioned, always a good idea to talk to us first to omit duplicate work, but I think this is something worth supporting and should go into master, once we know that it aligns with our goals 😅

@nejch
Copy link
Contributor Author

nejch commented Oct 29, 2024

Thanks for the PR! Could you please provide a minimal reproduction, so we can understand what this PR is trying to solve exactly? Also, if no trouble, could you please include the other cases we already support, so we can estimate whether this is something we're happy to support? As mentioned, always a good idea to talk to us first to omit duplicate work, but I think this is something worth supporting and should go into master, once we know that it aligns with our goals 😅

Ah sorry got a little ahead of myself, will provide a little repro project for both!

@squidfunk
Copy link
Owner

No worries! At some point in the future we'll have tests, that'll make it easier 😅

@nejch
Copy link
Contributor Author

nejch commented Oct 30, 2024

@squidfunk here's a mini repro: 9.5.42-external-svg-images-excluded-in-privacy-plugin.zip

You can see with the current version the background is still loaded from the external source, which won't work offline.

I just used https://www.w3schools.com/graphics/tryit.asp?filename=trysvg_image2 but I think you'll get the idea 🙂 I'll get to the other issue in a bit!

@squidfunk
Copy link
Owner

This one LGTM, thanks for the PR again!

@squidfunk squidfunk merged commit 7dc96f1 into squidfunk:master Oct 30, 2024
4 checks passed
@nejch nejch deleted the feat/privacy-image-svg branch October 30, 2024 13:32
@squidfunk
Copy link
Owner

Released as part of 9.5.43.

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