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

notebook 7 does not render images in SVG, notebook 6 did. #7114

Closed
adl opened this issue Oct 18, 2023 · 7 comments
Closed

notebook 7 does not render images in SVG, notebook 6 did. #7114

adl opened this issue Oct 18, 2023 · 7 comments
Labels
bug status:Needs Triage Applied to issues that need triage

Comments

@adl
Copy link

adl commented Oct 18, 2023

Description

Here is an example notebook I used to teach model checking. The lift_display functions is returning a SVG file (built by GraphViz that contains links to other SVG files stored in a subdirectory. Each configuration of the lift in the displayed graph is represented by a separate SVG file. This allows those small pictures to be cached by the browser when we regenerate the graph with different values. This was working very well with Notebook 6.x: the main SVG was inlined into the page, and the <image href=...> tags inside that SVG were served by the notebook server.

notebook6

Nowadays, with notebook 7, I believe this has been broken in two ways:

  1. the SVG is no longer inlined as SVG but instead embedded as a base64-encoded IMG tag. That seems wrong because loading SVG with <img> will ignore links to external style sheets or external images.
  2. links such as href="cache/1234.svg" do not seem to be served by the notebook 7 server anymore. (Judging from how markdown links are rewritten) I believe those need to be rewriten as href="file/cache/1234.svg?_xsrf=sometoken.

notebook7

Reproduce

Here is a smaller reproducible example, assuming graphviz's dot command is installed:

!echo 'digraph "" { a[fillcolor=red,style=filled] }' | dot -Tsvg  > a.svg
from IPython.display import SVG
SVG('a.svg')
!echo 'digraph "" { x[image="a.svg", shape=none, label=""]; x -> x }' | dot -Tsvg  > b.svg
SVG('b.svg')

On Notebook 6 I get
short6

On Notebook 7 I get
short7

Obviously I would expect the last cell to contain the red ellipse.

For anyone without GraphViz, let me also attach a.svg and b.svg.

Context

Running Debian Unstable with Firefox 118.0.2.

The "notebook 6" I was using is 6.4.12 troubleshoot6.txt

The "notebook 7" I'm using is 7.0.4 troubleshoot7.txt

@adl adl added bug status:Needs Triage Applied to issues that need triage labels Oct 18, 2023
@adl
Copy link
Author

adl commented Oct 18, 2023

Also I just realized another unfortunate consequence of rendering SVG using <img>. Tooltips of individual SVG elements are not shown anymore. (On the first screenshot, I'm using a tooltip to show the values of a set of variable that represent the current state of the left.) This is even more important when the SVG is used to plot statistical data, and tooltips used to show where data is coming from.

@jtpio
Copy link
Member

jtpio commented Oct 18, 2023

Thanks @adl for the detailed report 👍

Do you know if this also happens in JupyterLab 4.0.7?

@adl
Copy link
Author

adl commented Oct 18, 2023

Yes, I first discovered this issue a long time ago when JupyterLab came out. Back then, SVG was still inserted inline (i.e., not using <img>) so the only problem was that the images linked from SVG were not served because of the new way to request files. At that time, that issue was the only reason for me to stick to the classic interface. I'm sorry I did not report it back then (I simply assumed JupyterLab was new, and that those quirks would be fixed over time). I can reproduce with JupyterLab 4.0.6.

Today, the fact that <img> is now used to show SVG images makes SVG even less useful (for me at least) in this new interface. From scanning other issues, I believe the switch to <img> was made to allow copy/pasting SVG images out of a notebook. But as far as I'm concerned, using <img> kills lots of features that make SVG attractive to me. Because of <img>:

  1. You can only copy/paste the full image, you can't copy/paste part of the text that is embedded into a SVG file. (For instance I'm using SVG to display graphs and would like to select and copy the label of some node: I can do that if SVG is inlined, not if its used as )
  2. You can't have SVG that include other SVGs (because <img> have to be self-contained)
  3. You can't have SVG that include external style-sheets (because <img> have to be self-contained)
  4. You can't have SVG with tooltips

By using HTML('file.svg') instead of SVG('file.svg') I can somehow circumvent the <img> issue. However the nested-SVG example I showed in my initial message is not fixed, because the embedded href needs to be tweaked to be served.

@jtpio
Copy link
Member

jtpio commented Oct 19, 2023

Thanks for checking.

Since Notebook 7 uses the same components as in JupyterLab, should we move this issue to https://github.com/jupyterlab/jupyterlab then?

@adl
Copy link
Author

adl commented Oct 19, 2023

I don't now much about the architecture of Notebook, so whatever makes sense for you works for me.

@JasonWeill
Copy link
Collaborator

Closing in favor of jupyterlab/jupyterlab#10464, which was opened on JupyterLab 3, but which has attracted new comments even after JupyterLab 4 was released. @adl thank you for your contribution!

@adl
Copy link
Author

adl commented Oct 25, 2023

I have read jupyterlab/jupyterlab#10464 but it seems to focus on embedding SVG into the notebook, while I'm precisely trying to achieve the opposite: I use SVG that refer to third-party files (shared style-sheets, or images) that cannot be embedded. These would require some rewriting of the href links so that the JupyterLab can serve those third-party files.

Should I copy the above examples in that other issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status:Needs Triage Applied to issues that need triage
Projects
None yet
Development

No branches or pull requests

3 participants