opendev/gerrit
Code Issues Proposed changes

Enable multiline range comments in Firefox

Browse Source 
Apparently, Firefox returns one range per line of selected code for
multiline range comments, while Chrome and Safari merge those ranges
into one. This change uses first and last range for multiple range
selections as, respectively, a start and end points.

Bug: Issue 6557
Change-Id: Ib59f0c273b41433b07d333d084cadd5749ba36d9
This commit is contained in:
Viktar Donich
2017-06-26 13:51:19 -07:00
committed by Wyatt Allen
parent 8803103ebe
commit f49a7884d5
2 changed files with 78 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
View File

@@ -94,6 +94,47 @@
}
},
/**
* Get current normalized selection.
* Merges multiple ranges, accounts for triple click, accounts for
* syntax highligh, convert native DOM Range objects to Gerrit concepts
* (line, side, etc).
* @return {{
* start: {
* node: Node,
* side: string,
* line: Number,
* column: Number
* },
* end: {
* node: Node,
* side: string,
* line: Number,
* column: Number
* }
* }}
*/
_getNormalizedRange: function() {
var selection = window.getSelection();
var rangeCount = selection.rangeCount;
if (rangeCount === 0) {
return null;
} else if (rangeCount === 1) {
return this._normalizeRange(selection.getRangeAt(0));
} else {
var startRange = this._normalizeRange(selection.getRangeAt(0));
var endRange = this._normalizeRange(
selection.getRangeAt(rangeCount - 1));
return {
start: startRange.start,
end: endRange.end,
};
}
},
/**
* Normalize a specific DOM Range.
*/
_normalizeRange: function(domRange) {
var range = GrRangeNormalizer.normalize(domRange);
return this._fixTripleClickSelection({
@@ -204,16 +245,13 @@
},
_handleSelection: function() {
var selection = window.getSelection();
if (selection.rangeCount != 1) {
var normalizedRange = this._getNormalizedRange();
if (!normalizedRange) {
return;
}
var range = selection.getRangeAt(0);
if (range.collapsed) {
return;
}
var normalizedRange = this._normalizeRange(range);
var domRange = window.getSelection().getRangeAt(0);
var start = normalizedRange.start;
if (!start) {
return;
}
@@ -239,7 +277,7 @@
};
actionBox.side = start.side;
if (start.line === end.line) {
actionBox.placeAbove(range);
actionBox.placeAbove(domRange);
} else if (start.node instanceof Text) {
actionBox.placeAbove(start.node.splitText(start.column));
start.node.parentElement.normalize(); // Undo splitText from above.

32
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
View File

@@ -304,6 +304,37 @@ limitations under the License.
assert.equal(getActionSide(), 'right');
});
test('multiple ranges aka firefox implementation', () => {
var startContent = stubContent(119, 'right');
var endContent = stubContent(120, 'right');
var startRange = document.createRange();
startRange.setStart(startContent.firstChild, 10);
startRange.setEnd(startContent.firstChild, 11);
var endRange = document.createRange();
endRange.setStart(endContent.lastChild, 6);
endRange.setEnd(endContent.lastChild, 7);
var getRangeAtStub = sandbox.stub();
getRangeAtStub
.onFirstCall().returns(startRange)
.onSecondCall().returns(endRange);
sandbox.stub(window, 'getSelection').returns({
rangeCount: 2,
getRangeAt: getRangeAtStub,
removeAllRanges: sandbox.stub(),
});
element._handleSelection();
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
startLine: 119,
startChar: 10,
endLine: 120,
endChar: 36,
});
});
test('multiline grow end highlight over tabs', function() {
var startContent = stubContent(119, 'right');
var endContent = stubContent(120, 'right');
@@ -502,6 +533,7 @@ limitations under the License.
result = GrRangeNormalizer._getTextOffset(content, child);
assert.equal(result, 0);
});
// TODO (viktard): Selection starts in line number.
// TODO (viktard): Empty lines in selection start.
// TODO (viktard): Empty lines in selection end.