Footnotes exports incorrect semantic html

[diminishedprime]

Description

The generated html for footnotes has the <sup> tag outside of the <p> tag of the footnote. Phrasing Content such as <pre> tags should be inside a paragraph. Because the <sup> doesn't belong to the <p>, it ends up on its own line and requires additional css to make it display correctly.

I'm happy to tackle this in a Pull Request, but wanted to make sure there's agreement that this is a valid bug report first.

Expected Behavior

Markdown with footnotes generates html with the correct semantic behavior. The <sup> tag for the footnote number should be inside the <p> tag for the footnote content.

jsfiddle

Actual Behavior

Markdown with footnotes generates html with a <sup> as a direct child of a <div>

jsfiddle

Note: This html was copied directly out of the ./specs/footnotes.txt

[marcusklaas]

This seems sensible. The way to change this is to update the spec first, and then update the implementation to follow that spec.

The spec file that we're currently following seems to be based on hoedown, but I couldn't find a similar file in their repository. @notriddle, do you remember how our footnotes spec file came from?

[notriddle]

I really didn't base the generated HTML on anything else. I just wrote the spec file to provide an easy way to test, and document, the parser.

If you want to change the HTML output, it will obviously require a breaking change release, but it's probably a good idea. The current markup is quite bad, and I really don't remember why I designed it that way. Other than the fact that I spent far more time designing the syntax-to-be-parsed than I spent on the HTML output.

[marcusklaas]

Ok, thank you for your clarification @notriddle. We welcome pull requests to make this change, @diminishedprime :-)

[diminishedprime]

I took an initial look into this yesterday, but I think it might end up being a somewhat complex change.

Hope to take another stab at it this weekend, but I'm not sure it'll be possible without either modifying the parser to change the order it pushes footnote items, or turning the run iterator into a peek-able iterator.

[marcusklaas]

Hmm yes, it seems those are our options. Perhaps easiest would be to never generate a paragraph node for reference definitions, since they will always contain exactly one anyway. That's a larger breaking change than doing some peaking-ahead wizardry inside the renderer, but the peaking solutions seems less attractive from a technical perspective.

[notriddle]

Perhaps easiest would be to never generate a paragraph node for reference definitions, since they will always contain exactly one anyway.

But that's not true. Footnotes are allowed to contain more than one paragraph. You can see that they do both in the spec file, and in "markdown guide".

What the multimarkdown seems to do, based on babelmark results, is generate an <ol>.

[marcusklaas]

TIL. That's interesting. Perhaps the solution is to simply generate an ordered list always? That'd be good from a semantic standpoint, right? And if I'm not mistaken, it should be trivial to implement.

[krtab]

TIL. That's interesting. Perhaps the solution is to simply generate an ordered list always? That'd be good from a semantic standpoint, right? And if I'm not mistaken, it should be trivial to implement.

I'm not sure I get what your proposing: if you have footnotes definitions scattered in the document (which I think Commonmark allows), would you generate an ol for each footnote? As far as I can tell there is currently no concept of "the single footnote block of this documents, where all footnotes definitions go".

[ragona]

The strategy of generating a list seems common. I'm trying to get littlefoot.css working with zola, which relies on pulldown-cmark, and what I'm noticing is that the non-standard output here causes littlefoot to fail to identify the footnotes as such.

Would be in favor of standardization here.