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

Anchors in Heading don't work. #6324

Closed
ruoyu0088 opened this issue Aug 17, 2014 · 7 comments · Fixed by #6483
Closed

Anchors in Heading don't work. #6324

ruoyu0088 opened this issue Aug 17, 2014 · 7 comments · Fixed by #6483
Milestone

Comments

@ruoyu0088
Copy link

I recently updated to IPython 2.2, and the anchors in heading don't work any more. I debugged this, and found that the the HTML of the rendered heading is as

<h3 id="This-is-a-heading
">This is a heading<a href="#This-is-a-heading
" class="anchor-link">¶</a></h3>

there is a "\n" char at the end of the id and href attribute. I can reproduct this by some simplified code in the HeadingCell.prototype.render() function in textcell.js

$($.parseHTML(IPython.security.sanitize_html(marked.parser(marked.lexer("this is a heading"))))).text()

There is a \n char at the end of the result. To fix this I added an line of code that removes '\n' in hash.

hash = hash.replace(/\n/g, '');

@minrk minrk added this to the 2.3 milestone Aug 17, 2014
@minrk
Copy link
Member

minrk commented Aug 17, 2014

Might have been caused by an update to marked.js. A judiciously placed .trim() might be what we need.

@dsblank
Copy link
Contributor

dsblank commented Aug 17, 2014

Thanks for finding this! I was just trying to demo our Table of Contents extension, and it had stopped working.

@dsblank
Copy link
Contributor

dsblank commented Sep 14, 2014

As the talk of backporting and releasing a new 2.x release is being made, it would be great to get this fixed and out. I confirm that this is still an issue in master 3.0.0-dev. Looks like ruoyu0088 has a fix, so it shouldn't be too hard.

@Carreau
Copy link
Member

Carreau commented Sep 14, 2014

Backport are usually only critical bug fix. Not doing so might expose us at not beeing back ported on Debian. So I wouldn't be too optimistic on having this back ported once fixed.

Envoyé de mon iPhone

Le 14 sept. 2014 à 07:02, Doug Blank notifications@github.com a écrit :

As the talk of backporting and releasing a new 2.x release is being made, it would be great to get this fixed and out. I confirm that this is still an issue in master 3.0.0-dev. Looks like ruoyu0088 has a fix, so it shouldn't be too hard.


Reply to this email directly or view it on GitHub.

@dsblank
Copy link
Contributor

dsblank commented Sep 14, 2014

I contribute to lots of software that is distributed by debian, and I can assure you that a bugfix that:

  • fixes a regression (it did work in a previous version, and later versions broke it)
  • fixes internal links in a system that is used by a large number of people in a browser
  • is a one-line change
  • can arguably be declared "critical" by those of us that use things like "table of contents" with large notebooks
  • can have a test written for it
  • there is no workaround

would not risk your status as a stable project. I appreciate being conservative in changing things, but such a usability bug can make the system look very broken, because it is broken.

@Carreau
Copy link
Member

Carreau commented Sep 14, 2014

fixes a regression (it did work in a previous version, and later versions broke it)

My

@Carreau Carreau closed this as completed Sep 14, 2014
@Carreau Carreau reopened this Sep 14, 2014
@Carreau
Copy link
Member

Carreau commented Sep 14, 2014

Rhahh, sorry for previous post, try again:

Any way it's marked as 2.3 milestone, so I supposed it is accepted by the rest of the core team, so it will be done. I guess the limitting factor is someone fixing the bug and sending a PR :-)

jhamrick added a commit to jhamrick/ipython that referenced this issue Sep 16, 2014
minrk added a commit that referenced this issue Sep 16, 2014
Trim anchor link in heading cells, fixes #6324
takluyver added a commit that referenced this issue Sep 30, 2014
This should fix #6324, which was really irritating me 😃 I used `.trim()` as suggested by @minrk.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
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 a pull request may close this issue.

4 participants