Skip to content

Conversation

@gjtorikian
Copy link
Contributor

Closes #264.

Similar to #256, the trick here was to look at the RST generation code and figure out how to override the method to do what we need it to do.

We're plucking attributes off the object and applying them to the image. These attributes might seem somewhat arbitrary at first:

* src
* alt
* width
* height

The original code also calls for style, class, align, etc., but we strip these out with sanitization anyway. Edit: this is no longer true, see below.

/cc @bkeepers @github/user-content @github/prose Which team is best pinged for these sorts of issues?

@ymendel
Copy link
Contributor

ymendel commented Apr 12, 2014

Are we showing SVGs at all? I thought we blocked those for security reasons.

@gjtorikian
Copy link
Contributor Author

@ymendel Oh we most certainly are showing them: https://github.com/gjtorikian/testing/blob/master/images.rst

Travis and other services provide the SVG option alongside PNG. We're just blocking the object tag for SVG, not img.

N.B. In 7a9da18 I just let all the attributes get applied to img. The sanitizer can sort out what to do with them.

@ymendel
Copy link
Contributor

ymendel commented Apr 12, 2014

@gjtorikian: Ah, I've not kept up with the conversation, apparently. 🆒

I think applying all the attributes and letting the sanitizer deal with them is probably the right way to go.

@bkeepers
Copy link
Contributor

👍 👍 Looks great.

gjtorikian added a commit that referenced this pull request Apr 16, 2014
Swap RST's use of `object` for SVG to use `image`
@gjtorikian gjtorikian merged commit 6b0f5bc into master Apr 16, 2014
@gjtorikian gjtorikian deleted the fix-rst-dotdot branch April 16, 2014 05:30
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.

svg images in ReST documents omitted due to use of <object> tag filtering

4 participants