Add name attribute to heading anchors, fixes #231

[kasbah]

This makes the anchor tags work by default by adding a name attribute.

[kasbah]

Happy to talk about how to make this an option rather than a default though it seems unobtrusive and more sensible to me. Not sure why GitHub don't do this as it would also provide a fallback when Javascript is disabled.

[kasbah]

It seems the name attribute in <a> is not HTML5, but it is supported by all major browsers: http://www.w3schools.com/tags/att_a_name.asp

[kasbah]

Seems relevant: markdown-it/markdown-it#28

[ashleygwilliams]

i'm not entirely sure that adding the name really gets us much here. i could be wrong (i'm usually a backend dev) but i think name is a bit of an antipattern at this point.

@kasbah what does name give you that you can't accomplish with the current behavior?

[kasbah]

On http://render-readmes.preview.kitnic.it/boards/github.com/JarrettR/USBvil/, for instance, it makes the deep links work by default without interfering with the id namespace.

I believe if this was used on npm, it would make the deep links work with javascript disabled.

I am not sure why it should be an anti-pattern. On markdown-it/markdown-it#28 they mention "dom clobbering" but, if I understand that term correctly (which I am really not sure of), it seems less worse than not working anchor links.

[revin]

Yeah it's a way to make things work without JavaScript. The prefixHeadingIds: false option predates my involvement with this project, but I'm pretty sure this use case is exactly what it's meant to cover.

[kasbah]

But prefixHeadingIds: false isn't really that useful because it totally takes over the id name space with whatever people feel like putting in their titles.

Would be happy to put this behind an option addNameAttribute which defaults to false. Personally though, I would set this to true by default because it gives a better experience out of the box and doesn't break anything (as far as I can tell).

Let me know, and I will implement it. I deployed my fork to https://kitnic.it and it works fine for me, but am anxious to upstream this.

[revin]

Hmm. I definitely understand where you're coming from, but since name isn't technically valid anymore, and since GitHub doesn't generate name attributes, I wonder if maybe the best thing is to just have projects that need this style of operation just use the cheerio DOM object to do this, something like (once #225 is merged and the generated DOM structure matches GitHub's):

var $ = marky(whatever)

$('a.deep-link').each(function() {
  var $el = $(this)
  $el.parent().attr('name', $el.attr('href').substring(1))
})

(or similar, I didn't test this)

@ashleygwilliams thoughts?

[kasbah]

That would be perfectly usable and it's a small thing to do a work-around on my end. I just have to vent though and say I don't really understand the deprecation of name when there is no viable (js free) alternative. I am betting something will be introduced in HTML6 that takes care of this.

[revin]

Interestingly, it looks like HTML5 does specify that name on <a> elements should be used as part of the fragment resolution algorithm, presumably for backwards compatibility (see https://w3c.github.io/html/browsers.html#an-indicated-part-of-the-document, step 6). However, name is nowhere on the list of valid attributes anywhere. Ah, the web 😛

[revin]

@kasbah Thanks for the PR for sure, but I think for now we'll hold off, given the standards & GH behavior and the availability of a workaround. We can definitely revisit this once we've hit feature parity with GH's rendering.

[kasbah]

No problem, awaiting merge of #225 and then I'll do the workaround on my end like you suggested. Thanks for your time!