Heading names in toc_tokens contain stashed HTML placeholders

[jimporter]

If a Markdown heading contains HTML, the corresponding entry in the .toc_tokens property ends up with HTML placeholders when returned to the user. The following example should illustrate the problem:

>>> import markdown
>>> md = markdown.Markdown(extensions=['toc'])
>>> md.convert('# <code>Heading</code>\n')
'<h1 id="heading"><code>Heading</code></h1>'
>>> md.toc_tokens
[{'level': 1, 'id': 'heading', 'name': '\x02wzxhzdk:0\x03Heading\x02wzxhzdk:1\x03', 'children': []}]

While this isn't too hard to fix (we could just un-stash the HTML immediately before returning it to the user), it does raise a bigger question: what should the data format of the name field in toc_tokens be? Is it...

• Markdown (so the value would be exactly as in the source file: <code>Heading</code>)
• Plain text (so the value would strip HTML: Heading)
• HTML (similar to the Markdown format, but with HTML entities replaced, so <code>a>b</code> becomes <code>a&gt;b</code>)

In particular, this is relevant for mkdocs/mkdocs#1970. Prior to that PR, MkDocs would build an internal representation of the TOC by parsing the HTML from .toc. With the change, it (tries to) use .toc_tokens, but fails due to this issue.

FWIW, I think MkDocs wants this to be plain text in the end, but HTML makes the most sense to me in general: after all, the purpose of this lib is to convert Markdown to HTML. (MkDocs would then just need to parse the HTML fragment and strip out the tags.)

[waylan]

I'm of two minds about this. I agree that HTML provides the most flexibility. After all this feature is intended to give the user control over how the raw data is used.

On the other hand, I expect in most cases, plain text will be fine and requiring the user to strip the plain text from HTML is less than ideal. For example, in the MkDocs case, this could require a template to do the conversion, which is not something template authors generally want to deal with. Of course, MkDocs could do the conversion before passing it to the template, but do we want to require all users to do that step?

One alternative which occurs to me is to store both the plain text and the HTML as two different keys in the dict. For example: {... 'text': 'Heading', 'html': '<code>Heading</code>' ...}. But then what do we do with the name keyword (I don't recall why I named it that)? Although we could work off of name and have the HTML called complex_name or formatted_name or something. Not sure I like that though.

[jimporter]

After trying to fix this to just emit the HTML version, I think having both fields probably makes the most sense. My current patch in mkdocs/mkdocs#1970 strips the HTML tags on the MkDocs side, but it's a lot of boilerplate code for something that's probably fairly common.

For naming, maybe name and html_name would make sense? If we expect that most people want the HTML-stripped name, then having that be the "default" makes sense to me.

[waylan]

I took a look at the code for this:

markdown/markdown/extensions/toc.py

Lines 252 to 263 in 6651746

	text = ''.join(el.itertext()).strip()
	# Do not override pre-existing ids
	if "id" not in el.attrib:
	innertext = unescape(stashedHTML2text(text, self.md))
	el.attrib["id"] = unique(self.slugify(innertext, self.sep), used_ids)
	toc_tokens.append({
	'level': int(el.tag[-1]),
	'id': el.attrib["id"],
	'name': el.attrib.get('data-toc-label', text)
	})

The calls to stashedHTML2text and unescape just need to be moved from line 256 up to line 252 (probably, needs testing). Note that we are already striping out all HTML from the Markdown generated content of the header. It appears that we forgot to account for raw HTML in the header. In other words the header # foo *bar* results in 'name': 'foo bar', not 'name': 'foo <em>bar</em>'. Raw HTML shouldn't be any different.

I think that adding an html key (or whatever we call it) would be an additional add-on and not part of any bugfix. Unless someone wants to do the work, I'm okay with addressing the bugfix only.

[jimporter]

The calls to stashedHTML2text and unescape just need to be moved from line 256 up to line 252 (probably, needs testing).

Hmm. This does change the output of the HTML version of the TOC (in .toc) though: currently, it's passed through as-is, so you can use HTML to add formatting to your TOC items. Maybe it's ok to change that though. I'm actually surprised that Markdown forwarding in a header gets stripped from the TOC entry, but I don't think that's worth worrying about at the moment...

[waylan]

I'm actually surprised that Markdown forwarding in a header gets stripped from the TOC entry

That is intentional behavior. The fact that raw HTML gets through is an oversight. We intended to strip everything and only pass plain text through.

That said, there is an argument to be made for allowing users to format their TOC entries. And, in fact, we have had requests for it. But that would be a separate issue from this. Although, if that issue was addressed, it might fix this issue in the process.

[jimporter]

Gotcha. I think it makes sense to just strip all the HTML then, at least for now. Supporting HTML is tricky, since you'd still want to strip <a> tags so as to avoid conflicting with the <a> tags that the TOC adds. (There might be other HTML tags we'd want to strip too.)