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

Some svg strings that work on the browser do not work with st.image #1957

Closed
karriebear opened this issue Sep 2, 2020 · 10 comments
Closed
Labels
type:bug Something isn't working

Comments

@karriebear
Copy link
Contributor

karriebear commented Sep 2, 2020

Example 1:

Input:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="100" height="100">
    <clipPath id="clipCircle">
        <circle r="25" cx="25" cy="25"/>
    </clipPath>
    <image href="https://avatars.githubusercontent.com/karriebear" width="50" height="50" clip-path="url(#clipCircle)"/>
</svg>

Output:
image

Expected:
image

Example 2:

Input

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="500" height="100">
<text x="0" y="50">"I am a quote" - https://avatars.githubusercontent.com/karriebear</text>
</svg>

Expected:
image

@karriebear karriebear added type:bug Something isn't working status:needs-triage Has not been triaged by the Streamlit team labels Sep 2, 2020
@nthmost nthmost added media and removed status:needs-triage Has not been triaged by the Streamlit team labels Sep 5, 2020
@yashshah1
Copy link
Contributor

Hi, I'm new here but I'd like to work on this. Is this something I can take up?

@karriebear
Copy link
Contributor Author

Hi @yashshah1, you are more than welcome to take this on. Thank you for the help! We really appreciate it ☺️

Please let us know if you need any assistance. We'll be happy to take a look at any draft PRs too. Once you're ready, just submit a PR ready for review. Thanks again!

@yashshah1
Copy link
Contributor

Hi, before I raise a PR; I just wanted to update you on a few things.

The Problem:

I've isolated the problem down to image_to_url in elements/image_proto.py and down to this line which handles inline svg images:

return f"data:image/svg+xml,{image}"

What I think is going wrong

When passing images as inline-svgs, care needs to be taken for properly encoding them as URLs, which isn't done right now

Solution:

Modify the code to be :

from urllib.parse import quote
#inside url_to_image
return f"data:image/svg+xml,{quote(image)}"

Output Change:

For the input

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="100" height="100">
    <clipPath id="clipCircle">
        <circle r="25" cx="25" cy="25"/>
    </clipPath>
    <image xlink:href="https://avatars.githubusercontent.com/karriebear" width="50" height="50" clip-path="url(#clipCircle)"/>
</svg>

Earlier:

data:image/svg+xml,
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="100" height="100">
    <clipPath id="clipCircle">
        <circle r="25" cx="25" cy="25"/>
    </clipPath>
    <image xlink:href="https://avatars.githubusercontent.com/karriebear" width="50" height="50" clip-path="url(#clipCircle)"/>
</svg>

Note: I've also tried approaching this by only removing the newlines, but that doesn;t help

Now:

data:image/svg+xml,%0A%3Csvg%20xmlns%3D%22http%3A//www.w3.org/2000/svg%22%20xmlns%3Axlink%3D%22http%3A//www.w3.org/1999/xlink%22%20width%3D%22100%22%20height%3D%22100%22%3E%0A%20%20%20%20%3CclipPath%20id%3D%22clipCircle%22%3E%0A%20%20%20%20%20%20%20%20%3Ccircle%20r%3D%2225%22%20cx%3D%2225%22%20cy%3D%2225%22/%3E%0A%20%20%20%20%3C/clipPath%3E%0A%20%20%20%20%3Cimage%20xlink%3Ahref%3D%22https%3A//avatars.githubusercontent.com/karriebear%22%20width%3D%2250%22%20height%3D%2250%22%20clip-path%3D%22url%28%23clipCircle%29%22/%3E%0A%3C/svg%3E

Footnote(s)

  • Another thing that can be done is get rid of the newlines, spaces before passing the svg string to quote()
  • We could also pass safe='' to quote() which will urlencode even / characters, could lead to an even safer image.

If this is acceptable, I can raise a PR by EOD

@karriebear
Copy link
Contributor Author

Thanks for that analysis and everything you said makes sense! Also agree with both your footnotes.

  1. It looks like svg currently do not honor new lines for text so we might as well get rid of them.
  2. This is for urls specifically so I don't think it should be an issue. Safer definitely sounds better and a quick test of text elements looks like it should be fine (added as Example 2 above).

@yashshah1
Copy link
Contributor

@karriebear I've opened the PR

@karriebear
Copy link
Contributor Author

Awesome, thanks! We'll take a look!

@karriebear
Copy link
Contributor Author

Looks like this is problematic because we are currently putting a data uri into an image tag. Unfortunately image tag must be one image file where our example is importing image from elsewhere (source).

@exalate-issue-sync
Copy link

Naomi Most commented: Might be we could use JS file blobs to address this.

@exalate-issue-sync
Copy link

Naomi Most commented: {noformat} var imageUrl = urlCreator.createObjectURL(this.response);
document.querySelector("#image").src = imageUrl;{noformat}

@AnOctopus
Copy link
Contributor

With #2666 merged in, this should be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants