Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inserted <p> into <li> on different unordered lists #1461

Closed
ybiquitous opened this issue Apr 8, 2019 · 14 comments · Fixed by #1810
Closed

Inserted <p> into <li> on different unordered lists #1461

ybiquitous opened this issue Apr 8, 2019 · 14 comments · Fixed by #1810
Labels
category: lists L2 - annoying Similar to L1 - broken but there is a known workaround available for the issue released

Comments

@ybiquitous
Copy link

ybiquitous commented Apr 8, 2019

Describe the bug

When putting different two unordered lists (- and *) with blank lines, a <p> tag is inserted into <li> tag.

E.g.

- a1
- a2

* b1
* b2

To Reproduce

  1. Marked Demo
  2. CommonMark Demo

On terminal:

  1. Install marked: npm install --save marked@0.6.2
  2. Run marked:
$ cat <<EOF | ./node_modules/.bin/marked
- a1
- a2

* b1
* b2
EOF
  1. Actual output (or error) is:
<ul>
<li><p>a1</p>
</li>
<li><p>a2</p>
</li>
<li><p>b1</p>
</li>
<li><p>b2</p>
</li>
</ul>

Expected behavior

Like CommonMark, I think marked should output the following HTML:

<ul>
<li>a1</li>
<li>a2</li>
</ul>
<ul>
<li>b1</li>
<li>b2</li>
</ul>
@ybiquitous ybiquitous changed the title Inserted <p> in <li> on different unordered lists Inserted <p> into <li> on different unordered lists Apr 8, 2019
@UziTech UziTech added category: lists L2 - annoying Similar to L1 - broken but there is a known workaround available for the issue labels Apr 8, 2019
@UziTech
Copy link
Member

UziTech commented Apr 8, 2019

Adding an extra new line will make it render correctly but this is a bug.

@ybiquitous
Copy link
Author

I think so, too. 👍

@ansuz
Copy link

ansuz commented Jun 5, 2019

I played around with the parser and saw a suspicious looking line.

The patch below seems to fix this issue:

22c22
<   list: /^( *)(bull) [\s\S]+?(?:hr|def|\n{1,}(?! )(?!\1bull )\n*|\s*$)/,
---
>   list: /^( *)(bull) [\s\S]+?(?:hr|def|\n{2,}(?! )(?!\1bull )\n*|\s*$)/,

...but I don't know that it doesn't cause other problems which are potentially much worse.

If anyone can point me towards any tests I should run I can turn this into a PR pretty quickly.

@UziTech
Copy link
Member

UziTech commented Jun 5, 2019

Running npm test from your marked directory will run the tests.

Alternatively, if you submit a PR the tests will automatically run on travis-ci

ansuz pushed a commit to xwiki-labs/marked that referenced this issue Jun 5, 2019
@ansuz
Copy link

ansuz commented Jun 5, 2019

As my PR says, the fix causes all sorts of failures. If the PR isn't acceptable hopefully it at least serves as a good starting point for an investigation into the problem.

@UziTech
Copy link
Member

UziTech commented Jun 5, 2019

I don't think that is the right fix. It seems like the right fix should change the regex to distinguish between - and *

@ansuz
Copy link

ansuz commented Jun 11, 2019

@UziTech: as per your comment on my PR:

according to the original markdown spec:

If list items are separated by blank lines, Markdown will wrap the items in <p> tags in the HTML output.

This seems to be intended behaviour, which I guess invalidates your above comment:

Adding an extra new line will make it render correctly but this is a bug.

If that's the case, I suppose this issue should be marked as wontfix and closed?

@UziTech
Copy link
Member

UziTech commented Jun 11, 2019

The original issue uses two different list item styles (- and *) which should render as two separate lists with a single blank line between them. Your test only uses a single list item style (*) which, as my comment states, should render a single loose list.

@UziTech UziTech mentioned this issue Nov 3, 2020
5 tasks
@github-actions
Copy link

github-actions bot commented Nov 4, 2020

🎉 This issue has been resolved in version 1.2.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@yannicklescure
Copy link

yannicklescure commented May 14, 2022

Hi, I'm using version "4.0.15" along with NodeJS. The bug is still present (or is back).
Edit: I'm fixing the issue by using this dirty code

const markdown = marked.parse(data).replaceAll('</p>\n</li>', '</li>').replaceAll('<li><p>', '<li>');

@calculuschild
Copy link
Contributor

calculuschild commented May 14, 2022

@yannicklescure Can you give us an example input and output showing yhe problem? We can add it to our test suite and reopen this issue.

@yannicklescure
Copy link

yannicklescure commented May 14, 2022

@calculuschild I did further test trying to narrow the issue. Here is what I have tried.

- a1
- a2

* b1
* b2

1. c1
2. c2

* d1

* d2

* e1

* e2

1. f1

2. f2

- g1

The serie a, b, c gives the expected result i.e.: <li>a1</li>
The serie d, e, f gives the list of elements adding p inside li. Notice the added space between the lines. i.e.: <li><p>a1</p></li>
g1 works fine.

@UziTech
Copy link
Member

UziTech commented May 15, 2022

That is called a loose list and that is intended behavior that follows the CommonMark spec.

@yannicklescure
Copy link

@UziTech Thank you, I learn something new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: lists L2 - annoying Similar to L1 - broken but there is a known workaround available for the issue released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants