nhn / tui.editor

🍞📝 Markdown WYSIWYG Editor. GFM Standard + Chart & UML Extensible.
http://ui.toast.com/tui-editor
MIT License
17.2k stars 1.76k forks source link

Switching between markdown and WYSIWYG mode breaks image links #633

Closed AlbinoDrought closed 5 years ago

AlbinoDrought commented 5 years ago

Version

1.4.6

Test Environment

Firefox 67.0 on Linux

Current Behavior

1. In markdown mode, paste this sample image link:

[![github version](https://img.shields.io/github/release/nhn/tui.editor.svg)](https://github.com/nhn/tui.editor/releases/latest)

2. Switch to WYSIWYG mode

3. Switch back to markdown mode. The image link has been escaped and no longer works. It now looks something like this:

[!\[github version\]\(https://img.shields.io/github/release/nhn/tui.editor.svg\)](https://github.com/nhn/tui.editor/releases/latest)

Expected Behavior

Markdown for the image link should not change while switching back and forth between modes.

AlbinoDrought commented 5 years ago

A simple unit test that reproduces the issue:

      it('should not break image links', () => {
        const markdown = '[![github version](https://img.shields.io/github/release/nhn/tui.editor.svg)](https://github.com/nhn/tui.editor/releases/latest)';

        editor.changeMode('markdown', true);
        editor.setMarkdown(markdown);
        editor.changeMode('wysiwyg', true);
        editor.changeMode('markdown', true);
        expect(editor.getMarkdown()).toBe(markdown);
      });

Not sure which file would be best for it. I had it in tesdt/unit/editor.spec @ describe('changeMode()', () => {:

diff --git a/test/unit/editor.spec.js b/test/unit/editor.spec.js
index c9ae90e..2451717 100644
--- a/test/unit/editor.spec.js
+++ b/test/unit/editor.spec.js
@@ -121,6 +121,16 @@ describe('Editor', () => {
         expect(cursor.ch).toEqual(0);
         expect(container.contains(document.activeElement)).toEqual(false);
       });
+
+      it('should not break image links', () => {
+        const markdown = '[![github version](https://img.shields.io/github/release/nhn/tui.editor.svg)](https://github.com/nhn/tui.editor/releases/latest)';
+
+        editor.changeMode('markdown', true);
+        editor.setMarkdown(markdown);
+        editor.changeMode('wysiwyg', true);
+        editor.changeMode('markdown', true);
+        expect(editor.getMarkdown()).toBe(markdown);
+      });
     });

     describe('height(pixel)', () => {

Output:

HeadlessChrome 0.0.0 (Linux 0.0.0) Editor Api changeMode() should not break image links FAILED
    Expected '[!\[github version\]\(https://img.shields.io/github/release/nhn/tui.editor.svg\)](https://github.com/nhn/tui.editor/releases/latest)' to be '[![github version](https://img.shields.io/github/release/nhn/tui.editor.svg)](https://github.com/nhn/tui.editor/releases/latest)'.
        at UserContext.<anonymous> (webpack:///test/unit/editor.spec.js:132:37 <- test/unit/test.bundle.js:61097:38)
HeadlessChrome 0.0.0 (Linux 0.0.0) Editor Api changeMode() should not break image links FAILED
    Expected '[!\[github version\]\(https://img.shields.io/github/release/nhn/tui.editor.svg\)](https://github.com/nhn/tui.editor/releases/latest)' to be '[![github version](https://img.shields.io/github/release/nhn/tui.editor.svg)](https://github.com/nhn/tui.editor/releases/latest)'.
        at UserContext.<anonymous> (webpack:///test/unit/editor.spec.js:132:37 <- test/unit/test.bundle.js:61097:38)
AlbinoDrought commented 5 years ago

I guess it might be an issue with the HTML -> markdown converter itself, possibly this line, this issue, this PR

The link/image text escaping can be globally disabled with:

import toMark from 'to-mark';
toMark.Renderer.prototype.escapeTextForLink = text => text;
seonim-ryu commented 5 years ago

@AlbinoDrought Right! The part you said is wrong in to-mark. Markdown syntax with an image in a link is not considered for escaping.

The solution is to fix the to-mark rather than use it globally.