Fix splitByGrapheme selection (#5588)

src/mixins/itext_click_behavior.mixin.js

@@ -205,7 +205,7 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
         height += this.getHeightOfLine(i) * this.scaleY;
         lineIndex = i;
         if (i > 0) {
-          charIndex += this._textLines[i - 1].length + 1;
+          charIndex += this._textLines[i - 1].length + this.missingNewlineOffset(i);
         }
       }
       else {

src/mixins/itext_key_behavior.mixin.js

@@ -296,7 +296,7 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
         widthBeforeCursor = this._getWidthBeforeCursor(lineIndex, charIndex),
         indexOnOtherLine = this._getIndexOnLine(lineIndex + 1, widthBeforeCursor),
         textAfterCursor = this._textLines[lineIndex].slice(charIndex);
-    return textAfterCursor.length + indexOnOtherLine + 2;
+    return textAfterCursor.length + indexOnOtherLine + 1 + this.missingNewlineOffset(lineIndex);
   },
 
   /**
@@ -331,9 +331,11 @@ fabric.util.object.extend(fabric.IText.prototype, /** @lends fabric.IText.protot
     var charIndex = cursorLocation.charIndex,
         widthBeforeCursor = this._getWidthBeforeCursor(lineIndex, charIndex),
         indexOnOtherLine = this._getIndexOnLine(lineIndex - 1, widthBeforeCursor),
-        textBeforeCursor = this._textLines[lineIndex].slice(0, charIndex);
+        textBeforeCursor = this._textLines[lineIndex].slice(0, charIndex),
+        missingNewlineOffset = this.missingNewlineOffset(lineIndex - 1);
     // return a negative offset
-    return -this._textLines[lineIndex - 1].length + indexOnOtherLine - textBeforeCursor.length;
+    return -this._textLines[lineIndex - 1].length
+     + indexOnOtherLine - textBeforeCursor.length + (1 - missingNewlineOffset);
   },
 
   /**

src/mixins/text_style.mixin.js

@@ -147,7 +147,7 @@
       var loc = this.get2DCursorLocation(index);
 
       if (!this._getLineStyle(loc.lineIndex)) {
-        this._setLineStyle(loc.lineIndex, {});
+        this._setLineStyle(loc.lineIndex);
       }
 
       if (!this._getStyleDeclaration(loc.lineIndex, loc.charIndex)) {
@@ -166,16 +166,16 @@
       if (typeof selectionStart === 'undefined') {
         selectionStart = this.selectionStart;
       }
-      var lines = skipWrapping ? this._unwrappedTextLines : this._textLines;
-      var len = lines.length;
+      var lines = skipWrapping ? this._unwrappedTextLines : this._textLines,
+          len = lines.length;
       for (var i = 0; i < len; i++) {
         if (selectionStart <= lines[i].length) {
           return {
             lineIndex: i,
             charIndex: selectionStart
           };
         }
-        selectionStart -= lines[i].length + 1;
+        selectionStart -= lines[i].length + this.missingNewlineOffset(i);
       }
       return {
         lineIndex: i - 1,
@@ -295,19 +295,20 @@
 
     /**
      * @param {Number} lineIndex
+     * @return {Boolean} if the line exists or not
      * @private
      */
     _getLineStyle: function(lineIndex) {
-      return this.styles[lineIndex];
+      return !!this.styles[lineIndex];
     },
 
     /**
+     * Set the line style to an empty object so that is initialized
      * @param {Number} lineIndex
-     * @param {Object} style
      * @private
      */
