Fix comment copy logic
The addition of formatting in comments broke a variety of things having
to do with the copying logic. This change updates the logic and tests
to reflect the new DOM.
This issue arose because of a lack of integration tests for copying and
selection. That test is coming in a descendant change.
Bug: Issue 4969
Change-Id: I4e1994ab07947506c77b07877a46a9369d666d50
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 24887e0..6ba3ff8 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
@@ -59,7 +59,7 @@
return;
}
var commentSelected =
- e.target.parentNode.classList.contains('gr-diff-comment');
+ this._elementDescendedFromClass(e.target, 'gr-diff-comment');
var side = this.diffBuilder.getSideByLineEl(lineEl);
var targetClasses = [];
targetClasses.push(side === 'left' ?
@@ -215,15 +215,6 @@
_getCommentLines: function(sel, side) {
var range = sel.getRangeAt(0);
var content = [];
- // Fall back to default copy behavior if the selection lies within one
- // comment body.
- if (range.startContainer === range.endContainer) {
- return;
- }
- if (this._elementDescendedFromClass(range.commonAncestorContainer,
- 'message')) {
- return;
- }
// Query the diffElement for comments.
var messages = this.diffBuilder.diffElement.querySelectorAll(
'.side-by-side [data-side="' + side +
@@ -233,15 +224,25 @@
var el = messages[i];
// Check if the comment element exists inside the selection.
if (sel.containsNode(el, true)) {
- content.push(el.textContent);
+ // Padded elements require newlines for accurate spacing.
+ if (el.parentElement.id === 'container' ||
+ el.parentElement.nodeName === 'BLOCKQUOTE') {
+ if (content.length && content[content.length - 1] !== '') {
+ content.push('');
+ }
+ }
+
+ if (!el.children.length) {
+ content.push(el.textContent);
+ }
}
}
- // Deal with offsets.
- content[0] = content[0].substring(range.startOffset);
+
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');
},
});
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 d6a6298..a5c26e1 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
@@ -34,8 +34,8 @@
<div class="contentText" data-side="left">ba ba</div>
<div data-side="left">
<div class="gr-diff-comment-thread">
- <div class="message">
- <span>This is a comment</span>
+ <div class="gr-formatted-text message">
+ <span class="gr-linked-text">This is a comment</span>
</div>
</div>
</div>
@@ -55,8 +55,8 @@
<div class="contentText" data-side="right">more more more</div>
<div data-side="right">
<div class="gr-diff-comment-thread">
- <div class="message">
- <span>This is a comment on the right</span>
+ <div class="gr-formatted-text message">
+ <span class="gr-linked-text">This is a comment on the right</span>
</div>
</div>
</div>
@@ -68,8 +68,8 @@
<div class="contentText" data-side="left">ga ga</div>
<div data-side="left">
<div class="gr-diff-comment-thread">
- <div class="message">
- <span>This is a different comment</span>
+ <div class="gr-formatted-text message">
+ <span class="gr-linked-text">This is a different comment</span>
</div>
</div>
</div>
@@ -213,13 +213,13 @@
element.classList.remove('selected-right');
var selection = window.getSelection();
+ selection.removeAllRanges();
var range = document.createRange();
range.setStart(element.querySelector('div.contentText').firstChild, 3);
range.setEnd(
element.querySelectorAll('div.contentText')[4].firstChild, 2);
selection.addRange(range);
assert.equal(element._getSelectedText('left'), 'ba\nzin\nga');
- selection.removeAllRanges();
});
test('copies comments', function() {
@@ -227,14 +227,15 @@
element.classList.add('selected-comment');
element.classList.remove('selected-right');
var selection = window.getSelection();
+ selection.removeAllRanges();
var range = document.createRange();
- range.setStart(element.querySelector('.message *').firstChild, 3);
+ range.setStart(
+ element.querySelector('.gr-formatted-text *').firstChild, 3);
range.setEnd(
- element.querySelectorAll('.message *')[2].firstChild, 16);
+ element.querySelectorAll('.gr-formatted-text *')[2].firstChild, 16);
selection.addRange(range);
assert.equal('s is a comment\nThis is a differ',
element._getSelectedText('left', true));
- selection.removeAllRanges();
});
test('defers to default behavior for textarea', function() {
@@ -257,6 +258,7 @@
element.classList.remove('selected-left');
var selection = window.getSelection();
+ selection.removeAllRanges();
var range = document.createRange();
range.setStart(
element.querySelectorAll('div.contentText')[1].firstChild, 4);
@@ -264,7 +266,6 @@
element.querySelectorAll('div.contentText')[1].firstChild, 10);
selection.addRange(range);
assert.equal(element._getSelectedText('right'), ' other');
- selection.removeAllRanges();
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 1d63b47..f8f646a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -241,6 +241,7 @@
_handleCKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (this.$.diff.isRangeSelected()) { return; }
+ if (this.modifierPressed(e)) { return; }
e.preventDefault();
var line = this.$.cursor.getTargetLineElement();
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
index 7496e59..90c37cd 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
@@ -32,7 +32,7 @@
_commentMap: {
type: Object,
value: function() { return {left: [], right: []}; },
- }
+ },
},
observers: [
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index c610073..3046534 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -80,6 +80,7 @@
_handleCKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (this.modifierPressed(e)) { return; }
e.preventDefault();
this._fireCreateComment();
