syntax-tree / hast-util-sanitize

utility to sanitize hast nodes
https://unifiedjs.com
MIT License
49 stars 20 forks source link

Github allows <li> without ancestors. #18

Closed snikitin-qtl closed 4 years ago

snikitin-qtl commented 4 years ago

Hello, we have found an issue in GitHub schema.

https://github.com/syntax-tree/hast-util-sanitize/blob/71283f0f639af239dfc4931dd2ca4d89c9541935/lib/github.json#L11-L14

It sanitizes the li tags without ul or ol ancestors. Github is allowing that. Should I make a PR to fix that? or there is a rationale to keep it this way?

wooorm commented 4 years ago

While you are correct that <li>a</li> on GitHub renders as a list item,

  1. Why would you want a <li> that is not in an <ol> or <ul>?
  2. GitHub does have code to remove <li> w/o <ol> / <ul> ancestor, which I based this schema on 🤔
snikitin-qtl commented 4 years ago

Well,

  1. I would not, honestly. But the other users tend to put those things inline in markdown without wrapping it to the <ul> or <ol>. This case is a true story based on real events.
  2. Yeah. I'm not sure why the behavior is like that on Github. VSCode extensions for Markdown preview behaving like Github as well. 🤷‍♂️

I believe in this case - consistency is the goal.

wooorm commented 4 years ago
<tbody>a</tbody>
<tfoot>b</tfoot>
<thead>c</thead>
<td>d</td>
<th>e</th>
<tr>f</tr>
<li>g</li>
<h7>h</h7>
<h8>i</h8>
<p>j</p>

Yields:

a b c d e f
  • g
  • h i

    j


    I also found that h7 and h8 are no longer supported. Are you interested in a PR that fixes li, and maybe also remove h7 and h8 too?

    snikitin-qtl commented 4 years ago

    Sure, let me do that.