Fix Cursor blot restoration and selection preservation

blots/cursor.js

@@ -59,20 +59,6 @@ class Cursor extends EmbedBlot {
   restore() {
     if (this.selection.composing || this.parent == null) return null;
     const range = this.selection.getNativeRange();
-    let restoreText;
-    let start;
-    let end;
-    if (
-      range != null &&
-      range.start.node === this.textNode &&
-      range.end.node === this.textNode
-    ) {
-      [restoreText, start, end] = [
-        this.textNode,
-        range.start.offset,
-        range.end.offset,
-      ];
-    }
     // Link format will insert text outside of anchor tag
     while (
       this.domNode.lastChild != null &&
@@ -83,32 +69,64 @@ class Cursor extends EmbedBlot {
         this.domNode,
       );
     }
-    const { parentNode } = this.domNode;
-    if (this.textNode.data !== Cursor.CONTENTS) {
-      const text = this.textNode.data.split(Cursor.CONTENTS).join('');
-      if (this.next instanceof TextBlot) {
-        restoreText = this.next.domNode;
-        this.next.insertAt(0, text);
-        this.textNode.data = Cursor.CONTENTS;
-      } else {
-        this.textNode.data = text;
-        this.parent.insertBefore(this.scroll.create(this.textNode), this);
-        this.textNode = document.createTextNode(Cursor.CONTENTS);
-        this.domNode.appendChild(this.textNode);
+
+    const prevTextBlot = this.prev instanceof TextBlot ? this.prev : null;
+    const prevTextLength = prevTextBlot ? prevTextBlot.length() : 0;
+    const nextTextBlot = this.next instanceof TextBlot ? this.next : null;
+    const nextText = nextTextBlot ? nextTextBlot.text : '';
+    const { textNode } = this;
+    // take text from inside this blot and reset it
+    const newText = textNode.data.split(Cursor.CONTENTS).join('');
+    textNode.data = Cursor.CONTENTS;
+
+    // proactively merge TextBlots around cursor so that optimization
+    // doesn't lose the cursor.  the reason we are here in cursor.restore
+    // could be that the user clicked in prevTextBlot or nextTextBlot, or
+    // the user typed something.
+    let mergedTextBlot;
+    if (prevTextBlot) {
+      mergedTextBlot = prevTextBlot;
+      if (newText || nextTextBlot) {
+        prevTextBlot.insertAt(prevTextBlot.length(), newText + nextText);
+        if (nextTextBlot) {
+          nextTextBlot.remove();
+        }
       }
+    } else if (nextTextBlot) {
+      mergedTextBlot = nextTextBlot;
+      nextTextBlot.insertAt(0, newText);
+    } else {
+      const newTextNode = document.createTextNode(newText);
+      mergedTextBlot = this.scroll.create(newTextNode);
+      this.parent.insertBefore(mergedTextBlot, this);
     }
+
     this.remove();
-    parentNode.normalize();
-    if (start != null) {
-      [start, end] = [start, end].map(offset => {
-        return Math.max(0, Math.min(restoreText.data.length, offset - 1));
-      });
-      return {
-        startNode: restoreText,
-        startOffset: start,
-        endNode: restoreText,
-        endOffset: end,
+    if (range) {
+      // calculate selection to restore
+      const remapOffset = (node, offset) => {
+        if (prevTextBlot && node === prevTextBlot.domNode) {
+          return offset;
+        }
+        if (node === textNode) {
+          return prevTextLength + offset - 1;
+        }
+        if (nextTextBlot && node === nextTextBlot.domNode) {
+          return prevTextLength + newText.length + offset;
+        }
+        return null;
       };
+
+      const start = remapOffset(range.start.node, range.start.offset);
+      const end = remapOffset(range.end.node, range.end.offset);
+      if (start !== null && end !== null) {
+        return {
+          startNode: mergedTextBlot.domNode,
+          startOffset: start,
+          endNode: mergedTextBlot.domNode,
+          endOffset: end,
+        };
+      }
     }
     return null;
   }

core/selection.js

@@ -380,7 +380,15 @@ class Selection {
         nativeRange.native.collapsed &&
         nativeRange.start.node !== this.cursor.textNode
       ) {
-        this.cursor.restore();
+        const range = this.cursor.restore();
+        if (range) {
+          this.setNativeRange(
+            range.startNode,
+            range.startOffset,
+            range.endNode,
+            range.endOffset,
+          );
+        }
       }
       const args = [
         Emitter.events.SELECTION_CHANGE,

test/functional/epic.js

@@ -210,7 +210,49 @@ describe('quill', function() {
     const header = await page.$('.ql-toolbar .ql-header.ql-active[value="1"]');
     expect(header).not.toBe(null);
 
+    await page.keyboard.press('ArrowDown');
+    await page.keyboard.press('ArrowDown');
+    await page.keyboard.press('Enter');
+    await page.keyboard.press('Enter');
+    await page.keyboard.press('ArrowUp');
+    await page.type('.ql-editor', 'AA');
+    await page.keyboard.press('ArrowLeft');
+    await page.keyboard.down(SHORTKEY);
+    await page.keyboard.press('b');
+    await page.keyboard.press('b');
+    await page.keyboard.up(SHORTKEY);
+    await page.type('.ql-editor', 'B');
+    html = await page.$$eval('.ql-editor p', paras => paras[2].innerHTML);
+    expect(html).toBe('ABA');
+    await page.keyboard.down(SHORTKEY);
+    await page.keyboard.press('b');
+    await page.keyboard.up(SHORTKEY);
+    await page.type('.ql-editor', 'C');
+    await page.keyboard.down(SHORTKEY);
+    await page.keyboard.press('b');
+    await page.keyboard.up(SHORTKEY);
+    await page.type('.ql-editor', 'D');
+    html = await page.$$eval('.ql-editor p', paras => paras[2].innerHTML);
+    expect(html).toBe('AB<strong>C</strong>DA');
+    const selection = await page.evaluate(getSelectionInTextNode);
+    expect(selection).toBe('["DA",1,"DA",1]');
+
     // await page.waitFor(1000000);
     await browser.close();
   });
 });
+
+function getSelectionInTextNode() {
+  const {
+    anchorNode,
+    anchorOffset,
+    focusNode,
+    focusOffset,
+  } = document.getSelection();
+  return JSON.stringify([
+    anchorNode.data,
+    anchorOffset,
+    focusNode.data,
+    focusOffset,
+  ]);
+}