opendev/gerrit
Code Issues Proposed changes

Normalize selection ranges for copy

Browse Source 
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
This commit is contained in:
Wyatt Allen
2016-09-13 15:23:25 -07:00
parent 9fcec7437f
commit b1fbd9b310
7 changed files with 124 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -37,5 +37,6 @@ limitations under the License.
</div>
</template>
<script src="gr-annotation.js"></script>
<script src="gr-range-normalizer.js"></script>
<script src="gr-diff-highlight.js"></script>
</dom-module>

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

@@ -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;
},
});
})();

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

@@ -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.

106
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-range-normalizer.js Normal file
View File

@@ -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);

1
polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.html
View File

@@ -43,5 +43,6 @@ limitations under the License.
<content></content>
</div>
</template>
<script src="../gr-diff-highlight/gr-range-normalizer.js"></script>
<script src="gr-diff-selection.js"></script>
</dom-module>

2
polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
View File

@@ -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);

11
polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
View File

@@ -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);