diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js index 1c3572da1a..518c7fed82 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js @@ -494,12 +494,13 @@ } // Skip nodes before startOffset. - while (startNode && - this._getLength(startNode) <= startOffset || - this._getLength(startNode) === 0) { - startOffset -= this._getLength(startNode); + var nodeLength = this._getLength(startNode); + while (startNode && (nodeLength <= startOffset || nodeLength === 0)) { + startOffset -= nodeLength; startNode = startNode.nextSibling; + nodeLength = startNode && this._getLength(startNode); } + if (!startNode) { return null; } // Split Text node. if (startNode instanceof Text) { @@ -546,12 +547,13 @@ } // Find the node where endOffset points at. - while (endNode && - this._getLength(endNode) < endOffset || - this._getLength(endNode) === 0) { - endOffset -= this._getLength(endNode); + var nodeLength = this._getLength(endNode); + while (endNode && (nodeLength < endOffset || nodeLength === 0)) { + endOffset -= nodeLength; endNode = endNode.nextSibling; + nodeLength = endNode && this._getLength(endNode); } + if (!endNode) { return null; } if (endNode instanceof Text) { endNode = diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html index 0df6a20b89..8771cc14b9 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html @@ -371,6 +371,40 @@ limitations under the License. assert.equal(tabs[1].nextSibling.textContent, 'verbis, '); }); + test('multiline highlight w/ start at end of 1st line', function() { + var diff = element.querySelector('#diffTable'); + var startContent = + diff.querySelector('.left.lineNum[data-value="138"] ~ .content'); + var betweenContent = + diff.querySelector('.left.lineNum[data-value="140"] ~ .content'); + var endContent = + diff.querySelector('.left.lineNum[data-value="141"] ~ .content'); + var commentThread = + diff.querySelector('gr-diff-comment-thread'); + var builder = { + getCommentThreadByContentEl: sandbox.stub().returns(commentThread), + getContentByLine: sandbox.stub().returns({}), + getContentsByLineRange: sandbox.stub().returns([betweenContent]), + getLineElByChild: sandbox.stub().returns( + {getAttribute: sandbox.stub()}), + }; + element._cachedDiffBuilder = builder; + element.enabled = true; + builder.getContentByLine.withArgs(138, 'left').returns( + startContent); + builder.getContentByLine.withArgs(141, 'left').returns( + endContent); + + var expectedStartContentNodes = startContent.childNodes.length; + + // The following should not cause an error. + element._applyRangedHighlight( + 'some', 138, startContent.textContent.length, 141, 28, 'left'); + + assert.equal(startContent.childNodes.length, expectedStartContentNodes, + 'Should not add a highlight to the start content'); + }); + suite('single line ranges', function() { var diff; var content;