Repository: nhn/tui.editor
Pull request #1807 (2021-09-24 09:03:11)

fix: `<br>` operation when converting editor mode, pasting data or calling API
<!-- EDIT TITLE PLEASE -->
<!-- It should be one of them
  <ISSUE TYPE>: Short Description (<CLOSING TYPE> #<ISSUE NUMBERS>)
  ex)
  feat: add new feature (close #111)
  fix: wrong behavior (fix #111)
  chore: change build tool (ref #111)
-->

<!-- SPECIFY A ISSUE TYPE AT HEAD
  feat: A new feature
  fix: A bug fix
  docs: Documentation only changes
  style: Changes that do not affect the meaning of the code (white-space, formatting etc)
  refactor: A code change that neither fixes a bug or adds a feature
  perf: A code change that improves performance
  test: Adding missing tests
  chore: Changes to the build process or auxiliary tools and libraries such as documentation generation
-->

<!-- ADD CLOSING TYPE AND ISSUE NUMBER AT TAIL
  (<CLOSING TYPE> #<ISSUE NUMBERS>)
  close: resolve not a bug(feature, docs, etc) completely
  fix: resolve a bug completely
  ref: not fully resolved or related to
-->

### Please check if the PR fulfills these requirements
- [x] It's the right issue type on the title
- [ ] When resolving a specific issue, it's referenced in the PR's title (e.g. `fix #xxx[,#xxx]`, where "xxx" is the issue number)
- [x] The commit message follows our guidelines
- [x] Tests for the changes have been added (for bug fixes/features)
- [ ] Docs have been added/updated (for bug fixes/features)
- [ ] It does not introduce a breaking change or has a description of the breaking change

### Description
* fixed `<br>` operation when converting editor mode, pasting data or calling API
* related issue
  * #1665
  * #1666
  * #1779
  * #1798


---
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨



Patch:
diff --git a/apps/editor/src/convertors/toWysiwyg/htmlToWwConvertors.ts b/apps/editor/src/convertors/toWysiwyg/htmlToWwConvertors.ts
index 53d3ca8f..4e659dc3 100644
--- a/apps/editor/src/convertors/toWysiwyg/htmlToWwConvertors.ts
+++ b/apps/editor/src/convertors/toWysiwyg/htmlToWwConvertors.ts
@@ -7,7 +7,7 @@ import {
   ToWwConvertorState,
 } from '@t/convertor';
 import { includes } from '@/utils/common';
-import { CLOSE_TAG, reHTMLTag } from '@/utils/constants';
+import { reHTMLTag } from '@/utils/constants';
 
 export function getTextWithoutTrailingNewline(text: string) {
   return text[text.length - 1] === '\n' ? text.slice(0, text.length - 1) : text;
@@ -30,6 +30,10 @@ export function isInlineNode({ type }: MdNode) {
   return includes(['text', 'strong', 'emph', 'strike', 'image', 'link', 'code'], type);
 }
 
+function isSoftbreak(mdNode: MdNode | null) {
+  return mdNode?.type === 'softbreak';
+}
+
 function isListNode({ type, literal }: MdNode) {
   const matched = type === 'htmlInline' && literal!.match(reHTMLTag);
 
@@ -162,12 +166,22 @@ const convertors: HTMLToWwConvertorMap = {
     const { parent, prev, next } = node;
 
     if (parent?.type === 'paragraph') {
-      if (prev && !(prev.type === 'htmlInline' && prev.literal!.match(CLOSE_TAG))) {
+      // should open a paragraph node when line text has only <br> tag
+      // ex) first line\n\n<br>\nfourth line
+      if (isSoftbreak(prev)) {
         state.openNode(paragraph);
       }
 
-      if (next) {
+      // should close a paragraph node when line text has only <br> tag
+      // ex) first line\n\n<br>\nfourth line
+      if (isSoftbreak(next)) {
         state.closeNode();
+        // should close a paragraph node and open a paragraph node to separate between blocks
+        // when <br> tag is in the middle of the paragraph
+        // ex) first <br>line\nthird line
+      } else if (next) {
+        state.closeNode();
+        state.openNode(paragraph);
       }
     } else if (parent?.type === 'tableCell') {
       if (prev && (isInlineNode(prev) || isCustomHTMLInlineNode(state, prev))) {
diff --git a/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts b/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts
index 0dbfbd88..ff00066a 100644
--- a/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts
+++ b/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts
@@ -27,10 +27,10 @@ import { ToWwConvertorMap } from '@t/convertor';
 import { createWidgetContent, getWidgetContent } from '@/widget/rules';
 import { getChildrenHTML, getHTMLAttrsByHTMLString } from '@/wysiwyg/nodes/html';
 import { includes } from '@/utils/common';
-import { reHTMLTag } from '@/utils/constants';
+import { reBR, reHTMLTag } from '@/utils/constants';
 
 function isBRTag(node: MdNode) {
-  return node.type === 'htmlInline' && /<br ?\/?>/.test(node.literal!);
+  return node.type === 'htmlInline' && reBR.test(node.literal!);
 }
 
 function addRawHTMLAttributeToDOM(parent: Node) {
diff --git a/apps/editor/src/editorCore.ts b/apps/editor/src/editorCore.ts
index 79d04ff6..84d86ef8 100644
--- a/apps/editor/src/editorCore.ts
+++ b/apps/editor/src/editorCore.ts
@@ -85,7 +85,7 @@ import { getHTMLRenderConvertors } from './markdown/htmlRenderConvertors';
  *     @param {autofocus} [options.autofocus=true] - automatically focus the editor on creation.
  */
 class ToastUIEditorCore {
-  private initialHtml: string;
+  private initialHTML: string;
 
   private toastMark: ToastMark;
 
@@ -118,7 +118,7 @@ class ToastUIEditorCore {
   protected pluginInfo: PluginInfoResult;
 
   constructor(options: EditorOptions) {
-    this.initialHtml = options.el.innerHTML;
+    this.initialHTML = options.el.innerHTML;
     options.el.innerHTML = '';
 
     this.options = extend(
@@ -256,7 +256,7 @@ class ToastUIEditorCore {
     }
 
     if (!this.options.initialValue) {
-      this.setHTML(this.initialHtml, false);
+      this.setHTML(this.initialHTML, false);
     }
 
     this.commandManager = new CommandManager(
@@ -481,18 +481,15 @@ class ToastUIEditorCore {
    */
   getHTML() {
     this.eventEmitter.holdEventInvoke(() => {
-      if (this.isWysiwygMode()) {
-        this.mdEditor.setMarkdown(this.convertor.toMarkdownText(this.wwEditor.getModel()));
+      if (this.isMarkdownMode()) {
+        const mdNode = this.toastMark.getRootNode();
+        const wwNode = this.convertor.toWysiwygModel(mdNode);
+
+        this.wwEditor.setModel(wwNode!);
       }
     });
 
-    const mdNode = this.toastMark.getRootNode();
-    const mdRenderer = this.preview.getRenderer();
-
-    return mdRenderer
-      .render(mdNode)
-      .replace(/\sdata-nodeid="\d{1,}"/g, '')
-      .trim();
+    return this.wwEditor.view.dom.innerHTML;
   }
 
   /**
diff --git a/apps/editor/src/utils/constants.ts b/apps/editor/src/utils/constants.ts
index 7a119719..be4c2fea 100644
--- a/apps/editor/src/utils/constants.ts
+++ b/apps/editor/src/utils/constants.ts
@@ -16,3 +16,6 @@ export const CLOSE_TAG = `</(${TAG_NAME})\\s*[>]`;
 export const HTML_TAG = `(?:${OPEN_TAG}|${CLOSE_TAG})`;
 
 export const reHTMLTag = new RegExp(`^${HTML_TAG}`, 'i');
+export const reBR = /<br\s*\/*>/i;
+
+export const ALTERNATIVE_TAG_FOR_BR = '</p><p>';
diff --git a/apps/editor/src/utils/dom.ts b/apps/editor/src/utils/dom.ts
index cebc57a7..7a12f744 100644
--- a/apps/editor/src/utils/dom.ts
+++ b/apps/editor/src/utils/dom.ts
@@ -6,7 +6,7 @@ import hasClass from 'tui-code-snippet/domUtil/hasClass';
 import addClass from 'tui-code-snippet/domUtil/addClass';
 import removeClass from 'tui-code-snippet/domUtil/removeClass';
 import matches from 'tui-code-snippet/domUtil/matches';
-import { HTML_TAG, OPEN_TAG } from './constants';
+import { ALTERNATIVE_TAG_FOR_BR, HTML_TAG, OPEN_TAG, reBR } from './constants';
 import { isNil } from './common';
 
 export function isPositionInBox(style: CSSStyleDeclaration, offsetX: number, offsetY: number) {
@@ -239,27 +239,28 @@ export function setAttributes(attributes: Record<string, any>, element: HTMLElem
 }
 
 export function replaceBRWithEmptyBlock(html: string) {
-  const reBr = /<br\s*\/*>/i;
+  // remove br in paragraph to compatible with markdown
+  let replacedHTML = html.replace(/<p><br\s*\/*><\/p>/gi, '<p></p>');
   const reHTMLTag = new RegExp(HTML_TAG, 'ig');
-  const htmlTagMatched = html.match(reHTMLTag);
+  const htmlTagMatched = replacedHTML.match(reHTMLTag);
 
   htmlTagMatched?.forEach((htmlTag, index) => {
-    if (reBr.test(htmlTag)) {
-      let alternativeTag = '';
+    if (reBR.test(htmlTag)) {
+      let alternativeTag = ALTERNATIVE_TAG_FOR_BR;
 
       if (index) {
         const prevTag = htmlTagMatched[index - 1];
         const openTagMatched = prevTag.match(OPEN_TAG);
 
-        if (openTagMatched) {
+        if (openTagMatched && !/br/i.test(openTagMatched[1])) {
           const [, tagName] = openTagMatched;
 
           alternativeTag = `</${tagName}><${tagName}>`;
         }
       }
-      html = html.replace(reBr, alternativeTag);
+      replacedHTML = replacedHTML.replace(reBR, alternativeTag);
     }
   });
 
-  return html;
+  return replacedHTML;
 }
diff --git a/apps/editor/src/wysiwyg/clipboard/paste.ts b/apps/editor/src/wysiwyg/clipboard/paste.ts
index 3c61641c..93de153b 100644
--- a/apps/editor/src/wysiwyg/clipboard/paste.ts
+++ b/apps/editor/src/wysiwyg/clipboard/paste.ts
@@ -2,6 +2,7 @@ import { Schema, Node, Slice, Fragment, NodeType } from 'prosemirror-model';
 
 import { isFromMso, convertMsoParagraphsToList } from '@/wysiwyg/clipboard/pasteMsoList';
 import { getTableContentFromSlice } from '@/wysiwyg/helper/table';
+import { ALTERNATIVE_TAG_FOR_BR } from '@/utils/constants';
 
 const START_FRAGMENT_COMMENT = '<!--StartFragment-->';
 const END_FRAGMENT_COMMENT = '<!--EndFragment-->';
@@ -14,7 +15,7 @@ function getContentBetweenFragmentComments(html: string) {
     html = html.slice(startFragmentIndex + START_FRAGMENT_COMMENT.length, endFragmentIndex);
   }
 
-  return html;
+  return html.replace(/<br[^>]*>/g, ALTERNATIVE_TAG_FOR_BR);
 }
 
 function convertMsoTableToCompletedTable(html: string) {


Test patch:
diff --git a/apps/editor/src/__test__/unit/convertor.spec.ts b/apps/editor/src/__test__/unit/convertor.spec.ts
index f06413c5..726fa811 100644
--- a/apps/editor/src/__test__/unit/convertor.spec.ts
+++ b/apps/editor/src/__test__/unit/convertor.spec.ts
@@ -438,7 +438,7 @@ describe('Convertor', () => {
       assertConverting(markdown, expected);
     });
 
-    it('<br> with html inline node ', () => {
+    it('<br> with html inline node', () => {
       const markdown = source`
         foo
         bar
@@ -452,6 +452,45 @@ describe('Convertor', () => {
 
       assertConverting(markdown, expected);
     });
+
+    it('<br> with following <br>', () => {
+      const markdown = source`
+        text1
+        <br>
+        text2<br>
+        <br>
+        text3
+      `;
+      const expected = source`
+        text1
+        
+        text2
+        
+        text3
+      `;
+
+      assertConverting(markdown, expected);
+    });
+
+    it('<br> in the middle of the paragraph', () => {
+      const markdown = source`
+        text1
+        <br>
+        te<br>xt2<br>
+        <br>
+        text3
+      `;
+      const expected = source`
+        text1
+        
+        te
+        xt2
+        
+        text3
+      `;
+
+      assertConverting(markdown, expected);
+    });
   });
 
   describe('convert inline html', () => {
diff --git a/apps/editor/src/__test__/unit/editor.spec.ts b/apps/editor/src/__test__/unit/editor.spec.ts
index 977a32aa..0528b614 100644
--- a/apps/editor/src/__test__/unit/editor.spec.ts
+++ b/apps/editor/src/__test__/unit/editor.spec.ts
@@ -1,5 +1,5 @@
 import '@/i18n/en-us';
-import { oneLineTrim, stripIndents } from 'common-tags';
+import { oneLineTrim, stripIndents, source } from 'common-tags';
 import { Emitter } from '@t/event';
 import { EditorOptions } from '@t/editor';
 import type { OpenTagToken } from '@toast-ui/toastmark';
@@ -74,7 +74,7 @@ describe('editor', () => {
       it('basic', () => {
         editor.setMarkdown('# heading\n* bullet');
 
-        const result = stripIndents`
+        const result = oneLineTrim`
           <h1>heading</h1>
           <ul>
             <li>
@@ -95,6 +95,28 @@ describe('editor', () => {
 
         expect(spy).not.toHaveBeenCalled();
       });
+
+      it('should be the same as wysiwyg contents', () => {
+        const input = source`
+          <p>first line</p>
+          <p>second line</p>
+          <p><br>\nthird line</p>
+          <p><br>\n<br>\nfourth line</p>
+        `;
+        const expected = oneLineTrim`
+          <p>first line</p>
+          <p>second line</p>
+          <p><br></p>
+          <p>third line</p>
+          <p><br></p>
+          <p><br></p>
+          <p>fourth line</p>
+        `;
+
+        editor.setHTML(input);
+
+        expect(editor.getHTML()).toBe(expected);
+      });
     });
 
     it('changeMode()', () => {
@@ -159,6 +181,59 @@ describe('editor', () => {
         expect(mdEditor).toContainHTML('<div>a</div><div>b</div>');
         expect(getPreviewHTML()).toBe('<p>a<br>b</p>');
       });
+
+      it('should parse the br tag with the paragraph block to separate between blocks in wysiwyg', () => {
+        editor.setHTML(
+          '<h1>test title</h1><p><strong>test bold</strong><br><em>test italic</em><br>normal text</p>'
+        );
+        editor.changeMode('wysiwyg');
+
+        const expected = oneLineTrim`
+          <h1>test title</h1>
+          <p><strong>test bold</strong></p>
+          <p><em>test italic</em></p>
+          <p>normal text</p>
+        `;
+
+        expect(wwEditor).toContainHTML(expected);
+      });
+
+      it('should parse the br tag with the paragraph block to separate between blocks', () => {
+        const input = source`
+          <p>first line</p>
+          <p>second line</p>
+          <p><br>\nthird line</p>
+          <p><br>\n<br>\nfourth line</p>
+        `;
+        const expected = oneLineTrim`
+          <p>first line<br>second line</p>
+          <p>third line</p>
+          <p><br>fourth line</p>
+        `;
+
+        editor.setHTML(input);
+
+        expect(getPreviewHTML()).toBe(expected);
+      });
+
+      it('should be parsed with the same content when calling setHTML() with getHTML() API result', () => {
+        const input = source`
+          <p>first line</p>
+          <p>second line</p>
+          <p><br>\nthird line</p>
+          <p><br>\n<br>\nfourth line</p>
+        `;
+
+        editor.setHTML(input);
+
+        const mdEditorHTML = mdEditor.innerHTML;
+        const mdPreviewHTML = getPreviewHTML();
+
+        editor.setHTML(editor.getHTML());
+
+        expect(mdEditor).toContainHTML(mdEditorHTML);
+        expect(getPreviewHTML()).toBe(mdPreviewHTML);
+      });
     });
 
     it('reset()', () => {