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

Same reference for multiple heading #18

Open
RadhiFadlillah opened this issue Jan 12, 2018 · 5 comments
Open

Same reference for multiple heading #18

RadhiFadlillah opened this issue Jan 12, 2018 · 5 comments
Labels

Comments

@RadhiFadlillah
Copy link

RadhiFadlillah commented Jan 12, 2018

For example, I have a markdown file like this :

# General

## General

### General

This package will render all headings with same reference :

<h1>
	<a name="general" class="anchor" href="#general" rel="nofollow" aria-hidden="true">
		<span class="octicon octicon-link"></span>
	</a>General
</h1>
<h2>
	<a name="general" class="anchor" href="#general" rel="nofollow" aria-hidden="true">
		<span class="octicon octicon-link"></span>
	</a>General
</h2>
<h3>
	<a name="general" class="anchor" href="#general" rel="nofollow" aria-hidden="true">
		<span class="octicon octicon-link"></span>
	</a>General
</h3>

While GitHub will render each heading with different reference :

<h1>
	<a id="user-content-general" class="anchor" href="#general" aria-hidden="true"></a>General
</h1>
<h2>
	<a id="user-content-general-1" class="anchor" href="#general-1" aria-hidden="true"></a>General
</h2>
<h3>
	<a id="user-content-general-2" class="anchor" href="#general-2" aria-hidden="true"></a>General
</h3>

By the way, name attribute in a is now obsolete in HTML5, so it might be a good idea to move the name attribute into id like GitHub did.

@RadhiFadlillah RadhiFadlillah changed the title Same href for multiple heading Same reference for multiple heading Jan 12, 2018
@dmitshur
Copy link
Member

dmitshur commented Jan 15, 2018

Thanks for reporting (with good reproduce steps). This is a valid bug. It's a (well-defined) subset of issue #11.

Resolving this is likely going to be somewhat involved. It will require keeping track of a rendering context with heading IDs that have already been used. This may or may not need modifications to be made to the blackfriday API.

This library currently uses blackfriday v1. Resolving this might (or might not, pending investigation) require updating (rewriting?) to blackfriday v2.

Unfortunately, this is a low a priority for me so I don't plan to get to working on this soon.

@dmitshur dmitshur added the bug label Jan 15, 2018
@RadhiFadlillah
Copy link
Author

RadhiFadlillah commented Jan 15, 2018

Hi @shurcooL, I've solved it yesterdays but forgot to update my issue. This can be resolved using existing blackfriday v1. Before I create a PR, I want to ask a question.

Right now, my fork render this markdown :

# General

## General

### General

Into this HTML :

<h1>
	<a id="general" class="anchor" href="#general" aria-hidden="true" rel="nofollow">
		<span class="octicon octicon-link"></span>
	</a>General
</h1>
<h2>
	<a id="general-1" class="anchor" href="#general-1" aria-hidden="true" rel="nofollow">
		<span class="octicon octicon-link"></span>
	</a>General
</h2>
<h3>
	<a id="general-2" class="anchor" href="#general-2" aria-hidden="true" rel="nofollow">
		<span class="octicon octicon-link"></span>
	</a>General
</h3>

My question is, should we add prefix user-content to id just like what GitHub did, or should we leave it as it is ?

@dmitshur
Copy link
Member

Nice!

My question is, should we add prefix user-content to id just like what GitHub did, or should we leave it as it is ?

That's a good question.

It's interesting that clicking on anchors in GitHub READMEs still takes you to the expected section, despite the "user-content-" prefix. E.g., this works:

https://github.com/shurcooL/github_flavored_markdown/blob/master/README.md#installation

Even though there's no element with id or name "installation", only "user-content-installation".

I've looked, and it works because of custom JavaScript on github.com that runs on click, hashchange, etc., events and takes the "user-content-" prefix into account. Without that custom JavaScript code, clicking on the anchors wouldn't work.

I don't want to require custom JavaScript to make the links clickable, so let's not include "user-content-" prefix for now. We can consider and discuss it in a separate issue.

@RadhiFadlillah
Copy link
Author

I've made a PR to fix this, so I will close it.

@dmitshur
Copy link
Member

On the contrary, the bug is still valid, so it's best to keep this issue open in order to track it.

A better time to close it is when the PR fixing the bug is merged.

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

No branches or pull requests

2 participants