Fix copy/paste in comments

Previous comment logic relied on the assumption that any element in
a comment with textContent would be childless, which is false. Now, the
method is to fetch all comments within the selection and recursively
extract their text content.

Bug: Issue 5144
Change-Id: I22eac0b9b147fe0f3360d87529661efadb15940a
This commit is contained in:
Kasper Nilsson
2016-12-15 17:23:07 -08:00
parent aaeaf09a96
commit 61e253ca6a
3 changed files with 97 additions and 14 deletions

View File

@@ -31,8 +31,8 @@ limitations under the License.
:host-context(.selected-left:not(.selected-comment)) .contentWrapper ::content .unified .left.lineNum ~ .content:not(.both) .contentText,
:host-context(.selected-right:not(.selected-comment)) .contentWrapper ::content .unified .right.lineNum ~ .content .contentText,
:host-context(.selected-left.selected-comment) .contentWrapper ::content .side-by-side .left + .content .message,
:host-context(.selected-right.selected-comment) .contentWrapper ::content .side-by-side .right + .content .message,
:host-context(.selected-comment) .contentWrapper ::content .unified .message {
:host-context(.selected-right.selected-comment) .contentWrapper ::content .side-by-side .right + .content .message :not(.collapsedContent),
:host-context(.selected-comment) .contentWrapper ::content .unified .message :not(.collapsedContent){
-webkit-user-select: text;
-moz-user-select: text;
-ms-user-select: text;

View File

@@ -213,7 +213,7 @@
* @return {string} The selected comment text.
*/
_getCommentLines: function(sel, side) {
var range = sel.getRangeAt(0);
var range = GrRangeNormalizer.normalize(sel.getRangeAt(0));
var content = [];
// Query the diffElement for comments.
var messages = this.diffBuilder.diffElement.querySelectorAll(
@@ -232,18 +232,45 @@
}
}
if (!el.children.length) {
content.push(el.textContent);
if (el.id === 'output' &&
!this._elementDescendedFromClass(el, 'collapsed')) {
content.push(this._getTextContentForRange(el, sel, range));
}
}
}
if (range.endOffset) {
content[content.length - 1] =
content[content.length - 1].substring(0, range.endOffset);
}
content[0] = content[0].substring(range.startOffset);
return content.join('\n');
},
/**
* Given a DOM node, a selection, and a selection range, recursively get all
* of the text content within that selection.
* Using a domNode that isn't in the selection returns an empty string.
*
* @param {Element} domNode The root DOM node.
* @param {Selection} sel The selection.
* @param {Range} range The normalized selection range.
* @return {string} The text within the selection.
*/
_getTextContentForRange: function(domNode, sel, range) {
if (!sel.containsNode(domNode, true)) { return ''; }
var text = '';
if (domNode instanceof Text) {
text = domNode.textContent;
if (domNode === range.endContainer) {
text = text.substring(0, range.endOffset);
}
if (domNode === range.startContainer) {
text = text.substring(range.startOffset);
}
} else {
for (var i = 0; i < domNode.childNodes.length; i++) {
text += this._getTextContentForRange(domNode.childNodes[i],
sel, range);
}
}
return text;
},
});
})();

View File

@@ -35,7 +35,7 @@ limitations under the License.
<div data-side="left">
<div class="gr-diff-comment-thread">
<div class="gr-formatted-text message">
<span class="gr-linked-text">This is a comment</span>
<span id="output" class="gr-linked-text">This is a comment</span>
</div>
</div>
</div>
@@ -56,7 +56,7 @@ limitations under the License.
<div data-side="right">
<div class="gr-diff-comment-thread">
<div class="gr-formatted-text message">
<span class="gr-linked-text">This is a comment on the right</span>
<span id="output" class="gr-linked-text">This is a comment on the right</span>
</div>
</div>
</div>
@@ -69,7 +69,7 @@ limitations under the License.
<div data-side="left">
<div class="gr-diff-comment-thread">
<div class="gr-formatted-text message">
<span class="gr-linked-text">This is a different comment</span>
<span id="output" class="gr-linked-text">This is <a>a</a> different comment 💩 unicode is fun</span>
</div>
</div>
</div>
@@ -232,12 +232,27 @@ limitations under the License.
range.setStart(
element.querySelector('.gr-formatted-text *').firstChild, 3);
range.setEnd(
element.querySelectorAll('.gr-formatted-text *')[2].firstChild, 16);
element.querySelectorAll('.gr-formatted-text *')[2].childNodes[2], 7);
selection.addRange(range);
assert.equal('s is a comment\nThis is a differ',
element._getSelectedText('left', true));
});
test('respects astral chars in comments', function() {
element.classList.add('selected-left');
element.classList.add('selected-comment');
element.classList.remove('selected-right');
var selection = window.getSelection();
selection.removeAllRanges();
var range = document.createRange();
var nodes = element.querySelectorAll('.gr-formatted-text *');
range.setStart(nodes[2].childNodes[2], 13);
range.setEnd(nodes[2].childNodes[2], 23);
selection.addRange(range);
assert.equal('mment 💩 u',
element._getSelectedText('left', true));
});
test('defers to default behavior for textarea', function() {
element.classList.add('selected-left');
element.classList.remove('selected-right');
@@ -267,5 +282,46 @@ limitations under the License.
selection.addRange(range);
assert.equal(element._getSelectedText('right'), ' other');
});
suite('_getTextContentForRange', function() {
var selection;
var range;
var nodes;
setup(function() {
element.classList.add('selected-left');
element.classList.add('selected-comment');
element.classList.remove('selected-right');
selection = window.getSelection();
selection.removeAllRanges();
range = document.createRange();
nodes = element.querySelectorAll('.gr-formatted-text *');
});
test('multi level element contained in range', function() {
range.setStart(nodes[2].childNodes[0], 1);
range.setEnd(nodes[2].childNodes[2], 7);
selection.addRange(range);
assert.equal(element._getTextContentForRange(element, selection, range),
'his is a differ');
});
test('multi level element as startContainer of range', function() {
range.setStart(nodes[2].childNodes[1], 0);
range.setEnd(nodes[2].childNodes[2], 7);
selection.addRange(range);
assert.equal(element._getTextContentForRange(element, selection, range),
'a differ');
});
test('startContainer === endContainer', function() {
range.setStart(nodes[0].firstChild, 2);
range.setEnd(nodes[0].firstChild, 12);
selection.addRange(range);
assert.equal(element._getTextContentForRange(element, selection, range),
'is is a co');
});
});
});
</script>