Merge pull request #275 from bustlelabs/fix-marker-emoji-deletion-274

marker#deleteValueAtOffset detects if the character is outside the BMP

src/js/editor/post.js

@@ -515,8 +515,8 @@ class PostEditor {
 
     const offsetToDeleteAt = markerOffset - 1;
 
-    marker.deleteValueAtOffset(offsetToDeleteAt);
-    nextPosition.offset -= 1;
+    let lengthChange = marker.deleteValueAtOffset(offsetToDeleteAt);
+    nextPosition.offset -= lengthChange;
     this._markDirty(marker);
 
     return nextPosition;

src/js/models/marker.js

@@ -4,6 +4,16 @@ import { detect, commonItemLength, forEach, filter } from '../utils/array-utils'
 import LinkedItem from '../utils/linked-item';
 import assert from '../utils/assert';
 
+// Unicode uses a pair of "surrogate" characters" (a high- and low-surrogate)
+// to encode characters outside the basic multilingual plane (like emoji and
+// some languages).
+// These values are the unicode code points for the start and end of the
+// high- and low-surrogate characters.
+// See "high surrogate" and "low surrogate" on
+// https://en.wikipedia.org/wiki/Unicode_block
+const HIGH_SURROGATE_RANGE = [0xD800, 0xDBFF];
+const LOW_SURROGATE_RANGE  = [0xDC00, 0xDFFF];
+
 const Marker = class Marker extends LinkedItem {
   constructor(value='', markups=[]) {
     super();
@@ -31,14 +41,6 @@ const Marker = class Marker extends LinkedItem {
     return this.value.length;
   }
 
-  truncateFrom(offset) {
-    this.value = this.value.substr(0, offset);
-  }
-
-  truncateTo(offset) {
-    this.value = this.value.substr(offset);
-  }
-
   clearMarkups() {
     this.markups = [];
   }
@@ -69,17 +71,34 @@ const Marker = class Marker extends LinkedItem {
     }
   }
 
-  // delete the character at this offset,
-  // update the value with the new value
+  /**
+   * delete the character at this offset,
+   * update the value with the new value.
+   * This method mutates the marker.
+   *
+   * @return {Number} the length of the change
+   * (usually 1 but can be 2 when deleting an emoji, e.g.)
+   */
   deleteValueAtOffset(offset) {
-    if (offset < 0 || offset > this.length) {
-      throw new Error(`Invalid offset "${offset}"`);
+    assert('Cannot delete value at offset outside bounds',
+           offset >= 0 && offset <= this.length);
+
+    let width = 1;
+    let code = this.value.charCodeAt(offset);
+    if (code >= HIGH_SURROGATE_RANGE[0] && code <= HIGH_SURROGATE_RANGE[1]) {
+      width = 2;
+    } else if (code >= LOW_SURROGATE_RANGE[0] && code <= LOW_SURROGATE_RANGE[1]) {
+      width = 2;
+      offset = offset - 1;
     }
+
     const [ left, right ] = [
       this.value.slice(0, offset),
-      this.value.slice(offset+1)
+      this.value.slice(offset+width)
     ];
     this.value = left + right;
+
+    return width;
   }
 
   hasMarkup(tagNameOrMarkup) {

tests/acceptance/editor-sections-test.js

@@ -268,6 +268,40 @@ test('keystroke of delete removes that character', (assert) => {
                    'cursor is at start of new text node');
 });
 
+test('keystroke of delete removes emoji character', (assert) => {
+  let monkey = 'monkey🙈';
+  let mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker(monkey)])]);
+  });
+  editor = new Editor({mobiledoc});
+  editor.render(editorElement);
+  let textNode = editorElement.firstChild. // section
+                               firstChild; // marker 
+  assert.equal(textNode.textContent, monkey, 'precond - correct text');
+
+  Helpers.dom.moveCursorTo(textNode, monkey.length);
+  Helpers.dom.triggerDelete(editor);
+
+  assert.equal($('#editor p:eq(0)').text(), 'monkey', 'deletes the emoji');
+});
+
+test('keystroke of forward delete removes emoji character', (assert) => {
+  let monkey = 'monkey🙈';
+  let mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker(monkey)])]);
+  });
+  editor = new Editor({mobiledoc});
+  editor.render(editorElement);
+  let textNode = editorElement.firstChild. // section
+                               firstChild; // marker 
+  assert.equal(textNode.textContent, monkey, 'precond - correct text');
+
+  Helpers.dom.moveCursorTo(textNode, 'monkey'.length);
+  Helpers.dom.triggerForwardDelete(editor);
+
+  assert.equal($('#editor p:eq(0)').text(), 'monkey', 'deletes the emoji');
+});
+
 test('keystroke of delete when cursor is at beginning of marker removes character from previous marker', (assert) => {
   editor = new Editor({mobiledoc: mobileDocWith2Markers});
   editor.render(editorElement);

tests/unit/models/marker-test.js

@@ -12,24 +12,6 @@ module('Unit: Marker', {
   }
 });
 
-test('a marker can truncated from an offset', (assert) => {
-  const m1 = builder.createMarker('hi there!');
-
-  const offset = 5;
-  m1.truncateFrom(offset);
-
-  assert.equal(m1.value, 'hi th');
-});
-
-test('a marker can truncated to an offset', (assert) => {
-  const m1 = builder.createMarker('hi there!');
-
-  const offset = 5;
-  m1.truncateTo(offset);
-
-  assert.equal(m1.value, 'ere!');
-});
-
 test('a marker can have a markup applied to it', (assert) => {
   const m1 = builder.createMarker('hi there!');
   m1.addMarkup(builder.createMarkup('b'));
@@ -124,3 +106,17 @@ test('#clone a marker', (assert) => {
   assert.equal(marker.value, cloned.value, 'value is present');
   assert.equal(marker.markups.length, cloned.markups.length, 'markup length is the same');
 });
+
+// https://github.com/bustlelabs/mobiledoc-kit/issues/274
+test('#deleteValueAtOffset handles emoji', (assert) => {
+  let str = 'monkey 🙈';
+  assert.equal(str.length, 'monkey '.length + 2,
+               'string length reports monkey emoji as length 2');
+  let marker = builder.createMarker(str);
+  marker.deleteValueAtOffset(str.length - 1);
+  assert.equal(marker.value, 'monkey ', 'deletes correctly from low surrogate');
+
+  marker = builder.createMarker(str);
+  marker.deleteValueAtOffset(str.length - 2);
+  assert.equal(marker.value, 'monkey ', 'deletes correctly from high surrogate');
+});