Remove zero width space when getting text range for comment highlights

spec/dispatcher.spec.js

@@ -154,6 +154,9 @@ describe('Dispatcher:', function () {
           expect(content.getInnerHtmlOfFragment(before)).to.equal('ba')
           expect(content.getInnerHtmlOfFragment(after)).to.equal('r')
           expect(cursor.isCursor).to.equal(true)
+          // Check again to avoid regression (DOM was mutated)
+          expect(content.getInnerHtmlOfFragment(before)).to.equal('ba')
+          expect(content.getInnerHtmlOfFragment(after)).to.equal('r')
         })
 
         const evt = new KeyboardEvent('keydown', {keyCode: key.enter})

spec/highlight-support.spec.js

@@ -327,6 +327,38 @@ ke The <br> World Go Round`)
       expect(this.getHtml()).to.equal(expectedHtml)
     })
 
+    it('limits start and end values to text range', function () {
+      setupHighlightEnv(this, 'ab')
+      this.highlightRange('ab', 'myId', -1, 100)
+      const expectedHtml = this.formatHtml(
+        `<span class="highlight-comment" data-editable="ui-unwrap" data-highlight="comment" data-word-id="myId">ab</span>`
+      )
+
+      expect(this.getHtml()).to.equal(expectedHtml)
+    })
+
+    it('always selects the first character if values are too small', function () {
+      setupHighlightEnv(this, 'ab')
+      const highlightText = undefined // Avoid validation
+      this.highlightRange(highlightText, 'myId', 0, 0)
+      const expectedHtml = this.formatHtml(
+        `<span class="highlight-comment" data-editable="ui-unwrap" data-highlight="comment" data-word-id="myId">a</span>b`
+      )
+
+      expect(this.getHtml()).to.equal(expectedHtml)
+    })
+
+    it('always selects the last character if values are too large', function () {
+      setupHighlightEnv(this, 'ab')
+      const highlightText = undefined // Avoid validation
+      this.highlightRange(highlightText, 'myId', 2, 5)
+      const expectedHtml = this.formatHtml(
+        `a<span class="highlight-comment" data-editable="ui-unwrap" data-highlight="comment" data-word-id="myId">b</span>`
+      )
+
+      expect(this.getHtml()).to.equal(expectedHtml)
+    })
+
     it('handles a <br> tag without whitespaces', function () {
       setupHighlightEnv(this, 'a<br>b')
       this.highlightRange('b', 'myId', 1, 2)
@@ -482,7 +514,7 @@ ke The <br> World Go Round`)
 
     it('extracts a readable text', function () {
       setupHighlightEnv(this, '😐 Make&nbsp;The \r\n 🌍 Go \n🔄')
-      this.highlightRange('😐 Make The 🌍 Go 🔄', 'myId', 0, 23)
+      this.highlightRange('😐 Make The \n 🌍 Go \n🔄', 'myId', 0, 23)
       const expectedRanges = {
         myId: {
           text: '😐 Make The \n 🌍 Go \n🔄',
@@ -499,7 +531,7 @@ ke The <br> World Go Round`)
       setupHighlightEnv(this, '😐 Make&nbsp;The \r\n 🌍 Go \n🔄')
       let called = 0
       const dispatcher = {notify: () => called++}
-      this.highlightRange('😐 Make The 🌍 Go 🔄', 'myId', 0, 20, dispatcher)
+      this.highlightRange('😐 Make The \n 🌍 Go \n🔄', 'myId', 0, 23, dispatcher)
 
       expect(called).to.equal(1)
     })
@@ -508,7 +540,7 @@ ke The <br> World Go Round`)
       setupHighlightEnv(this, '😐 Make&nbsp;The \r\n 🌍 Go \n🔄')
       let called = 0
       const dispatcher = {notify: () => called++}
-      this.highlightRange('😐 Make The 🌍 Go 🔄', 'myId', 0, 20)
+      this.highlightRange('😐 Make The \n  🌍 Go \n🔄', 'myId', 0, 23)
       this.removeHighlight('first', dispatcher)
 
       expect(called).to.equal(1)

spec/selection.spec.js

@@ -142,6 +142,17 @@ describe('Selection', function () {
       })
     })
 
+    describe('getTextRange()', function () {
+
+      it('handles a zero width non-break space', function () {
+        const oneWord = createElement('<div>\uFEFFfoobar\uFEFF</div>')
+        const range = createRange()
+        range.selectNodeContents(oneWord)
+        const selection = new Selection(oneWord, range)
+        expect(selection.getTextRange()).to.deep.equal({start: 0, end: 6, text: 'foobar'})
+      })
+    })
+
     describe('custom:', function () {
 
       beforeEach(function () {

src/content.js

@@ -110,8 +110,10 @@ export function extractContent (element, keepUiElements) {
 }
 
 export function getInnerHtmlOfFragment (documentFragment) {
+  const deep = true
+  const clonedDocumentFragment = documentFragment.cloneNode(deep)
   const div = document.createElement('div')
-  div.appendChild(documentFragment)
+  div.appendChild(clonedDocumentFragment)
   return div.innerHTML
 }
 

src/highlight-support.js

@@ -45,8 +45,18 @@ const highlightSupport = {
       this.win
     )
 
-    const actualStartIndex = startIndex
-    const actualEndIndex = endIndex
+    // Do not let highlight exceed text range
+    const actualStartIndex = Math.min(Math.max(startIndex, 0), blockText.length - 1)
+    const actualEndIndex = Math.min(Math.max(endIndex, 1), blockText.length)
+
+    // If text is provided then validate that it matches
+    if (text) {
+      const tempElement = document.createElement('div')
+      tempElement.innerHTML = text
+      if (tempElement.textContent !== blockText.slice(actualStartIndex, actualEndIndex)) {
+        return -1
+      }
+    }
 
     highlightText.highlightMatches(editableHost, [{
       startIndex: actualStartIndex,
@@ -107,7 +117,7 @@ const highlightSupport = {
       if (position) res[highlightId] = position
     }
 
-    return res
+    return Object.keys(res).length > 0 ? res : undefined
   },
 
   extractMarkerNodePosition (editableHost, markers) {

src/util/dom.js

@@ -182,7 +182,10 @@ export const toCharacterRange = (range, container) => {
   startRange.setStart(container, 0)
   startRange.setEnd(range.startContainer, range.startOffset)
 
-  const rangeText = range.toString()
+  // Remove zero width space to make selection more accurate,
+  // because it will be removed on component blur via extractContent.
+  const zeroWidthNonBreakingSpace = /\uFEFF/g
+  const rangeText = range.toString().replace(zeroWidthNonBreakingSpace, '')
   const start = startRange.toString().length
   const end = start + rangeText.length