From f3b72aef366896b511a6719cebc98a36f1818fba Mon Sep 17 00:00:00 2001 From: Cory Forsyth Date: Thu, 17 Dec 2015 19:10:24 -0500 Subject: [PATCH] marker#deleteValueAtOffset detects if the character is outside the BMP fixes #274 --- src/js/editor/post.js | 4 +-- src/js/models/marker.js | 45 +++++++++++++++++------- tests/acceptance/editor-sections-test.js | 34 ++++++++++++++++++ tests/unit/models/marker-test.js | 32 ++++++++--------- 4 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/js/editor/post.js b/src/js/editor/post.js index c9710d0e2..abdabcf22 100644 --- a/src/js/editor/post.js +++ b/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; diff --git a/src/js/models/marker.js b/src/js/models/marker.js index 30e83a033..ec374ba4e 100644 --- a/src/js/models/marker.js +++ b/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) { diff --git a/tests/acceptance/editor-sections-test.js b/tests/acceptance/editor-sections-test.js index a212ccb10..f11026089 100644 --- a/tests/acceptance/editor-sections-test.js +++ b/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); diff --git a/tests/unit/models/marker-test.js b/tests/unit/models/marker-test.js index c0c824fba..fd1431631 100644 --- a/tests/unit/models/marker-test.js +++ b/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'); +});