Normalize some invalid comment ranges

Formerly, nonsensical comment ranges would prevent comments from
appearing in diffs. With this change, ranges are normalized by the
ranged comment layer so that they can be translated into valid
annotations.

See also
* Ibcab6e537abe8b81e764b09982a2581ae81463f8 should reject such ranges.
* I019e00063ab5c45c99379f3f9fb74eda0408d63f should avoid this error when
  constructing comment emails.

Bug: Issue 5744
Change-Id: Ib5834017f81f81a877b32264ee57d72302911a6c
This commit is contained in:
Wyatt Allen
2017-03-10 12:21:03 -08:00
parent 726a1b9cfe
commit 6a96c8b248
2 changed files with 33 additions and 2 deletions

View File

@@ -170,11 +170,20 @@
var ranges = this.get(['_commentMap', side, lineNum]) || [];
return ranges
.map(function(range) {
return {
var range = {
start: range.start,
end: range.end === -1 ? line.text.length : range.end,
hovering: !!range.comment.__hovering,
};
// Normalize invalid ranges where the start is after the end but the
// start still makes sense. Set the end to the end of the line.
// @see Issue 5744
if (range.start >= range.end && range.start < line.text.length) {
range.end = line.text.length;
}
return range;
})
.sort(function(a, b) {
// Sort the ranges so that hovering highlights are on top.

View File

@@ -76,6 +76,16 @@ limitations under the License.
start_character: 5,
start_line: 100,
},
}, {
id: '8675309',
line: 55,
message: 'nonsense range',
range: {
end_character: 2,
end_line: 55,
start_character: 32,
start_line: 55,
},
},
],
};
@@ -301,7 +311,7 @@ limitations under the License.
// on line 100.
var rightKeys = [];
for (i = 10; i <= 12; i++) { rightKeys.push('' + i); }
rightKeys.push('100');
rightKeys.push('55', '100');
assert.deepEqual(Object.keys(element._commentMap.right).sort(),
rightKeys.sort());
@@ -321,5 +331,17 @@ limitations under the License.
assert.equal(element._commentMap.right[100][0].start, 5);
assert.equal(element._commentMap.right[100][0].end, 15);
});
test('_getRangesForLine normalizes invalid ranges', function() {
var line = {
afterNumber: 55,
text: '_getRangesForLine normalizes invalid ranges'
};
var ranges = element._getRangesForLine(line, 'right');
assert.equal(ranges.length, 1);
var range = ranges[0];
assert.isTrue(range.start < range.end, 'start and end are normalized');
assert.equal(range.end, line.text.length);
});
});
</script>