Repository: go
Issue: proposal: html/template: allow output of some html comments
Labels: Proposal

<!--
Our proposal process is documented here:
https://go.dev/s/proposal-process
-->

# Background

The `html/template` package completely strips HTML comments from the rendered output. This is apparently intentional (#14256) but is not documented (#28628). The best summation of the rationale I could find comes from [the mailing list](https://groups.google.com/g/golang-nuts/c/8y6by6SERyU/m/IfBSc2k0wC0J):

> That would not be safe. html/template needs to know the proper context at each point of the template evaluation, but these types of comments cause their contents to be in a browser-dependent context, so the autoescaping can not be done reliably.

To wit, "these types of comments" seems to refer to old-school conditional comments like `<!--[if lt IE 9]>...<![endif]-->`. I don't know if these comments even do anything anymore on modern browsers, and a quick Google search wasn't very revealing.

The canonical solution to this problem seems to be to use the `template.HTML` type to wrap "trusted" strings. For example, there's [this StackOverflow answer](https://stackoverflow.com/a/34351058/814422) recommending this approach. However, this means that only way to use comments literally in the template is via function-call indirection, e.g.:

```
{{ safe "<!-- I just wanted to put a comment in my template -->" }}
```

This also imposes the limitation that comments cannot have injected values within them.

# Rationale for Change

I contend that based upon my own and a number of others' experiences, this behavior is unexpected. First of all, there are uses for comments besides affecting the browser's behavior (e.g. including debugging info useful to the developer). Second, many will have never seen or used IE "conditional comments" in no small part because IE is (almost) dead. Third, if the rationale for this behavior is to address the specific concern of conditional context confounding the auto-escaping mechanism, then I would think that only those comments which raise that issue would be stripped, not all comments. Fourth, of course, is the total lack of documentation of this behavior in the package itself.

There is another proposed "solution" already which is to fall back to using `text/template` but that is obviously a dangerous regression as then one gives up on all the benefits of `html/template`.

Though server-side manipulation of HTML is falling by the wayside in favor of script-heavy web apps doing extensive DOM manipulation client-side, the `html/template` package is not deprecated, and all browsers still (mostly) support "Web 1.0" sites that have minimal or no scripts.

Finally, the package documentation *does say* the following:

> The security model used by this package assumes that template authors are trusted, while Execute's data parameter is not.

Whereas the policy of stripping comments seems to contradict this maxim.

Thus I believe this unintuitive behavior is ripe for change, albeit ideally in a backwards-compatible way.

# Possible Paths

1. Add a new built-in function to the template language: Basically take the SO answer and bless it as canonical, eliminating the need for the user to define a function on their own. This is probably the most conservative approach, especially if the user is allowed to override the built-in function with one of their own (since otherwise there could be a naming conflict from old code), however I also consider this the least beneficial solution, since you still can't safely inject values into comments.
    - An improved approach would be to add a `comment` function that takes a string argument, escapes it properly for the body of a comment, surrounds it with comment markup, and finally wraps the result in `template.HTML`, this way you could at least inject values safely but indirectly with `printf`
2. Just stop stripping comments altogether. If there's a literal comment in the source, pass it through, injecting properly escaped values if there are any. Put a tombstone on IE while you're at it. This is probably the most aggressive approach and could have thorny repercussions if turned on by default.
3. Fail to compile any template containing HTML comments. The author probably put the comments there on purpose, and may not even be aware that they're being stripped. The template language has its own comment syntax which can be used for local-only comments. This replaces one unexpected outcome (comments just disappear) with another (some templates that used to compile now don't) but IMO reduces the amount of surprise involved.
4. Narrow the scope of the comment stripping to only those comments known to be problematic. I don't know if there are other kinds of problematic comments besides the aforementioned conditional comments from IE.
5. Introduce a new block to the template language inside of which comments are allowed. This improves on idea 1 by allowing safely escaped value injection.

Any of the above suggestions could also be gated, e.g. with [`template.Option`](https://pkg.go.dev/html/template#Template.Option), to improve backwards compatibility.