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 HTML tag removal for footnotes #458

Closed
wants to merge 1 commit into from
Closed

Conversation

@scherr
Copy link
Contributor

@scherr scherr commented Oct 10, 2017

The existing RemoveHtmlTags converter does not handle footnotes:

doc = Kramdown::Document.new("Test[^1]\n\n[^1]: <script>alert(1);</script>")
Kramdown::Converter::RemoveHtmlTags.convert(doc.root, :remove_block_html_tags => true, :remove_span_html_tags => true)
Kramdown::Converter::Kramdown.convert(doc.root)
# => ["Test[^1]\n\n[^1]:\n    <script>alert(1);</script>\n\n", []]
Kramdown::Converter::Html.convert(doc.root)
# => ["<p>Test<sup id=\"fnref:1\"><a href=\"#fn:1\" class=\"footnote\">1</a></sup></p>\n\n<div class=\"footnotes\">\n  <ol>\n    <li id=\"fn:1\">\n      <script>alert(1);</script>\n\n      <p><a href=\"#fnref:1\" class=\"reversefootnote\">&#8617;</a></p>\n    </li>\n  </ol>\n</div>\n", []]

This PR attempts to address this. However, it's quite possible that there is a more sensible way to cover this.

@gettalong gettalong self-assigned this Oct 10, 2017
@gettalong
Copy link
Owner

@gettalong gettalong commented Oct 10, 2017

Thanks for the pull request - I will have some time this weekend to see if your change covers everything.

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 5, 2017

I have merged your commit and amended it to also return the correct value in case of el.type == :footnote. Will be in the next release.

@gettalong gettalong closed this Nov 5, 2017
@scherr
Copy link
Contributor Author

@scherr scherr commented Nov 5, 2017

Ah, great. I had missed that. 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants