Enable multiline range comments in Firefox
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
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 e32d7d6..53eed73 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
@@ -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.
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 827437c..c4c2993 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
@@ -304,6 +304,37 @@
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 @@
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.
