fix: Handle nodes and text when trimming whitespace from a selection

Author: ajwild

spec/selection.spec.js

@@ -433,14 +433,16 @@ describe('Selection', function () {
     })
 
     it('trims a range with special whitespaces', function () {
-      // at the beginning we have U+2002, U+2005 and U+2006 in the end a normal whitespace
-      const wordWithSpecialWhitespaces = createElement('<div>   bar </div>')
+      // At the beginning we have U+2002, U+2005, U+2006, U+FEFF.
+      // At the end a normal whitespace.
+      // Note: U+200B is not handled by regular expression \s whitespace.
+      const wordWithSpecialWhitespaces = createElement('<div>   bar </div>')
       const range = rangy.createRange()
       range.selectNodeContents(wordWithSpecialWhitespaces.firstChild)
       const selection = new Selection(wordWithSpecialWhitespaces, range)
       selection.trimRange()
-      expect(selection.range.startOffset).to.equal(3)
-      expect(selection.range.endOffset).to.equal(6)
+      expect(selection.range.startOffset).to.equal(4)
+      expect(selection.range.endOffset).to.equal(7)
     })
 
     it('does trim if only a whitespace is selected', function () {
@@ -460,7 +462,7 @@ describe('Selection', function () {
     })
 
     it('does not apply tags to whitespace when toggling', function () {
-      const range = createRange()
+      const range = rangy.createRange()
       range.setStart(this.wordWithWhitespace.firstChild, 0)
       range.setEnd(this.wordWithWhitespace.firstChild, 1)
       const selection = new Selection(this.wordWithWhitespace, range)
@@ -470,14 +472,33 @@ describe('Selection', function () {
     })
 
     it('does not apply tags to whitespace when wrapping', function () {
-      const range = createRange()
+      const range = rangy.createRange()
       range.setStart(this.wordWithWhitespace.firstChild, 0)
       range.setEnd(this.wordWithWhitespace.firstChild, 1)
       const selection = new Selection(this.wordWithWhitespace, range)
       selection.makeBold()
       expect(selection.toString()).to.equal('')
       expect(this.wordWithWhitespace.innerHTML).to.equal(' foobar ')
     })
+
+    it('handles nodes and characters', function () {
+      // Split word into three nodes: ` `, `foo`, `bar `
+      const range = rangy.createRange()
+      range.setStart(this.wordWithWhitespace.firstChild, 1)
+      range.setEnd(this.wordWithWhitespace.firstChild, 4)
+      const selection = new Selection(this.wordWithWhitespace, range)
+      selection.save()
+      selection.restore()
+
+      // Select specific characters within nodes across multiple nodes
+      const rangeTwo = rangy.createRange()
+      rangeTwo.setStart(this.wordWithWhitespace, 0) // Select first node (start)
+      rangeTwo.setEnd(this.wordWithWhitespace.childNodes[2], 2) // Select middle of last node
+      const selectionTwo = new Selection(this.wordWithWhitespace, rangeTwo)
+      selectionTwo.makeBold()
+
+      expect(this.wordWithWhitespace.innerHTML).to.equal(' <strong>fooba</strong>r ')
+    })
   })
 
   describe('inherits form Cursor', function () {

src/selection.js

@@ -7,6 +7,10 @@ import * as block from './block'
 import config from './config'
 import highlightSupport from './highlight-support'
 import highlightText from './highlight-text'
+import {
+  findStartExcludingWhitespace,
+  findEndExcludingWhitespace
+} from './util/dom'
 
 /**
  * The Selection module provides a cross-browser abstraction layer for range
@@ -98,9 +102,25 @@ export default class Selection extends Cursor {
     const textToTrim = this.range.toString()
     const whitespacesOnTheLeft = textToTrim.search(/\S|$/)
     const lastNonWhitespace = textToTrim.search(/\S[\s]+$/)
-    const whitespacesOnTheRight = lastNonWhitespace === -1 ? 0 : textToTrim.length - (lastNonWhitespace + 1)
-    this.range.setStart(this.range.startContainer, this.range.startOffset + whitespacesOnTheLeft)
-    this.range.setEnd(this.range.endContainer, this.range.endOffset - whitespacesOnTheRight)
+    const whitespacesOnTheRight = lastNonWhitespace === -1
+      ? 0
+      : textToTrim.length - (lastNonWhitespace + 1)
+
+    const [startContainer, startOffset] = findStartExcludingWhitespace({
+      root: this.range.commonAncestorContainer,
+      startContainer: this.range.startContainer,
+      startOffset: this.range.startOffset,
+      whitespacesOnTheLeft
+    })
+    this.range.setStart(startContainer, startOffset)
+
+    const [endContainer, endOffset] = findEndExcludingWhitespace({
+      root: this.range.commonAncestorContainer,
+      endContainer: this.range.endContainer,
+      endOffset: this.range.endOffset,
+      whitespacesOnTheRight
+    })
+    this.range.setEnd(endContainer, endOffset)
   }
 
   unlink () {

src/util/dom.js

@@ -1,3 +1,6 @@
+import NodeIterator from '../node-iterator'
+import {textNode} from '../node-type'
+
 /**
  * @param {HTMLElement | Array | String} target
  * @param {Document} document
@@ -42,3 +45,88 @@ export const closest = (elem, selector) => {
   if (!elem.closest) elem = elem.parentNode
   if (elem && elem.closest) return elem.closest(selector)
 }
+
+export function findStartExcludingWhitespace ({root, startContainer, startOffset, whitespacesOnTheLeft}) {
+  const isTextNode = startContainer.nodeType === textNode
+  if (!isTextNode) {
+    return findStartExcludingWhitespace({
+      root,
+      startContainer: startContainer.childNodes[startOffset],
+      startOffset: 0,
+      whitespacesOnTheLeft
+    })
+  }
+
+  const offsetAfterWhitespace = startOffset + whitespacesOnTheLeft
+  if (startContainer.length > offsetAfterWhitespace) {
+    return [startContainer, offsetAfterWhitespace]
+  }
+
+  // Pass the root so that the iterator can traverse to siblings
+  const iterator = new NodeIterator(root)
+  // Set the position to the node which is selected
+  iterator.nextNode = startContainer
+  // Iterate once to avoid returning self
+  iterator.getNextTextNode()
+
+  const container = iterator.getNextTextNode()
+  if (!container) {
+    // No more text nodes - use the end of the last text node
+    const previousTextNode = iterator.getPreviousTextNode()
+    return [previousTextNode, previousTextNode.length]
+  }
+
+  return findStartExcludingWhitespace({
+    root,
+    startContainer: container,
+    startOffset: 0,
+    whitespacesOnTheLeft: offsetAfterWhitespace - startContainer.length
+  })
+}
+
+export function findEndExcludingWhitespace ({root, endContainer, endOffset, whitespacesOnTheRight}) {
+  const isTextNode = endContainer.nodeType === textNode
+  if (!isTextNode) {
+    const isFirstNode = !endContainer.childNodes[endOffset - 1]
+    const container = isFirstNode
+      ? endContainer.childNodes[endOffset]
+      : endContainer.childNodes[endOffset - 1]
+    let offset = 0
+    if (!isFirstNode) {
+      offset = container.nodeType === textNode
+        ? container.length
+        : container.childNodes.length
+    }
+    return findEndExcludingWhitespace({
+      root,
+      endContainer: container,
+      endOffset: offset,
+      whitespacesOnTheRight
+    })
+  }
+
+  const offsetBeforeWhitespace = endOffset - whitespacesOnTheRight
+  if (offsetBeforeWhitespace > 0) {
+    return [endContainer, offsetBeforeWhitespace]
+  }
+
+  // Pass the root so that the iterator can traverse to siblings
+  const iterator = new NodeIterator(root)
+  // Set the position to the node which is selected
+  iterator.previous = endContainer
+  // Iterate once to avoid returning self
+  iterator.getPreviousTextNode()
+
+  const container = iterator.getPreviousTextNode()
+  if (!container) {
+    // No more text nodes - use the start of the first text node
+    return [iterator.getNextTextNode(), 0]
+  }
+
+  return findEndExcludingWhitespace({
+    root,
+    endContainer: container,
+    endOffset: container.length,
+    whitespacesOnTheRight: whitespacesOnTheRight - endOffset
+  })
+}