From 9957094d8242121b94e2584b57ba04cd1c7067ed Mon Sep 17 00:00:00 2001 From: Joseph Schenk Date: Tue, 19 Apr 2022 11:41:29 +0200 Subject: [PATCH] fix(comment index): index is found with search in added case --- spec/highlighting.spec.js | 265 ++++++++++++++++++++------------------ src/core.js | 2 +- src/highlight-support.js | 30 ++++- 3 files changed, 169 insertions(+), 128 deletions(-) diff --git a/spec/highlighting.spec.js b/spec/highlighting.spec.js index b9620fbf..bf477806 100644 --- a/spec/highlighting.spec.js +++ b/spec/highlighting.spec.js @@ -14,9 +14,10 @@ function setupHighlightEnv (context, text) { if (context.editable) context.editable.unload() context.editable = new Editable() context.editable.add(context.div) - context.highlightRange = (highlightId, start, end, dispatcher, type) => { + context.highlightRange = (text, highlightId, start, end, dispatcher, type) => { return highlightSupport.highlightRange( context.div, + text, highlightId, start, end, @@ -156,7 +157,8 @@ describe('Highlighting', function () { }) it('can handle a single highlight', function () { - const startIndex = this.highlightRange('myId', 3, 7) + const text = 'ple ' + const startIndex = this.highlightRange(text, 'myId', 3, 7) const expectedRanges = { myId: { text: 'ple ', @@ -174,16 +176,16 @@ Make The
World Go Round`) }) it('has the native range', function () { - this.highlightRange('myId', 3, 7) + this.highlightRange('ple ', 'myId', 3, 7) const extracted = this.extract() expect(extracted.myId.nativeRange.constructor.name).to.equal('Range') }) it('can handle adjaccent highlights', function () { - this.highlightRange('first', 0, 1) - this.highlightRange('second', 1, 2) - this.highlightRange('third', 2, 3) - this.highlightRange('fourth', 3, 4) + this.highlightRange('P', 'first', 0, 1) + this.highlightRange('e', 'second', 1, 2) + this.highlightRange('o', 'third', 2, 3) + this.highlightRange('p', 'fourth', 3, 4) const expectedRanges = { first: { @@ -219,10 +221,10 @@ le Make The
World Go Round`) }) it('can handle nested highlights', function () { - this.highlightRange('first', 0, 1) - this.highlightRange('second', 1, 2) - this.highlightRange('third', 2, 6) - this.highlightRange('fourth', 0, 6) + this.highlightRange('P', 'first', 0, 1) + this.highlightRange('e', 'second', 1, 2) + this.highlightRange('ople', 'third', 2, 6) + this.highlightRange('People', 'fourth', 0, 6) const expectedRanges = { first: { text: 'P', @@ -256,9 +258,9 @@ le Make The
World Go Round`) }) it('can handle intersecting highlights', function () { - this.highlightRange('first', 0, 3) - this.highlightRange('second', 2, 7) - this.highlightRange('third', 4, 6) + this.highlightRange('Peo', 'first', 0, 3) + this.highlightRange('ople', 'second', 2, 7) + this.highlightRange('le', 'third', 4, 6) const expectedRanges = { first: { text: 'Peo', @@ -285,7 +287,7 @@ le Make The
World Go Round`) }) it('can handle highlights containing break tags', function () { - this.highlightRange('first', 11, 22) + this.highlightRange(' The \nWorld', 'first', 11, 22) const expectedRanges = { first: { text: ' The \nWorld', @@ -303,8 +305,8 @@ le Make The
World Go Round`) }) it('can handle identical ranges', function () { - this.highlightRange('first', 11, 22) - this.highlightRange('second', 11, 22) + this.highlightRange(' The \nWorld', 'first', 11, 22) + this.highlightRange(' The \nWorld', 'second', 11, 22) const expectedRanges = { first: { text: ' The \nWorld', @@ -330,8 +332,8 @@ le Make The
World Go Round`) }) it('will update any existing range found under `highlightId` aka upsert', function () { - this.highlightRange('first', 11, 22) - this.highlightRange('first', 8, 9) + this.highlightRange('a', 'first', 11, 22) + this.highlightRange('a', 'first', 8, 9) const expectedRanges = { first: { text: 'a', @@ -347,81 +349,6 @@ ke The
World Go Round`) expect(this.getHtml()).to.equal(expectedHtml) }) - it('can handle all cases combined and creates consistent output', function () { - this.highlightRange('first', 4, 8) - this.highlightRange('second', 2, 10) - this.highlightRange('third', 4, 5) - this.highlightRange('first', 0, 24) - this.highlightRange('fourth', 20, 31) - this.highlightRange('fifth', 15, 16) - this.highlightRange('sixth', 15, 16) - - const expectedRanges = { - first: { - text: 'People Make The \nWorld G', - start: 0, - end: 24 - }, - second: { - text: 'ople Mak', - start: 2, - end: 10 - }, - third: { - text: 'l', - start: 4, - end: 5 - }, - fourth: { - text: 'ld Go Round', - start: 20, - end: 31 - }, - fifth: { - text: ' ', - start: 15, - end: 16 - }, - sixth: { - text: ' ', - start: 15, - end: 16 - } - } - const expectedHtml = this.formatHtml(`Pe -op -l -e Make The - - -
Wor - -ld G -o Round`) - - const extractedHtml = this.getHtml() - const extractedRanges = this.extractWithoutNativeRange() - - expect(extractedRanges).to.deep.equal(expectedRanges) - expect(extractedHtml).to.equal(expectedHtml) - - - const content = this.editable.getContent(this.div) - this.div.innerHTML = content - expect(content).to.equal(this.text) - const ids = ['first', 'second', 'third', 'fourth', 'fifth', 'sixth'] - ids.forEach(highlightId => { - this.highlightRange( - highlightId, - extractedRanges[highlightId].start, - extractedRanges[highlightId].end - ) - }) - - expect(this.extractWithoutNativeRange()).to.deep.equal(expectedRanges) - expect(this.getHtml()).to.equal(expectedHtml) - }) - it('skips and warns if an invalid range object was passed', function () { this.editable.highlight({ editableHost: this.div, @@ -454,27 +381,9 @@ o Round`) expect(highlightSpan.length).to.equal(0) }) - it('returns only highlightRanges with specific type', function () { - const startIndex = this.highlightRange('myId', 3, 7) - this.highlightRange('spellcheckId', 18, 22, undefined, 'spellcheck') - const expectedRanges = { - myId: { - text: 'ple ', - start: 3, - end: 7 - } - } - const expectedHtml = this.formatHtml(`Peo -ple -Make The
World Go Round`) - expect(this.getHtml()).to.equal(expectedHtml) - expect(this.extractWithoutNativeRange('comment')).to.deep.equal(expectedRanges) - expect(startIndex).to.equal(3) - }) - it('normalizes a simple text node after removing a highlight', function () { setupHighlightEnv(this, 'People Make The World Go Round') - this.highlightRange('myId', 3, 7) + this.highlightRange('ple ', 'myId', 3, 7) const normalizeSpy = sinon.spy(this.div, 'normalize') this.removeHighlight('myId') // There is no way to see the actual error in a test since it only happens in (non-headless) @@ -504,7 +413,7 @@ Make The
W W tags', function () { setupHighlightEnv(this, 'abcd') - this.highlightRange('first', 1, 3) + this.highlightRange('bc', 'first', 1, 3) const extract = this.extractWithoutNativeRange() expect(extract.first.text).to.equal('bc') @@ -535,7 +444,7 @@ Make The
W tags', function () { setupHighlightEnv(this, 'abcd') - this.highlightRange('first', 0, 2) + this.highlightRange('ab', 'first', 0, 2) const extract = this.extractWithoutNativeRange() expect(extract.first.text).to.equal('ab') @@ -561,7 +470,7 @@ Make The
W W W W W W called++} - this.highlightRange('first', 0, 20, dispatcher) + this.highlightRange('๐Ÿ˜ Makeย The ๐ŸŒ Go ๐Ÿ”„', 'first', 0, 20, dispatcher) expect(called).to.equal(1) }) @@ -647,10 +556,120 @@ Make The
W called++} - this.highlightRange('first', 0, 20) + this.highlightRange('๐Ÿ˜ Makeย The ๐ŸŒ Go ๐Ÿ”„', 'first', 0, 20) this.removeHighlight('first', dispatcher) expect(called).to.equal(1) }) }) + + describe('highlightSupport with multiple white spaces as received in browsers', function () { + beforeEach(function () { + setupHighlightEnv(this, 'People Make The 
 World Go Round') + }) + + afterEach(function () { + teardownHighlightEnv(this) + }) + + it('can handle all cases combined and creates consistent output', function () { + this.highlightRange('ople Mak', 'first', 2, 10) + this.highlightRange('l', 'second', 4, 5) + this.highlightRange('ld Go Round', 'third', 21, 32) + this.highlightRange(' ', 'fourth', 23, 24) + this.highlightRange(' ', 'fifth', 23, 24) + + const expectedRanges = { + first: { + start: 2, + end: 10, + text: 'ople Mak' + }, + second: { + start: 4, + end: 5, + text: 'l' + }, + third: { + start: 21, + end: 32, + text: 'ld Go Round' + }, + fourth: { + start: 23, + end: 24, + text: ' ' + }, + fifth: { + start: 23, + end: 24, + text: ' ' + } + } + const expectedHtml = this.formatHtml(`People Make The 
 World Go Round`) + const extractedRanges = this.extractWithoutNativeRange() + const content = this.editable.getContent(this.div) + expect(content).to.equal(this.text) + expect(extractedRanges).to.deep.equal(expectedRanges) + expect(this.getHtml()).to.deep.equal(expectedHtml) + }) + + it('returns only highlightRanges with specific type', function () { + const startIndex = this.highlightRange('ple ', 'myId', 3, 7) + this.highlightRange('orld', 'spellcheckId', 18, 22, undefined, 'spellcheck') + const expectedRanges = { + myId: { + text: 'ple ', + start: 3, + end: 7 + } + } + const expectedHtml = this.formatHtml(`Peo +ple +Make The 
 World Go Round`) + expect(this.getHtml()).to.equal(expectedHtml) + expect(this.extractWithoutNativeRange('comment')).to.deep.equal(expectedRanges) + expect(startIndex).to.equal(3) + }) + }) + + describe('highlightSupport matches based on both start index and match', function () { + beforeEach(function () { + setupHighlightEnv(this, 'People make the world go round and round and round the world') + }) + + afterEach(function () { + teardownHighlightEnv(this) + }) + + it('highlights based on both match and start index', function () { + this.highlightRange('round', 'myId', 35, 40) + const expectedRanges = { + myId: { + text: 'round', + start: 35, + end: 40 + } + } + const expectedHtml = this.formatHtml(`People make the world go round and round and round the world`) + expect(this.getHtml()).to.equal(expectedHtml) + expect(this.extractWithoutNativeRange('comment')).to.deep.equal(expectedRanges) + }) + + it('highlights based on nearest match despite wrong index', function () { + this.highlightRange('round', 'myId', 33, 38) + const expectedRanges = { + myId: { + text: 'round', + start: 35, + end: 40 + } + } + const expectedHtml = this.formatHtml(`People make the world go round and round and round the world`) + expect(this.getHtml()).to.equal(expectedHtml) + expect(this.extractWithoutNativeRange('comment')).to.deep.equal(expectedRanges) + }) + + + }) }) diff --git a/src/core.js b/src/core.js index fe46ddc0..57b9e412 100644 --- a/src/core.js +++ b/src/core.js @@ -400,7 +400,7 @@ export class Editable { ) return -1 } - return highlightSupport.highlightRange(editableHost, highlightId, textRange.start, textRange.end, raiseEvents ? this.dispatcher : undefined, type) + return highlightSupport.highlightRange(editableHost, text, highlightId, textRange.start, textRange.end, raiseEvents ? this.dispatcher : undefined, type) } /** diff --git a/src/highlight-support.js b/src/highlight-support.js index 513d9a6c..87dc3d85 100644 --- a/src/highlight-support.js +++ b/src/highlight-support.js @@ -12,7 +12,6 @@ const highlightSupport = { highlightText (editableHost, text, highlightId, type) { if (this.hasHighlight(editableHost, highlightId)) return - const blockText = highlightText.extractText(editableHost) const marker = `` @@ -28,12 +27,23 @@ const highlightSupport = { } }, - highlightRange (editableHost, highlightId, startIndex, endIndex, dispatcher, type = 'comment') { + + // This function was changed to track matches when text is added to the start of a component, but multiple white spaces break it in a strict sense + // The function works in the editor and in browsers, but tests with multiple white spaces will fail. + // Browsers change the white spaces to   and the function works, and the tests in highlight.spec.js have been updated to represent this. + + highlightRange (editableHost, text, highlightId, startIndex, endIndex, dispatcher, type = 'comment') { if (this.hasHighlight(editableHost, highlightId)) { this.removeHighlight(editableHost, highlightId) } + const firstMarker = `` + const markerNode = highlightSupport.createMarkerNode(firstMarker, type, this.win) + const textSearch = new TextHighlighting(markerNode, 'text') + const blockText = highlightText.extractText(editableHost) + const matchesArray = textSearch.findMatches(blockText, [text]) + const {actualStartIndex, actualEndIndex} = this.getIndex(matchesArray, startIndex, endIndex) const range = rangy.createRange() - range.selectCharacters(editableHost, startIndex, endIndex) + range.selectCharacters(editableHost, actualStartIndex, actualEndIndex) if (!isInHost(range.commonAncestorContainer, editableHost)) { return -1 @@ -52,7 +62,7 @@ const highlightSupport = { if (dispatcher) { dispatcher.notify('change', editableHost) } - return startIndex + return actualStartIndex }, updateHighlight (editableHost, highlightId, addCssClass, removeCssClass) { @@ -136,6 +146,18 @@ const highlightSupport = { marker.setAttribute('data-editable', 'ui-unwrap') marker.setAttribute('data-highlight', highlightType) return marker + }, + + // This function checks to see if text has been added to the component before the comment + // If it has, the start index is updated, otherwise it remains the same + getIndex (matchesArray, startIndex, endIndex) { + const newStartIndex = matchesArray.find((match) => { + return match.startIndex >= startIndex // checks if the startIndex has increased + }) + const actualStartIndex = newStartIndex ? newStartIndex.startIndex : startIndex + const actualEndIndex = actualStartIndex ? (actualStartIndex + (endIndex - startIndex)) : endIndex + + return {actualStartIndex, actualEndIndex} } }