opendev/gerrit
Code Issues Proposed changes

Fix copy within same line

Browse Source 
When the text selection exists within one line in diff-view, the
derivation of the selected text from the diff object using substring()
mutated the text and caused some selections to be artificially long.
Swapping the calls so that the end of the string is trimmed off first
fixes this.

This change also includes a regression test for this issue.

Bug: Issue 4794
Change-Id: I19dbab1cddd2e522cc6495011e8ef49f8ffe55b2
This commit is contained in:
Kasper Nilsson
2016-10-20 10:26:47 -07:00
parent 4e1d1b8427
commit bc0c634347
2 changed files with 23 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -170,9 +170,9 @@
endOffset, side) {
var lines = this._getDiffLines(side).slice(startLineNum - 1, endLineNum);
if (lines.length) {
lines[0] = lines[0].substring(startOffset);
lines[lines.length - 1] = lines[lines.length - 1]
.substring(0, endOffset);
lines[0] = lines[0].substring(startOffset);
}
return lines.join('\n');
},

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

@@ -244,5 +244,27 @@ limitations under the License.
emulateCopyOn(element.querySelector('textarea'));
assert.isFalse(selectedTextSpy.called);
});
test('regression test for 4794', function() {
element._cachedDiffBuilder.getLineElByChild = function(child) {
while (!child.classList.contains('content') && child.parentElement) {
child = child.parentElement;
}
return child.previousElementSibling;
};
element.classList.add('selected-right');
element.classList.remove('selected-left');
var selection = window.getSelection();
var range = document.createRange();
range.setStart(
element.querySelectorAll('div.contentText')[1].firstChild, 4);
range.setEnd(
element.querySelectorAll('div.contentText')[1].firstChild, 10);
selection.addRange(range);
assert.equal(element._getSelectedText('right'), ' other');
selection.removeAllRanges();
});
});
</script>