code(cursor-search): fix inconsistencies, simplify logic

Author: peyerluk

spec/api.spec.js

@@ -126,11 +126,11 @@ describe('Editable', function () {
 
     describe('findClosestCursorOffset:', function () {
     /*
-Cursor1:                     | (left: 130)
-Comp 1:   Cristiano Ronaldo wurde 2018 mit grossem Tamtam nach Turin geholt.
-Comp 2:   Der Spieler blieb bei fünf Champions-League-Titeln stehen.
-Cursor 2:                    | (offset: 19 chars)
-    */
+     * Cursor1:                     | (left: 130)
+     * Comp 1:   Cristiano Ronaldo wurde 2018 mit grossem Tamtam nach Turin geholt.
+     * Comp 2:   Der Spieler blieb bei fünf Champions-League-Titeln stehen.
+     * Cursor 2:                    | (offset: 19 chars)
+     */
       it('finds the index in a text node', function () {
         $div.html('Der Spieler blieb bei fünf Champions-League-Titeln stehen.')
         const {wasFound, offset} = editable.findClosestCursorOffset({
@@ -142,11 +142,11 @@ Cursor 2:                    | (offset: 19 chars)
       })
 
       /*
-Cursor1:                      | (left: 130)
-Comp 1:   Cristiano Ronaldo wurde 2018 mit grossem Tamtam nach Turin geholt.
-Comp 2:   <p>Der <em>Spieler</em> blieb bei fünf <span>Champions-League-Titeln</span> stehen.</p>
-Cursor 2:                                   |
-    */
+       * Cursor1:                      | (left: 130)
+       * Comp 1:   Cristiano Ronaldo wurde 2018 mit grossem Tamtam nach Turin geholt.
+       * Comp 2:   <p>Der <em>Spieler</em> blieb bei fünf <span>Champions-League-Titeln</span> stehen.</p>
+       * Cursor 2:                                   |
+       */
       it('finds the index in a nested html tag structure', function () {
         $div.html('<p>Der <em>Spieler</em> blieb bei fünf <span>Champions-League-Titeln</span> stehen.</p>')
         const {wasFound, offset} = editable.findClosestCursorOffset({
@@ -167,18 +167,19 @@ Cursor 2:                                   |
       })
 
       /*
-Cursor1:                                                   |
-Comp 1:   Cristiano Ronaldo wurde 2018 mit grossem Tamtam nach Turin geholt.
-Comp 2:   Foo
-Cursor 2: not found
-    */
+       * Cursor1:                                                   |
+       * Comp 1:   Cristiano Ronaldo wurde 2018 mit grossem Tamtam nach Turin geholt.
+       * Comp 2:   Foo
+       * Cursor 2: not found
+       */
       it('returns not found for coordinates that are out of the text area', function () {
         $div.html('Foo')
-        const {wasFound} = editable.findClosestCursorOffset({
+        const {wasFound, offset} = editable.findClosestCursorOffset({
           element: $div[0],
           origCoordinates: {top: 0, left: 130}
         })
-        expect(wasFound).toEqual(false)
+        expect(wasFound).toEqual(true)
+        expect(offset).toEqual(3)
       })
     })
   })

spec/node-iterator.spec.js

@@ -22,7 +22,8 @@ describe('NodeIterator', function () {
     it('sets its properties', function () {
       expect(this.iterator.root).toEqual(this.element)
       expect(this.iterator.current).toEqual(this.element)
-      expect(this.iterator.next).toEqual(this.element)
+      expect(this.iterator.nextNode).toEqual(this.element)
+      expect(this.iterator.previous).toEqual(this.element)
     })
   })
 
@@ -63,7 +64,7 @@ describe('NodeIterator', function () {
 
       this.iterator.replaceCurrent(replacement)
       expect(this.iterator.current).toEqual(replacement)
-      expect(this.iterator.next).toEqual(null)
+      expect(this.iterator.nextNode).toEqual(null)
     })
 
     it('replaces the first character of longer a text node', function () {

src/core.js

@@ -12,7 +12,7 @@ import highlightSupport from './highlight-support'
 import Highlighting from './highlighting'
 import createDefaultEvents from './create-default-events'
 import browser from 'bowser'
-import {getTotalCharCount, textNodesUnder, getTextNodeAndRelativeOffset} from './util/element'
+import {textNodesUnder, getTextNodeAndRelativeOffset} from './util/element'
 import {binaryCursorSearch} from './util/binary_search'
 
 /**
@@ -195,16 +195,15 @@ export default class Editable {
    */
 
   createCursor (element, position = 'beginning') {
-    const $host = $(element).closest(this.editableSelector)
-
-    if (!$host.length) return undefined
+    const host = Cursor.findHost(element, this.editableSelector)
+    if (!host) return undefined
 
     const range = rangy.createRange()
 
     if (position === 'beginning' || position === 'end') {
       range.selectNodeContents(element)
       range.collapse(position === 'beginning')
-    } else if (element !== $host[0]) {
+    } else if (element !== host) {
       if (position === 'before') {
         range.setStartBefore(element)
         range.setEndBefore(element)
@@ -216,26 +215,19 @@ export default class Editable {
       error('EditableJS: cannot create cursor outside of an editable block.')
     }
 
-    return new Cursor($host[0], range)
-  }
-
-  createRangyRange () {
-    return rangy.createRange()
-  }
-
-  createCursorWithRange ({element, range}) {
-    const $host = $(element).closest(this.editableSelector)
-    return new Cursor($host[0], range)
+    return new Cursor(host, range)
   }
 
   createCursorAtCharacterOffset ({element, offset}) {
     const textNodes = textNodesUnder(element)
     const {node, relativeOffset} = getTextNodeAndRelativeOffset({textNodes, absOffset: offset})
-    const newRange = this.createRangyRange()
+    const newRange = rangy.createRange()
     newRange.setStart(node, relativeOffset)
-    newRange.setEnd(node, relativeOffset)
-    newRange.collapse()
-    const nextCursor = this.createCursorWithRange({element, range: newRange})
+    newRange.collapse(true)
+
+    const host = Cursor.findHost(element, this.editableSelector)
+    const nextCursor = new Cursor(host, newRange)
+
     nextCursor.setVisibleSelection()
     return nextCursor
   }
@@ -470,83 +462,42 @@ export default class Editable {
     return this
   }
 
-  /*
-    Takes coordinates and uses its left value to find out how to offset a character in a string to
-    closely match the coordinates.left value.
-    Takes conditions for the result being on the first line, used when navigating to a paragraph from
-    the above paragraph and being on the last line, used when navigating to a paragraph from the below
-    paragraph.
-
-    Internally this sets up the methods used for a binary cursor search and calls this.
-
-    @param {DomNode} element The DOM Node (usually editable elem) to which the cursor jumps
-    @param {Object} coordinates The bounding rect of the preceeding cursor to be matched
-    @param {Boolean} requiredOnFirstLine set to true if you want to require the cursor to be on the first line of the paragraph
-    @param {Boolean} requiredOnLastLine set to true if you want to require the cursor to be on the last line of the paragraph
-
-    @return {Object} object with boolean `wasFound` indicating if the binary search found an offset and `offset` to indicate the actual character offset
-    */
-  findClosestCursorOffset (
-    {element, origCoordinates, requiredOnFirstLine = false, requiredOnLastLine = false}) {
-    // early terminate on empty editables
-    const totalCharCount = getTotalCharCount(element)
-    if (totalCharCount === 0) return {wasFound: false}
-    // move left if the coordinates are to the left
-    const leftCondition = ({data, coordinates}) => {
-      if (coordinates.left >= data.origCoordinates.left) return true
-      return false
-    }
-    // move up if cursor is required to be on first line but is currently not
-    const upCondition = ({data, cursor}) => {
-      if (data.requiredOnFirstLine && !cursor.isAtFirstLine()) return true
-      return false
-    }
-    // move down if cursor is required to be on last line but is currently not
-    const downCondition = ({data, cursor}) => {
-      if (data.requiredOnLastLine && !cursor.isAtLastLine()) return true
-      return false
-    }
-    // move the binary search index in between the current position and the left limit
-    const moveLeft = (data) => {
-      data.rightLimit = data.currentOffset
-      data.currentOffset = Math.floor((data.currentOffset - data.leftLimit) / 2)
-    }
-    // move the binary search index in between the current position and the right limit
-    const moveRight = (data) => {
-      data.leftLimit = data.currentOffset
-      data.currentOffset = data.currentOffset + Math.floor((data.rightLimit - data.currentOffset) / 2)
-    }
-    // consider a small bluriness since character never exactly match coordinates
-    const foundChecker = ({distance, bluriness}) => {
-      return distance <= bluriness
-    }
-    // consider converged if 2 consecutive history entries are equal
-    const convergenceChecker = (history) => {
-      const lastTwo = history.slice(-2)
-      if (lastTwo.length === 2 && lastTwo[0] === lastTwo[1]) return true
-      return false
-    }
+  /**
+   * Takes coordinates and uses its left value to find out how to offset a character in a string to
+   * closely match the coordinates.left value.
+   * Takes conditions for the result being on the first line, used when navigating to a paragraph from
+   * the above paragraph and being on the last line, used when navigating to a paragraph from the below
+   * paragraph.
+   *
+   * Internally this sets up the methods used for a binary cursor search and calls this.
+   *
+   * @param {DomNode} element
+   *  - the editable hostDOM Node to which the cursor jumps
+   * @param {object} coordinates
+   *  - The bounding rect of the preceeding cursor to be matched
+   * @param {boolean} requiredOnFirstLine
+   *  - set to true if you want to require the cursor to be on the first line of the paragraph
+   * @param {boolean} requiredOnLastLine
+   *  - set to true if you want to require the cursor to be on the last line of the paragraph
+   *
+   * @return {Object}
+   *  - object with boolean `wasFound` indicating if the binary search found an offset and `offset` to indicate the actual character offset
+   */
+  findClosestCursorOffset ({
+    element,
+    origCoordinates,
+    requiredOnFirstLine = false,
+    requiredOnLastLine = false
+  }) {
+    const positionX = this.dispatcher.switchContext
+      ? this.dispatcher.switchContext.positionX
+      : origCoordinates.left
 
-    const data = {
-      element,
-      currentOffset: Math.floor(totalCharCount / 2),
-      rightLimit: totalCharCount,
-      leftLimit: 0,
+    return binaryCursorSearch({
+      host: element,
       requiredOnFirstLine,
       requiredOnLastLine,
-      origCoordinates
-    }
-
-    return binaryCursorSearch({
-      moveLeft,
-      moveRight,
-      leftCondition,
-      upCondition,
-      downCondition,
-      convergenceChecker,
-      foundChecker,
-      createCursorAtCharacterOffset: this.createCursorAtCharacterOffset.bind(this),
-      data
+      positionX
     })
   }
 }

src/cursor.js

@@ -5,9 +5,11 @@ import * as viewport from './util/viewport'
 import * as content from './content'
 import * as parser from './parser'
 import * as string from './util/string'
-import * as nodeType from './node-type'
+import {elementNode, documentFragmentNode} from './node-type'
 import error from './util/error'
 import * as rangeSaveRestore from './range-save-restore'
+// import printRange from './util/print_range'
+import NodeIterator from './node-iterator'
 
 /**
  * The Cursor module provides a cross-browser abstraction layer for cursor.
@@ -17,6 +19,12 @@ import * as rangeSaveRestore from './range-save-restore'
  */
 
 export default class Cursor {
+
+  static findHost (elem, selector) {
+    if (!elem.closest) elem = elem.parentNode
+    return elem.closest(selector)
+  }
+
   /**
   * Class for the Cursor module.
   *
@@ -46,21 +54,61 @@ export default class Cursor {
   }
 
   isAtLastLine () {
-    const range = rangy.createRange()
-    range.selectNodeContents(this.host)
-
-    const hostCoords = range.nativeRange.getBoundingClientRect()
-    const cursorCoords = this.range.nativeRange.getBoundingClientRect()
+    const hostRange = this.win.document.createRange()
+    hostRange.selectNodeContents(this.host)
+    const hostCoords = hostRange.getBoundingClientRect()
+
+    let cursorCoords
+    if (this.range.nativeRange.startContainer.nodeType === elementNode) {
+      const container = this.range.nativeRange.startContainer
+      if ((container.children.length - 1) >= this.range.nativeRange.startOffset) {
+        const elem = container.children[this.range.nativeRange.startOffset]
+        const iterator = new NodeIterator(elem)
+        const textNode = iterator.getNextTextNode()
+        if (textNode) {
+          const cursorRange = this.win.document.createRange()
+          cursorRange.setStart(textNode, 0)
+          cursorRange.collapse(true)
+          cursorCoords = cursorRange.getBoundingClientRect()
+        } else {
+          cursorCoords = hostCoords
+        }
+      } else {
+        cursorCoords = hostCoords
+      }
+    } else {
+      cursorCoords = this.getBoundingClientRect()
+    }
 
     return hostCoords.bottom === cursorCoords.bottom
   }
 
   isAtFirstLine () {
-    const range = rangy.createRange()
-    range.selectNodeContents(this.host)
-
-    const hostCoords = range.nativeRange.getBoundingClientRect()
-    const cursorCoords = this.range.nativeRange.getBoundingClientRect()
+    const hostRange = this.win.document.createRange()
+    hostRange.selectNodeContents(this.host)
+    const hostCoords = hostRange.getBoundingClientRect()
+
+    let cursorCoords
+    if (this.range.nativeRange.startContainer.nodeType === elementNode) {
+      const container = this.range.nativeRange.startContainer
+      if ((container.children.length - 1) >= this.range.nativeRange.startOffset) {
+        const elem = container.children[this.range.nativeRange.startOffset]
+        const iterator = new NodeIterator(elem)
+        const textNode = iterator.getPreviousTextNode()
+        if (textNode) {
+          const cursorRange = this.win.document.createRange()
+          cursorRange.setStart(textNode, 0)
+          cursorRange.collapse(true)
+          cursorCoords = cursorRange.getBoundingClientRect()
+        } else {
+          cursorCoords = hostCoords
+        }
+      } else {
+        cursorCoords = hostCoords
+      }
+    } else {
+      cursorCoords = this.range.nativeRange.getBoundingClientRect()
+    }
 
     return hostCoords.top === cursorCoords.top
   }
@@ -83,7 +131,7 @@ export default class Cursor {
     element = this.adoptElement(element)
 
     let preceedingElement = element
-    if (element.nodeType === nodeType.documentFragmentNode) {
+    if (element.nodeType === documentFragmentNode) {
       const lastIndex = element.childNodes.length - 1
       preceedingElement = element.childNodes[lastIndex]
     }
@@ -169,6 +217,10 @@ export default class Cursor {
     return content.getInnerHtmlOfFragment(this.after())
   }
 
+  getBoundingClientRect () {
+    return this.range.nativeRange.getBoundingClientRect()
+  }
+
   // Get the BoundingClientRect of the cursor.
   // The returned values are transformed to be absolute
   // (relative to the document).