Comments aren't fully supported

[xepozz]

Describe the bug
The last version (1.1.1) doesn't fully supports comments. See screenshots:
Github:


Marked:

Supported:

<!-- comment -->

Not supported:

<!-- comment --!>

To Reproduce
Steps to reproduce the behavior:

Expected behavior
Comments with --!> at the ending are supported.

[UziTech]

This actually looks like an error in CommonMark/GFM. According to the html block spec:

2. Start condition: line begins with the string <!--.
   End condition: line contains the string -->.

There is no end condition that includes --!>

[UziTech]

It doesn't seem like that is a valid html comment anyways:

https://developer.mozilla.org/en-US/docs/Web/API/Comment

[xepozz]

Yes, you are right, it's not declared in https://www.w3.org/TR/html51/syntax.html#sec-comments
Can we include this functionality only for gfm?

[UziTech]

I think I see the issue. The CommonMark spec says (emphasis mine):

It ends with the first subsequent line that meets a matching end condition, or the last line of the document

So with markdown like:

**a**
<!-- comment -->
**a**

CommonMark/GFM outputs (demo):

<p><strong>a</strong></p>
<!-- comment -->
<p><strong>a</strong></p>

because <!-- matches a start condition and --> matches an end condition

but markdown like:

**a**
<!-- comment --!>
**a**

CommonMark/GFM outputs (demo):

<p><strong>a</strong></p>
<!-- comment --!>
**a**

since <!-- matches a start condition and there is no end condition so the end of the document closes the html block.

The issue is that marked doesn't see an end condition and thinks the start condition should not be the start of an HTML block.

Browsers do allow comments to be ended with --!> probably for legacy reasons.

In any case we shouldn't include --!> as the end of a comment but fix marked to see the end of the document as an end condition for html blocks.

[UziTech]

Actually according to the html spec comments can't contain the string -- so

<!-- comment --!>
**a**

should not be a valid html comment since it contains the string --.

I'm not sure what to do here. It seems like marked does the correct thing according to the HTML spec but the wrong thing according to the CommonMark spec.

@styfle @joshbruce @davisjam any thoughts?

[joshbruce]

I don't believe any version of HTML required the --!> closing. See https://www.w3.org/TR/html4/intro/sgmltut.html#h-3.2.4

Looking at the following HTML 5.2 less technical description (8.1.6): https://www.w3.org/TR/html52/syntax.html#comments

I would think the following would be a thing (if ignoring HTML):

**a**
<!-- this should work -->
**a**

**a**
<!-- this should fail --!>
**a**

**a**
<!-- this should work for reasons of spec <!-->
**a**

<p><strong>a</strong></p>
<!-- this should work -->
<p><strong>a</strong></p>

<p><strong>a</strong></p>
<p><!-- this should fail --!></p>
<p><strong>a</strong></p>

<p><strong>a</strong></p>
<!-- this should work for reasons of spec <!-->
<p><strong>a</strong></p>

Marked:

<p><strong>a</strong></p>
<!-- this should work -->
<p><strong>a</strong></p>
<p><strong>a</strong></p>
<!-- this should fail --!>
**a**

**a**
<!-- this should work for reasons of spec <!-->
<p><strong>a</strong></p>

The demo page seems to render all as comments...changed various setting in the options and got the same result each time. [Update: Markded appears to be applying the opening and closing as expected. Think the issue might be in corner cases of when to fail based on the internal string and whether we should be pedantic in that regard.]

With that said, we may also want to consider specifying which HTML versions we aim to support, not just Markdown specs. Just in case something in the HTML spec does cause something to be wrong. [Update: And which trumps which - the Markdown Spec in question or the HTML spec we compile to.]

[joshbruce]

Also, the browser renders the **a** in the "should fail" comment, which is odd.

[UziTech]

I think we should try to follow the markdown spec. According to commonmark/commonmark-spec#662

Requiring each conforming commonmark parser to include an accurate HTML5 parser is overkill for our purposes.