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 54294a1205..814a7601b0 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 @@ limitations under the License. + 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 62e58ad90a..9d7dc2ff2a 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 2612f9a93d..2f1ded94af 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 @@ limitations under the License. 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 0000000000..8685d7d57e --- /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 3c24291b0d..6a02a2dd00 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 @@ limitations under the License. + 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 35da901974..99c049b1fd 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 cdc7483857..1ac800d605 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 @@ limitations under the License. }); 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);