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

IPython.display HTML is overwriting the target attribute in hyperlinks #6827

Closed
michaele4321 opened this issue Jul 15, 2019 · 6 comments · Fixed by #7215
Closed

IPython.display HTML is overwriting the target attribute in hyperlinks #6827

michaele4321 opened this issue Jul 15, 2019 · 6 comments · Fixed by #7215
Labels
good first issue pkg:rendermime status:resolved-locked

Comments

@michaele4321
Copy link

@michaele4321 michaele4321 commented Jul 15, 2019

Describe the bug
When using iPython.Display HTML in Jupyter Lab code, the target attribute in hyperlinks is being overwritten with "_self" or"_blank"

To Reproduce
Steps to reproduce the behavior:

  1. Open a new Jupyter Labs notebook with a Python3 kernel
  2. enter and run
from IPython.display import HTML, display
sampleHtml = '<a target="jupyter_docs" href = "https://jupyterlab.readthedocs.io/en/stable/">Jupyter Docs</a>'
display(HTML(sampleHtml))
  1. The following link is created
<a target="_blank" href="https://jupyterlab.readthedocs.io/en/stable/" rel="noopener">Jupyter Docs</a>

Expected behavior
The link should be

<a target="jupyter_docs" href="https://jupyterlab.readthedocs.io/en/stable/" rel="noopener">Jupyter Docs</a>

Desktop (please complete the following information):

  • OS: macOS Sierra 10.12
  • Browser chrome 75.0
  • JupyterLab 1.0.0

Additional context
I'm not familiar enough with the Jupyter Lab code architecture to debug this but there is code in packages/rendermime/src/renderers.ts which looks like it might cause this problem.

if (isLocal) {
        el.target = '_self';
      } else {
        el.target = '_blank';
        el.rel = 'noopener';
      }

The HTML spec specifies that target allows values that are valid browsing context names or keywords which in practice means _blank, _self, _parent, _top, or anything else that does not start with a _.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jul 20, 2019

Hi @michaele4321, the adding of _blank to links is deliberate (as you note by linking to the code). We want external links to open in a new tab. May I ask the reason for adding a browsing context manually?

That said, I think it is reasonable for us to not override the target if the user has explicitly added the attribute to a link. I think adding that check would be a reasonable fix for a new contributor. One would go to this part of the code where we add the _blank target, and check to see if the element already has a target attribute:

if (isLocal) {
el.target = '_self';
} else {
el.target = '_blank';
el.rel = 'noopener';
}

One would then not override the target attribute if it was already specified.

@michaele4321
Copy link
Author

@michaele4321 michaele4321 commented Jul 22, 2019

Hi @ian-r-rose, thanks for looking at this. The context is that I am generating an HTML table of data with each row linked to an external page. I want to use a named target so that each link opens in the same external page (in my case on a second screen) rather than opening multiple new tabs.

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jul 23, 2019

Interesting, thanks for sharing @michaele4321!

@kavishdahekar
Copy link

@kavishdahekar kavishdahekar commented Sep 14, 2019

I'd like to take this issue up.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 14, 2019

Great, thanks @kavishdahekar!

@kavishdahekar
Copy link

@kavishdahekar kavishdahekar commented Sep 16, 2019

Unable to add reviewers for PR #7215 .
@ian-r-rose , @blink1073 could any of you please take a look? Thanks.

blink1073 pushed a commit that referenced this issue Sep 16, 2019
@lock lock bot added the status:resolved-locked label Oct 16, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue pkg:rendermime status:resolved-locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants