Merge "Addresses a gr-diff-highlight edge case"

polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js

@@ -494,12 +494,13 @@
       }
 
       // Skip nodes before startOffset.
-      while (startNode &&
+      var nodeLength = this._getLength(startNode);
-          this._getLength(startNode) <= startOffset ||
+      while (startNode && (nodeLength <= startOffset || nodeLength === 0)) {
-          this._getLength(startNode) === 0) {
+        startOffset -= nodeLength;
-        startOffset -= this._getLength(startNode);
         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 &&
+      var nodeLength = this._getLength(endNode);
-          this._getLength(endNode) < endOffset ||
+      while (endNode && (nodeLength < endOffset || nodeLength === 0)) {
-          this._getLength(endNode) === 0) {
+        endOffset -= nodeLength;
-        endOffset -= this._getLength(endNode);
         endNode = endNode.nextSibling;
+        nodeLength = endNode && this._getLength(endNode);
       }
+      if (!endNode) { return null; }
 
       if (endNode instanceof Text) {
         endNode =

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;