Merge pull request #52 from beyounic/cursor-position

Bug: wrong cursor position when merging

Author: peyerluk

spec/cursor.spec.js

@@ -4,10 +4,10 @@ describe('Cursor', function() {
     expect(Cursor).toBeDefined();
   });
 
-  describe('with a range', function() {
+  describe('with a collapsed range at the end', function() {
 
     beforeEach(function() {
-      this.oneWord = $('<div>foobar</div>')[0];
+      this.oneWord = $('<div class="'+ config.editableClass +'">foobar</div>')[0];
       this.range = rangy.createRange();
       this.range.selectNodeContents(this.oneWord);
       this.range.collapse(false);
@@ -25,6 +25,32 @@ describe('Cursor', function() {
       expect(this.range.startOffset).toEqual(1);
       expect(this.range.endOffset).toEqual(1);
     });
+
+    describe('isAtEnd()', function() {
+      it('is true', function() {
+        expect(this.cursor.isAtEnd()).toBe(true);
+      });
+    });
+
+    describe('isAtBeginning()', function() {
+      it('is false', function() {
+        expect(this.cursor.isAtBeginning()).toBe(false);
+      });
+    });
+
+    describe('save() and restore()', function() {
+
+      it('saves and restores the cursor', function() {
+        this.cursor.save();
+
+        // move the cursor so we can check the restore method.
+        this.cursor.moveAtBeginning();
+        expect(this.cursor.isAtBeginning()).toBe(true);
+
+        this.cursor.restore();
+        expect(this.cursor.isAtEnd()).toBe(true);
+      });
+    });
   });
 
 });

spec/parser.spec.js

@@ -31,6 +31,28 @@ describe('Parser', function() {
   var linkWithSpan = $('<div><a href="#">foo <span class="important">bar</span></a></div>')[0];
 
 
+  describe('getHost()', function() {
+
+    beforeEach(function() {
+      this.$host = $('<div class="'+ config.editableClass +'""></div>');
+    });
+
+    it('works if host is passed', function() {
+      expect( parser.getHost(this.$host[0]) ).toBe( this.$host[0] );
+    });
+
+    it('works if a child of host is passed', function() {
+      this.$host.html('a<em>b</em>');
+      expect( parser.getHost(this.$host.find('em')[0]) ).toBe( this.$host[0] );
+    });
+
+    it('works if a text node is passed', function() {
+      this.$host.html('a<em>b</em>');
+      expect( parser.getHost(this.$host[0].firstChild) ).toBe( this.$host[0] );
+    });
+  });
+
+
   describe('getNodeIndex()', function() {
 
     it('gets element index of link in text', function() {

src/behavior.js

@@ -119,9 +119,9 @@ var behavior = (function() {
         return;
 
       if(container.childNodes.length > 0)
-        cursor.moveAfter(container.lastChild);
+        cursor.moveAtEnd(container);
       else
-        cursor.moveBefore(container);
+        cursor.moveAtBeginning(container);
       cursor.setSelection();
 
       fragment = document.createDocumentFragment();
@@ -133,10 +133,11 @@ var behavior = (function() {
 
       merger.parentNode.removeChild(merger);
 
-      range = rangeSaveRestore.save(cursor.range);
+      cursor.save();
       content.normalizeTags(container);
       content.normalizeSpaces(container);
-      rangeSaveRestore.restore(element, range);
+      cursor.restore();
+      cursor.setSelection();
     },
 
     empty: function(element) {
@@ -151,15 +152,15 @@ var behavior = (function() {
       switch(direction) {
       case 'before':
         previous = block.previous(element);
-        if(previous) {
-          cursor.moveAfter(previous);
+        if (previous) {
+          cursor.moveAtEnd(previous);
           cursor.setSelection();
         }
         break;
       case 'after':
         next = block.next(element);
-        if(next) {
-          cursor.moveBefore(next);
+        if (next) {
+          cursor.moveAtBeginning(next);
           cursor.setSelection();
         }
         break;

src/cursor.js

@@ -118,13 +118,55 @@ var Cursor = (function() {
       },
 
       moveBefore: function(element) {
-        this.range.setStart(element, 0);
-        this.range.setEnd(element, 0);
+        this.setHost(element);
+        this.range.setStartBefore(element);
+        this.range.setEndBefore(element);
+        if (this.isSelection) return new Cursor(this.host, this.range);
       },
 
       moveAfter: function(element) {
+        this.setHost(element);
+        this.range.setStartAfter(element);
+        this.range.setEndAfter(element);
+        if (this.isSelection) return new Cursor(this.host, this.range);
+      },
+
+      moveAtBeginning: function(element) {
+        if (!element) element = this.host;
+        this.setHost(element);
+        this.range.selectNodeContents(element);
+        this.range.collapse(true);
+        if (this.isSelection) return new Cursor(this.host, this.range);
+      },
+
+      moveAtEnd: function(element) {
+        if (!element) element = this.host;
+        this.setHost(element);
         this.range.selectNodeContents(element);
         this.range.collapse(false);
+        if (this.isSelection) return new Cursor(this.host, this.range);
+      },
+
+      setHost: function(element) {
+        this.host = parser.getHost(element);
+        if (!this.host) {
+          error('Can not set cursor outside of an editable block');
+        }
+      },
+
+      save: function() {
+        this.savedRangeInfo = rangeSaveRestore.save(this.range);
+        this.savedRangeInfo.host = this.host;
+      },
+
+      restore: function() {
+        if (this.savedRangeInfo) {
+          this.host = this.savedRangeInfo.host;
+          this.range = rangeSaveRestore.restore(this.host, this.savedRangeInfo);
+          this.savedRangeInfo = undefined;
+        } else {
+          error('Could not restore selection');
+        }
       },
 
       equals: function(cursor) {

src/dispatcher.js

@@ -57,7 +57,7 @@ var dispatcher = (function() {
         return;
 
       cursor = selectionWatcher.getSelection();
-      if (cursor.isSelection) return;
+      if (!cursor || cursor.isSelection) return;
 
       // Detect if the browser moved the cursor in the next tick.
       // If the cursor stays at its position, fire the switch event.

src/parser.js

@@ -31,6 +31,19 @@ var parser = (function() {
    */
   return {
 
+    /**
+     * Get the editableJS host block of a node.
+     *
+     * @method getHost
+     * @param {DOM Node}
+     * @return {DOM Node}
+     */
+    getHost: function(node) {
+      var editableSelector = '.' + config.editableClass;
+      var hostNode = $(node).closest(editableSelector);
+      return hostNode.length ? hostNode[0] : undefined;
+    },
+
     /**
      * Get the index of a node.
      * So that parent.childNodes[ getIndex(node) ] would return the node again

src/selection-watcher.js

@@ -23,10 +23,9 @@ var selectionWatcher = (function() {
     // (on a mac hold down the command key to select multiple ranges)
     if (rangySelection.rangeCount) {
       var range = rangySelection.getRangeAt(0);
-      var editableSelector = '.' + config.editableClass;
-      var hostNode = $(range.commonAncestorContainer).closest(editableSelector);
-      if (hostNode.length) {
-        return new RangeContainer(hostNode[0], range);
+      var hostNode = parser.getHost(range.commonAncestorContainer);
+      if (hostNode) {
+        return new RangeContainer(hostNode, range);
       }
     }