Fix HTML tags appearing in wiki table of contents

[heathdutton]

Fixes #36106

When wiki headings contain HTML elements (like <a name="anchor"></a>), the raw HTML code was appearing verbatim in the table of contents instead of being stripped out.

Before: ToC displays <a name="asdf"></a> has strange html
After: ToC displays has strange html

---

Also fix #17958 by the way

[lunny]

Could you add some test?

[lunny]

Whether any a tag in the heading should be ignored or we could have an option to do that?

[heathdutton]

Good question! I went with stripping all HTML tags rather than just <a> specifically - the ToC should really just show plain text for navigation, so things like <span>, <strong>, etc. wouldn't make sense there either.

The heading itself still renders with the HTML in the document body, so anchor links like <a name="anchor"> still work fine for linking purposes.

I think adding an option would be overkill for this - can't think of a case where someone would actually want raw HTML showing up in their ToC. But happy to discuss if you see it differently!

[delvh]

can't think of a case where someone would actually want raw HTML showing up in their ToC

What about case <a> and <b>?

[delvh]

So, I think 'raw HTML' is useful when it is only accidentally HTML.
And that one can most certainly happen in titles

[heathdutton]

Good edge case to think about! I tested this and the fix handles it correctly:

## <a href="link">Click</a> and <b>Bold</b>

ToC shows: Click and Bold

The HTML tags get stripped but the text content inside them is preserved - which is exactly what we want for a readable ToC.

I've added test cases covering this scenario. Also verified that code spans like \`` are handled separately by the markdown parser and aren't affected.

[wxiaoguang]

I fixed the tests, it needs to clearly assert what we want.

And we can see that the result doesn't seem good.

By the way: no need to rebase or force push, see the contribution guideline https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#maintaining-open-prs

[heathdutton]

Roger that, I'll step back.

[wxiaoguang]

@silverwind this PR still has problems, see #36284 (comment), AI's review won't really help.

[silverwind]

Sorry, I was just going by "ready" PRs.

[wxiaoguang]

Well, it seems that this PR is unlikely to get progresses.

Then, it becomes my work, again.

[wxiaoguang]

Now, the result is what it should be.

[Copilot]

Pull request overview

This pull request fixes a bug where HTML tags in wiki headings were appearing verbatim in the table of contents instead of being stripped out. The fix refactors the ToC generation from an AST-based approach to an HTML-based approach, extracting plain text from heading nodes and properly escaping it during ToC rendering.

Changes:

• Refactored ToC generation to work with HTML nodes instead of Markdown AST nodes
• Added proper text extraction from heading elements that strips HTML tags while preserving text content
• Implemented HTML-safe ToC rendering using new htmlutil helper functions

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
web_src/js/markup/anchors.ts	Added FIXME comment expressing architectural opinion (non-functional)
routers/web/repo/wiki.go	Updated to use new ToC rendering approach with proper HTML handling
modules/markup/render.go	Added new types TocShowInSectionType and TocHeadingItem for ToC data structure
modules/markup/mdstripper/mdstripper.go	Removed WithAutoHeadingID() parser option as heading IDs are now handled in HTML post-processing
modules/markup/markdown/transform_heading.go	Deleted (replaced by HTML-based heading processing)
modules/markup/markdown/toc.go	Deleted (replaced by RenderTocHeadingItems in html.go)
modules/markup/markdown/prefixed_id.go	Deleted (ID prefixing now handled in HTML post-processing)
modules/markup/markdown/markdown.go	Removed custom prefix ID generator and AutoHeadingID parser option
modules/markup/markdown/goldmark.go	Updated to set ToC mode flags instead of generating AST nodes
modules/markup/html_toc_test.go	Added comprehensive test verifying HTML tags are stripped from ToC
modules/markup/html_node.go	Enhanced to extract heading text and populate ToC items during HTML post-processing
modules/markup/html.go	Added RenderTocHeadingItems function with proper HTML escaping
modules/markup/common/footnote.go	Updated footnote IDs to include "user-content-" prefix in renderer
modules/htmlutil/html.go	Added HTMLPrintf, HTMLPrint, and HTMLPrintTag helper functions for safe HTML output

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wxiaoguang]

Sorry, I was just going by "ready" PRs.

