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.

Sign up for GitHub

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

Jump to bottom

clean_html: allow SVG tags and SVG attributes #1890

Merged
merged 2 commits into from
Oct 27, 2022
Merged

clean_html: allow SVG tags and SVG attributes #1890

merged 2 commits into from
Oct 27, 2022

Conversation

akx
Copy link
Contributor

@akx akx commented Oct 26, 2022

Fixes #1849
Refs #1854

@akx akx mentioned this pull request Oct 26, 2022
Closed
@akx
Copy link
Contributor Author

akx commented Oct 27, 2022

@blink1073 How can I make enforce-label pass? I can't set labels on the PR by hand...

@blink1073 blink1073 added the bug label Oct 27, 2022
@blink1073
Copy link
Contributor

I added the tag. Thanks for working on this again!

@akx akx force-pushed the bleach-svg branch 2 times, most recently from b16b0a2 to 93a1ea4 Compare October 27, 2022 14:13
@akx akx marked this pull request as draft October 27, 2022 14:14
@akx
Copy link
Contributor Author

akx commented Oct 27, 2022

Ah, I'll still need to see that xlink:... gets correctly kept.

@akx
Copy link
Contributor Author

akx commented Oct 27, 2022

Ah, darnit. bleach apparently happily cleans out xlink:href to href, which will probably break embedded SVGs... mozilla/bleach#362

@akx akx marked this pull request as ready for review October 27, 2022 14:30
@akx
Copy link
Contributor Author

akx commented Oct 27, 2022 •
edited
Loading

@blink1073 Right, this PR will allow a certain subset of SVG embedded into HTML.

The templates for actual image/svg+xml cells (e.g.

nbconvert/share/templates/lab/base.html.j2

Line 166 in a806744

{{ output.data['image/svg+xml'].encode("utf-8") | clean_html }}
) should use an XML-savvy cleaner since a image/svg+xml is not HTML to begin with.

This PR is nevertheless an improvement to the status quo... What do you think?

blink1073
blink1073 approved these changes Oct 27, 2022
View reviewed changes
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@blink1073 blink1073 merged commit b91c7a5 into jupyter:main Oct 27, 2022
@SylvainCorlay
Copy link
Member

@martinRenou FYI.

@akx akx mentioned this pull request Oct 28, 2022
Closed
jstorrs added a commit to jstorrs/nbconvert that referenced this pull request Jul 6, 2023
@jstorrs
Revert "clean_html: allow SVG tags and SVG attributes (jupyter#1890)"
1ff15ce 
This reverts commit b91c7a5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blink1073 blink1073 blink1073 approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect conversion of matplotlib SVG plots
3 participants
@akx @blink1073 @SylvainCorlay