-    _setLineStyle: function(lineIndex, style) {
-      this.styles[lineIndex] = style;
+    _setLineStyle: function(lineIndex) {
+      this.styles[lineIndex] = {};
     },
 
     /**

src/shapes/text.class.js

@@ -416,6 +416,16 @@
       return lineIndex === this._textLines.length - 1;
     },
 
+    /**
+     * Detect if a line has a linebreak and so we need to account for it when moving
+     * and counting style.
+     * It return always for text and Itext.
+     * @return Number
+     */
+    missingNewlineOffset: function() {
+      return 1;
+    },
+
     /**
      * Returns string representation of an instance
      * @return {String} String representation of text object

src/shapes/textbox.class.js

@@ -127,7 +127,7 @@
           charCount++;
           realLineCount++;
         }
-        else if (!this.graphemeSplit && this._reSpaceAndTab.test(textInfo.graphemeText[charCount]) && i > 0) {
+        else if (!this.splitByGrapheme && this._reSpaceAndTab.test(textInfo.graphemeText[charCount]) && i > 0) {
           // this case deals with space's that are removed from end of lines when wrapping
           realLineCharCount++;
           charCount++;
@@ -234,34 +234,27 @@
     },
 
     /**
-    * probably broken need a fix
+     * probably broken need a fix
+     * Returns the real style line that correspond to the wrapped lineIndex line
+     * Used just to verify if the line does exist or not.
      * @param {Number} lineIndex
+     * @returns {Boolean} if the line exists or not
      * @private
      */
     _getLineStyle: function(lineIndex) {
       var map = this._styleMap[lineIndex];
-      return this.styles[map.line];
+      return !!this.styles[map.line];
     },
 
     /**
-     * probably broken need a fix
+     * Set the line style to an empty object so that is initialized
      * @param {Number} lineIndex
      * @param {Object} style
      * @private
      */
-    _setLineStyle: function(lineIndex, style) {
+    _setLineStyle: function(lineIndex) {
       var map = this._styleMap[lineIndex];
-      this.styles[map.line] = style;
-    },
-
-    /**
-     * probably broken need a fix
-     * @param {Number} lineIndex
-     * @private
-     */
-    _deleteLineStyle: function(lineIndex) {
-      var map = this._styleMap[lineIndex];
-      delete this.styles[map.line];
+      this.styles[map.line] = {};
     },
 
     /**
@@ -390,6 +383,18 @@
       return false;
     },
 
+    /**
+     * Detect if a line has a linebreak and so we need to account for it when moving
+     * and counting style.
+     * @return Number
+     */
+    missingNewlineOffset: function(lineIndex) {
+      if (this.splitByGrapheme) {
+        return this.isEndOfWrapping(lineIndex) ? 1 : 0;
+      }
+      return 1;
+    },
+
     /**
     * Gets lines of text to render in the Textbox. This function calculates
     * text wrapping on the fly every time it is called.

test/unit/itext_key_behaviour.js

@@ -341,4 +341,11 @@
     assert.equal(iText.styles[2][0].fill, 'col5', 'style 2 0 has been inserted col5');
     assert.equal(iText.styles[2][1].fill, 'blue', 'style 2 1 has been inserted blue');
   });
+
+  QUnit.test('missingNewlineOffset', function(assert) {
+    var iText = new fabric.IText('由石墨\n分裂的石墨分\n裂\n由石墨分裂由石墨分裂的石\n墨分裂');
+
+    var offset = iText.missingNewlineOffset(0);
+    assert.equal(offset, 1, 'it returns always 1');
+  });
 })();

test/unit/textbox.js

@@ -239,7 +239,7 @@
     assert.equal(canvas._currentTransform.newScaleX, text.width / originalWidth, 'newScaleX is not undefined');
   });
   QUnit.test('_removeExtraneousStyles', function(assert) {
-    var iText = new fabric.IText('a\nq\qo', { styles: {
+    var iText = new fabric.Textbox('a\nq\qo', { styles: {
       0: { 0: { fontSize: 4 } },
       1: { 0: { fontSize: 4 } },
       2: { 0: { fontSize: 4 } },
@@ -252,4 +252,113 @@
     assert.equal(iText.styles[3], undefined, 'style line 3 has been removed');
     assert.equal(iText.styles[4], undefined, 'style line 4 has been removed');
   });
+
+  QUnit.test('get2DCursorLocation with splitByGrapheme', function(assert) {
+    var iText = new fabric.Textbox('aaaaaaaaaaaaaaaaaaaaaaaa',
+      { width: 60, splitByGrapheme: true });
+    var loc = iText.get2DCursorLocation();
+
+    // [ [ '由', '石', '墨' ],
+    //   [ '分', '裂', '的' ],
+    //   [ '石', '墨', '分' ],
+    //   [ '裂', '由', '石' ],
+    //   [ '墨', '分', '裂' ],
+    //   [ '由', '石', '墨' ],
+    //   [ '分', '裂', '的' ],
+    //   [ '石', '墨', '分' ],
+    //   [ '裂' ] ]
+
+    assert.equal(loc.lineIndex, 0);
+    assert.equal(loc.charIndex, 0);
+
+    // '由石墨|分裂的石墨分裂由石墨分裂由石墨分裂的石墨分裂'
+    iText.selectionStart = iText.selectionEnd = 4;
+    loc = iText.get2DCursorLocation();
+
+    assert.equal(loc.lineIndex, 1, 'selection end 4 line 1');
+    assert.equal(loc.charIndex, 1, 'selection end 4 char 1');
+
+    iText.selectionStart = iText.selectionEnd = 7;
+    loc = iText.get2DCursorLocation();
+
+    assert.equal(loc.lineIndex, 2, 'selection end 7 line 2');
+    assert.equal(loc.charIndex, 1, 'selection end 7 char 1');
+
+    iText.selectionStart = iText.selectionEnd = 14;
+    loc = iText.get2DCursorLocation();
+
+    assert.equal(loc.lineIndex, 4, 'selection end 14 line 4');
+    assert.equal(loc.charIndex, 2, 'selection end 14 char 2');
+  });
+
+  QUnit.test('missingNewlineOffset with splitByGrapheme', function(assert) {
+    var textbox = new fabric.Textbox('aaa\naaaaaa\na\naaaaaaaaaaaa\naaa',
+      { width: 80, splitByGrapheme: true });
+
+    // [ [ 'a', 'a', 'a' ],
+    //   [ 'a', 'a', 'a', 'a' ],
+    //   [ 'a', 'a' ],
+    //   [ 'a' ],
+    //   [ 'a', 'a', 'a', 'a' ],
+    //   [ 'a', 'a', 'a', 'a' ],
+    //   [ 'a', 'a', 'a', 'a' ],
+    //   [ 'a', 'a', 'a' ] ]
+
+    var offset = textbox.missingNewlineOffset(0);
+    assert.equal(offset, 1, 'line 0 is interrupted by a \n so has an offset of 1');
+
+    offset = textbox.missingNewlineOffset(1);
+    assert.equal(offset, 0, 'line 1 is wrapped without a \n so it does have an extra char count');
+  });
+
+  QUnit.test('missingNewlineOffset with normal split', function(assert) {
+    var texbox = new fabric.Textbox('aaa\naaaaaa\na\naaaaaaaaaaaa\naaa',
+      { width: 160 });
+
+    var offset = texbox.missingNewlineOffset(0);
+    assert.equal(offset, 1, 'it returns always 1');
+    var offset = texbox.missingNewlineOffset(1);
+    assert.equal(offset, 1, 'it returns always 1');
+    var offset = texbox.missingNewlineOffset(2);
+    assert.equal(offset, 1, 'it returns always 1');
+  });
+
+  QUnit.test('_getLineStyle', function(assert) {
+    var textbox = new fabric.Textbox('aaa aaq ggg gg\noee eee', {
+      styles: {
+        1: { 0: { fontSize: 4 } },
+      },
+      width: 80,
+    });
+
+    assert.equal(textbox._getLineStyle(0), false, 'wrapped line 0 has no style');
+    assert.equal(textbox._getLineStyle(1), false, 'wrapped line 1 has no style');
+    assert.equal(textbox._getLineStyle(4), true, 'wrapped line 2 has style');
+  });
+
+  QUnit.test('_setLineStyle', function(assert) {
+    var textbox = new fabric.Textbox('aaa aaq ggg gg\noee eee', {
+      styles: {
+        1: { 0: { fontSize: 4 } },
+      },
+      width: 80,
+    });
+
+    assert.equal(textbox._getLineStyle(0), false, 'wrapped line 0 has no style');
+    assert.equal(textbox._getLineStyle(1), false, 'wrapped line 1 has no style');
+    assert.equal(textbox._getLineStyle(2), false, 'wrapped line 2 has no style');
+    assert.equal(textbox._getLineStyle(3), false, 'wrapped line 3 has no style');
+
+    assert.deepEqual(textbox.styles[0], undefined, 'style is undefined');
+    textbox._setLineStyle(0);
+
+    assert.equal(textbox._getLineStyle(0), true, 'wrapped line 0 has style');
+    assert.equal(textbox._getLineStyle(1), true, 'wrapped line 1 has style');
+    assert.equal(textbox._getLineStyle(2), true, 'wrapped line 2 has style');
+    assert.equal(textbox._getLineStyle(3), true, 'wrapped line 3 has style');
+
+    assert.deepEqual(textbox.styles[0], {}, 'style is an empty object');
+  });
+
+
 })();