Normalize selection ranges for copy
At the time that syntax highlighting DOM was introduced, the offsets of
selection ranges had been broken. In change [1] Kasper fixed this for
GR-DIFF-HIGHLIGHT with selection normalization functions. However,
selections for copying code as implemented in GR-DIFF-SELECTION were
still un-normalized.
With this change, the normalization functionality introduced in [1] is
moved to a JS library so that it can be used by both components. Tests
are updated.
[1] I26c61ca706575ea5df6e3b7b18a27225834396e8
Change-Id: I35ab0f71a46b3fc1d7356a314a0cae856f2ef28e
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html
index 54294a1..814a760 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html
@@ -37,5 +37,6 @@
</div>
</template>
<script src="gr-annotation.js"></script>
+ <script src="gr-range-normalizer.js"></script>
<script src="gr-diff-highlight.js"></script>
</dom-module>
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 62e58ad..9d7dc2f 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,40 +94,12 @@
}
},
- _getContentTextParent: function(target) {
- var element = target;
- if (element.nodeName === '#text') {
- element = element.parentElement;
- }
- while (!element.classList.contains('contentText')) {
- if (element.parentElement === null) {
- return target;
- }
- element = element.parentElement;
- }
- return element;
- },
-
- /**
- * Remap DOM range to whole lines of a diff if necessary. If the start or
- * end containers are DOM elements that are singular pieces of syntax
- * highlighting, the containers are remapped to the .contentText divs that
- * contain the entire line of code.
- *
- * @param {Object} range - the standard DOM selector range.
- * @return {Object} A modified version of the range that correctly accounts
- * for syntax highlighting.
- */
_normalizeRange: function(range) {
- var startContainer = this._getContentTextParent(range.startContainer);
- var startOffset = range.startOffset + this._getTextOffset(startContainer,
- range.startContainer);
- var endContainer = this._getContentTextParent(range.endContainer);
- var endOffset = range.endOffset + this._getTextOffset(endContainer,
- range.endContainer);
+ range = GrRangeNormalizer.normalize(range);
return {
- start: this._normalizeSelectionSide(startContainer, startOffset),
- end: this._normalizeSelectionSide(endContainer, endOffset),
+ start: this._normalizeSelectionSide(range.startContainer,
+ range.startOffset),
+ end: this._normalizeSelectionSide(range.endContainer, range.endOffset),
};
},
@@ -306,36 +278,5 @@
return GrAnnotation.getLength(node);
}
},
-
- /**
- * Gets the character offset of the child within the parent.
- * Performs a synchronous in-order traversal from top to bottom of the node
- * element, counting the length of the syntax until child is found.
- *
- * @param {!Element} The root DOM element to be searched through.
- * @param {!Element} The child element being searched for.
- * @return {number}
- */
- _getTextOffset: function(node, child) {
- var count = 0;
- var stack = [node];
- while (stack.length) {
- var n = stack.pop();
- if (n === child) {
- break;
- }
- if (n.childNodes && n.childNodes.length !== 0) {
- var arr = [];
- for (var i = 0; i < n.childNodes.length; i++) {
- arr.push(n.childNodes[i]);
- }
- arr.reverse();
- stack = stack.concat(arr);
- } else {
- count += this._getLength(n);
- }
- }
- return count;
- },
});
})();
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 2612f9a..2f1ded9 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
@@ -498,14 +498,14 @@
assert.notDeepEqual(spyCall.returnValue, range);
});
- test('_getTextOffset computes text offset', function() {
+ test('GrRangeNormalizer._getTextOffset computes text offset', function() {
var content = stubContent(140, 'left');
var child = content.lastChild.lastChild;
- var result = element._getTextOffset(content, child);
+ var result = GrRangeNormalizer._getTextOffset(content, child);
assert.equal(result, 73);
content = stubContent(146, 'right');
child = content.lastChild;
- result = element._getTextOffset(content, child);
+ result = GrRangeNormalizer._getTextOffset(content, child);
assert.equal(result, 0);
});
// TODO (viktard): Selection starts in line number.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-range-normalizer.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-range-normalizer.js
new file mode 100644
index 0000000..8685d7d
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-range-normalizer.js
@@ -0,0 +1,106 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the 'License');
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an 'AS IS' BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+(function(window) {
+ 'use strict';
+
+ // Prevent redefinition.
+ if (window.GrRangeNormalizer) { return; }
+
+ // Astral code point as per https://mathiasbynens.be/notes/javascript-unicode
+ var REGEX_ASTRAL_SYMBOL = /[\uD800-\uDBFF][\uDC00-\uDFFF]/;
+
+ var GrRangeNormalizer = {
+ /**
+ * Remap DOM range to whole lines of a diff if necessary. If the start or
+ * end containers are DOM elements that are singular pieces of syntax
+ * highlighting, the containers are remapped to the .contentText divs that
+ * contain the entire line of code.
+ *
+ * @param {Object} range - the standard DOM selector range.
+ * @return {Object} A modified version of the range that correctly accounts
+ * for syntax highlighting.
+ */
+ normalize: function(range) {
+ var startContainer = this._getContentTextParent(range.startContainer);
+ var startOffset = range.startOffset + this._getTextOffset(startContainer,
+ range.startContainer);
+ var endContainer = this._getContentTextParent(range.endContainer);
+ var endOffset = range.endOffset + this._getTextOffset(endContainer,
+ range.endContainer);
+ return {
+ startContainer: startContainer,
+ startOffset: startOffset,
+ endContainer: endContainer,
+ endOffset: endOffset,
+ };
+ },
+
+ _getContentTextParent: function(target) {
+ var element = target;
+ if (element.nodeName === '#text') {
+ element = element.parentElement;
+ }
+ while (!element.classList.contains('contentText')) {
+ if (element.parentElement === null) {
+ return target;
+ }
+ element = element.parentElement;
+ }
+ return element;
+ },
+
+ /**
+ * Gets the character offset of the child within the parent.
+ * Performs a synchronous in-order traversal from top to bottom of the node
+ * element, counting the length of the syntax until child is found.
+ *
+ * @param {!Element} The root DOM element to be searched through.
+ * @param {!Element} The child element being searched for.
+ * @return {number}
+ */
+ _getTextOffset: function(node, child) {
+ var count = 0;
+ var stack = [node];
+ while (stack.length) {
+ var n = stack.pop();
+ if (n === child) {
+ break;
+ }
+ if (n.childNodes && n.childNodes.length !== 0) {
+ var arr = [];
+ for (var i = 0; i < n.childNodes.length; i++) {
+ arr.push(n.childNodes[i]);
+ }
+ arr.reverse();
+ stack = stack.concat(arr);
+ } else {
+ count += this._getLength(n);
+ }
+ }
+ return count;
+ },
+
+ /**
+ * The DOM API textContent.length calculation is broken when the text
+ * contains Unicode. See https://mathiasbynens.be/notes/javascript-unicode .
+ * @param {Text} A text node.
+ * @return {Number} The length of the text.
+ */
+ _getLength: function(node) {
+ return node.textContent.replace(REGEX_ASTRAL_SYMBOL, '_').length;
+ },
+ };
+
+ window.GrRangeNormalizer = GrRangeNormalizer;
+})(window);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.html b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.html
index 3c24291..6a02a2d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.html
@@ -43,5 +43,6 @@
<content></content>
</div>
</template>
+ <script src="../gr-diff-highlight/gr-range-normalizer.js"></script>
<script src="gr-diff-selection.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
index 35da901..99c049b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
@@ -83,7 +83,7 @@
if (sel.rangeCount != 1) {
return; // No multi-select support yet.
}
- var range = sel.getRangeAt(0);
+ var range = GrRangeNormalizer.normalize(sel.getRangeAt(0));
var startLineEl = this.diffBuilder.getLineElByChild(range.startContainer);
var endLineEl = this.diffBuilder.getLineElByChild(range.endContainer);
var startLineNum = parseInt(startLineEl.getAttribute('data-value'), 10);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
index cdc7483..1ac800d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
@@ -157,12 +157,17 @@
});
test('copies content correctly', function() {
- element.classList.add('selected-left');
- element.classList.remove('selected-right');
// Fetch the line number.
element._cachedDiffBuilder.getLineElByChild = function(child) {
- return child.parentElement.parentElement.previousElementSibling;
+ while (!child.classList.contains('content') && child.parentElement) {
+ child = child.parentElement;
+ }
+ return child.previousElementSibling;
};
+
+ element.classList.add('selected-left');
+ element.classList.remove('selected-right');
+
var selection = window.getSelection();
var range = document.createRange();
range.setStart(element.querySelector('div.contentText').firstChild, 3);
