Ionaru / easy-markdown-editor

EasyMDE: A simple, beautiful, and embeddable JavaScript Markdown editor. Delightful editing for beginners and experts alike. Features built-in autosaving and spell checking.
https://stackblitz.com/edit/easymde
MIT License
2.39k stars 316 forks source link

Incorrect color highlighting: editor believes it's an HTML tag, while renderer correctly detects it's not #234

Open dalboris opened 4 years ago

dalboris commented 4 years ago

First of all, thanks for the great editor!

The bug is: if we write a+b<c in the editor, it looks like this:

image

Notice how the <c is highlighted in green in the left side. In other words, the editor (on the left side) interprets the LESS-THAN SIGN as the beginning of an HTML tag, and color-highlights it as such. But the markdown renderer is actually smarter and understands that it's not the beginning of a tag (since it is invalid), and therefore converts it to &lt; as expected.

The expected behavior is that the <c is colored normally in black in the editor, like in the renderer. Only HTML tags which are considered valid by the renderer should be color-highlighted in the editor. Well, this can't work 100% in case a custom sanitizer is provided, but at least it should work with the default renderer and default sanitizer.

Alternatively, it'd be nice to have an option to disable HTML completely, in which case every single < and > are converted to &lt; and &gt; (also mentioned here: https://github.com/Ionaru/easy-markdown-editor/issues/154)

Version information

Ionaru commented 3 years ago

It looks like the issue is in CodeMirror, if you enter the same text on https://codemirror.net/mode/markdown/ it has the same result.

dalboris commented 3 years ago

Yes, I think you're right. I just had a 10min look out of curiosity, the bug might be somewhere here:

https://github.com/codemirror/CodeMirror/blob/1cb6de23c7e2b965201972ac5c6dcd2317e9eacf/mode/markdown/markdown.js#L549

    if (modeCfg.xml && ch === '<' && stream.match(/^(!--|\?|!\[CDATA\[|[a-z][a-z0-9-]*(?:\s+[a-z_:.\-]+(?:\s*=\s*[^>]+)?)*\s*(?:>|$))/i, false)) {
      var end = stream.string.indexOf(">", stream.pos);
      if (end != -1) {
        var atts = stream.string.substring(stream.start, end);
        if (/markdown\s*=\s*('|"){0,1}1('|"){0,1}/.test(atts)) state.md_inside = true;
      }
      stream.backUp(1);
      state.htmlState = CodeMirror.startState(htmlMode);
      return switchBlock(stream, state, htmlBlock);
    }

There is some code that checks whether the end of the stream is reached before the closing tag > is found, but then state.htmlState = CodeMirror.startState(htmlMode); is set regardless, even if > is not found. I'm not familiar with the code base but at a first glance this looks potentially bogus. I'm just sharing this in case someone have more time to investigate, those 10min were all I had for now for this quite low-priority issue.