It is ready now. And maybe you can take a look at this FIXME later (in following up PRs) https://github.com/go-gitea/gitea/pull/36284/files#diff-a1ea66825d0703d3cd1da7b9428ad1efecce97e52fb828246cc2b7729283faa0R3

// FIXME: don't see why these tricks make sense. If these prefixes are not needed, they should be removed entirely by backend.

[silverwind]

// FIXME: don't see why these tricks make sense. If these prefixes are not needed, they should be removed entirely by backend.

The reason for these prefixes is so that markdown content is not in direct control of the id namespace of the page because that may lead to broken JS features or even vulnerabilities.

It would be better if backend sets these prefixes.

[wxiaoguang]

// FIXME: don't see why these tricks make sense. If these prefixes are not needed, they should be removed entirely by backend.

The reason for these prefixes is so that markdown content is not in direct control of the id namespace of the page because that may lead to broken JS features or even vulnerabilities.

It would be better if backend sets these prefixes.

But the frontend code just removes the user-content- prefix which was added by backend, so what's the purpose?

[silverwind]

The removal on href attributes (but not on id) is so that these prefixes are invisible to the user and their links README.md#a-b work. This behaviour matches GitHub exactly.

[wxiaoguang]

The removal on href attributes (but not on id) is so that these prefixes are invisible to the user and their links README.md#a-b work. This behaviour matches GitHub exactly.

It matches, if eventually these prefixes don't exist when users visit the page, why backend should add them?

// FIXME: don't see why these tricks make sense. If these prefixes are not needed, they should be removed entirely by backend.

[silverwind]

why backend should add them

because it's usually best to alter HTML in backend, but I see this mechanism with the removal requires JS to work, so it can not work with JS disabled.

While could move the addition of the prefix to frontend but then we give markdown documents access to the full id namespace, which is bad because it can breaks the page's link targets.

I see no better way. The need for prefixes is to prevent markdown from altering the full id namespace.

[wxiaoguang]

The need for prefixes is to prevent markdown from altering the full id namespace.

Sorry I don't understand.

If backend doesn't add these prefixes, isn't it the same as current approach for end users? The same as GitHub? So I don't see why "even vulnerabilities" #36284 (comment)

I don't see what is a "need for prefixes", what is the full "id" namespace. Can you show a real example?

[silverwind]

For example, repo homepage has #readme anchor, without the prefix a markdown document could generate the same anchor which results in invalid HTML because you can not have two same ids. When a user opens the page with that anchor, it's undefined behaviour to which anchor the browser would scroll to.

[wxiaoguang]

For example, repo homepage has #readme anchor, without the prefix a markdown document could generate the same anchor which results in invalid HTML because you can not have two same ids.

But you also always remove these prefixes, right? The same as GitHub?

Do you mean :

• GitHub has vulnerabilities? And they should add user-content- prefix?
• If no, why Gitea adds the prefix and then removes it?

[silverwind]

But you also removed these prefixes, right? The same as GitHub?

No, id is never altered by JS so the page stays without id conflicts at all times.

GitHub has vulnerabilities? And they should add user-content- prefix?

No, Github leaves the ids also unchanged in JS, so id uniqueness is guaranteed with or without JS.

If no, why Gitea adds the prefix and then removes it?

Both Github and Gitea only remove it from href, never from id.

[wxiaoguang]

If no, why Gitea adds the prefix and then removes it?

Both Github and Gitea only remove it from href, never from id.

Hmm ... thanks, I think I can understand more, so the comment can be updated to clarify the purpose of that part of code

[silverwind]

So in summary:

• backend adds prefix to guarantee unique ids on the page
• js strips prefix so users gets nice prefix-less links
• js intercepts the hash navigation to add the prefix so the correct prefixed id element is focused

[silverwind]

#36443

[wxiaoguang]

If no, why Gitea adds the prefix and then removes it?

Both Github and Gitea only remove it from href, never from id.

Hmm ... thanks, I think I can understand more, so the comment can be updated to clarify the purpose of that part of code

https://github.com/go-gitea/gitea/pull/36438/files#diff-a1ea66825d0703d3cd1da7b9428ad1efecce97e52fb828246cc2b7729283faa0

Fixed the fixme

---

Update: just found #36443

[silverwind]

I find my description in #36443 better, let's continue there.