From 133055a697a3aebb7a1aae970969502d3c0ad5f2 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 23 Sep 2013 15:45:41 +0200 Subject: [PATCH 1/5] Add parser#getHost() In order to add functionality to save a Cursor we need a convenient way to get the host of an element, so the Cursor can make sure its host is always correct even if the Cursor is moved to another block (like for example in the case of block merge). --- spec/parser.spec.js | 22 ++++++++++++++++++++++ src/parser.js | 13 +++++++++++++ src/selection-watcher.js | 7 +++---- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/spec/parser.spec.js b/spec/parser.spec.js index b147d59b..5f6883ad 100644 --- a/spec/parser.spec.js +++ b/spec/parser.spec.js @@ -31,6 +31,28 @@ describe('Parser', function() { var linkWithSpan = $('
foo bar
')[0]; + describe('getHost()', function() { + + beforeEach(function() { + this.$host = $('
'); + }); + + 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('ab'); + expect( parser.getHost(this.$host.find('em')[0]) ).toBe( this.$host[0] ); + }); + + it('works if a text node is passed', function() { + this.$host.html('ab'); + expect( parser.getHost(this.$host[0].firstChild) ).toBe( this.$host[0] ); + }); + }); + + describe('getNodeIndex()', function() { it('gets element index of link in text', function() { diff --git a/src/parser.js b/src/parser.js index be60b99a..e48df64d 100644 --- a/src/parser.js +++ b/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 diff --git a/src/selection-watcher.js b/src/selection-watcher.js index 670cbf8e..a1b3d1cd 100644 --- a/src/selection-watcher.js +++ b/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); } } From 25fb73839091baf96aba8b22fcc3928d7be7232d Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 23 Sep 2013 16:36:19 +0200 Subject: [PATCH 2/5] Add check for selection on keydown I got errors in the console when moving around with the cursor. So I just added this conditional to get rid of the errors. --- src/dispatcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dispatcher.js b/src/dispatcher.js index d9c558bd..f96e4093 100644 --- a/src/dispatcher.js +++ b/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. From c9a5e93a441014fccf61002af71c1d4a1ec0688f Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 23 Sep 2013 16:45:01 +0200 Subject: [PATCH 3/5] Add Cursor#save() and #restore() Add RangeSaveRestore functionality directly to the Cursor class. To get this to work the cursor must also update its host if someone moves the cursor. For that I added the #setHost method that should be called whenever the host could have changed. I also store the host when saving a cursor, so it becomes possible to move the cursor to another block and then restore it to the previous block. On top of that I added the convenience methods #moveAtBeginning() and #moveAtEnd() which I wanted to use in the specs. While at it I also refactored the #moveAfter() and #moveBefore() so they use the rangy methods and set the range more where one would expect it. --- spec/cursor.spec.js | 30 +++++++++++++++++++++++++++-- src/cursor.js | 46 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/spec/cursor.spec.js b/spec/cursor.spec.js index 654faa4d..81f95f94 100644 --- a/spec/cursor.spec.js +++ b/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 = $('
foobar
')[0]; + this.oneWord = $('
foobar
')[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); + }); + }); }); }); diff --git a/src/cursor.js b/src/cursor.js index 65e48ab3..0044b565 100644 --- a/src/cursor.js +++ b/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) { From 0cf3445ac8fb77f8d7aa9cb2d3e8dc490a8cf62d Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 23 Sep 2013 16:50:54 +0200 Subject: [PATCH 4/5] And finally... the bugfix! The bug itself was more a matter of calling restore with the right container and then setting the selection again. But with the new Cursor#restore() method one does not need to think about these details anymore :) --- src/behavior.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/behavior.js b/src/behavior.js index 51237085..cf6580c4 100644 --- a/src/behavior.js +++ b/src/behavior.js @@ -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) { From dcb77081ca9047148fd408cccdc4675abe9edad4 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 23 Sep 2013 17:11:04 +0200 Subject: [PATCH 5/5] Use correct Cursor move methods The new #moveAtBeginning and #moveAtEnd are the correct methods to place the cursor at the beginning or end of a container. --- src/behavior.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/behavior.js b/src/behavior.js index cf6580c4..50ddb629 100644 --- a/src/behavior.js +++ b/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(); @@ -152,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;