Generate clipboard data via diff rather than DOM reconstruction
This change uses a new method of generating the clipboard data that
circumvents several strange issues caused by shadow/shady DOM. This
new method is much more legible and abstracts more logic away from the
DOM.
Bug: Issue 4494
Change-Id: I8c186d6cbbe9536548d934f734856b1f9ced1a26
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 bc55129..3c24291 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
@@ -26,11 +26,13 @@
user-select: none;
}
- :host.selected-right .contentWrapper ::content .side-by-side .right + .content,
- :host.selected-left .contentWrapper ::content .side-by-side .left + .content,
- :host.selected-right .contentWrapper ::content .unified .right.lineNum ~ .content,
- :host.selected-left .contentWrapper ::content .unified .left.lineNum ~ .content:not(.both),
- :host .contentWrapper ::content .unified .gr-diff-comment .message {
+ :host-context(.selected-left:not(.selected-comment)) .contentWrapper ::content .side-by-side .left + .content .contentText,
+ :host-context(.selected-right:not(.selected-comment)) .contentWrapper ::content .side-by-side .right + .content .contentText,
+ :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 {
-webkit-user-select: text;
-moz-user-select: text;
-ms-user-select: text;
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 80e758e..35da901 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
@@ -18,7 +18,12 @@
is: 'gr-diff-selection',
properties: {
+ diff: Object,
_cachedDiffBuilder: Object,
+ _linesCache: {
+ type: Object,
+ value: function() { return {left: null, right: null}; },
+ },
},
listeners: {
@@ -56,9 +61,12 @@
},
_handleCopy: function(e) {
- if (!e.target.classList.contains('contentText') &&
- !e.target.classList.contains('gr-syntax')) {
- return;
+ var el = e.target;
+ while (!el.classList.contains('content')) {
+ if (!el.parentElement) {
+ return;
+ }
+ el = el.parentElement;
}
var lineEl = this.diffBuilder.getLineElByChild(e.target);
if (!lineEl) {
@@ -76,21 +84,47 @@
return; // No multi-select support yet.
}
var range = sel.getRangeAt(0);
- var fragment = range.cloneContents();
- var selector = '.contentText';
- selector += '[data-side="' + side + '"]';
- selector += ':not(:empty)';
+ var startLineEl = this.diffBuilder.getLineElByChild(range.startContainer);
+ var endLineEl = this.diffBuilder.getLineElByChild(range.endContainer);
+ var startLineNum = parseInt(startLineEl.getAttribute('data-value'), 10);
+ var endLineNum = parseInt(endLineEl.getAttribute('data-value'), 10);
- var contentEls = Polymer.dom(fragment).querySelectorAll(selector);
- if (contentEls.length === 0) {
- return fragment.textContent;
+ return this._getRangeFromDiff(startLineNum, range.startOffset, endLineNum,
+ range.endOffset, side);
+ },
+
+ _getRangeFromDiff: function(startLineNum, startOffset, endLineNum,
+ 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);
+ }
+ return lines.join('\n');
+ },
+
+ _getDiffLines: function(side) {
+ if (this._linesCache[side]) {
+ return this._linesCache[side];
}
- var text = '';
- for (var i = 0; i < contentEls.length; i++) {
- text += contentEls[i].textContent + '\n';
+ var lines = [];
+ var chunk;
+ var key = side === 'left' ? 'a' : 'b';
+ for (var chunkIndex = 0;
+ chunkIndex < this.diff.content.length;
+ chunkIndex++) {
+ chunk = this.diff.content[chunkIndex];
+ if (chunk.ab) {
+ lines = lines.concat(chunk.ab);
+ } else if (chunk[key]) {
+ lines = lines.concat(chunk[key]);
+ }
}
- return text;
+
+ this._linesCache[side] = lines;
+ return lines;
},
});
})();
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 17512f3..cdc7483 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
@@ -29,31 +29,31 @@
<gr-diff-selection>
<table class="side-by-side">
<tr>
- <td class="lineNum left">1</td>
+ <td class="lineNum left" data-value="1">1</td>
<td class="content">
<div class="contentText" data-side="left">ba ba</div>
</td>
- <td class="lineNum right">1</td>
+ <td class="lineNum right" data-value="1">1</td>
<td class="content">
<div class="contentText" data-side="right">some other text</div>
</td>
</tr>
<tr>
- <td class="lineNum left">2</td>
+ <td class="lineNum left" data-value="2">2</td>
<td class="content">
<div class="contentText" data-side="left">zin</div>
</td>
- <td class="lineNum right">2</td>
+ <td class="lineNum right" data-value="2">2</td>
<td class="content">
<div class="contentText" data-side="right">more more more</div>
</td>
</tr>
<tr>
- <td class="lineNum left">2</td>
+ <td class="lineNum left" data-value="3">3</td>
<td class="content">
<div class="contentText" data-side="left">ga ga</div>
</td>
- <td class="lineNum right">3</td>
+ <td class="lineNum right" data-value="3">3</td>
<td class="other">
<div class="contentText" data-side="right">some other text</div>
</td>
@@ -85,6 +85,22 @@
getLineElByChild: sinon.stub().returns({}),
getSideByLineEl: sinon.stub(),
};
+ element.diff = {
+ content: [
+ {
+ a: ['ba ba'],
+ b: ['some other text'],
+ },
+ {
+ a: ['zin'],
+ b: ['more more more'],
+ },
+ {
+ a: ['ga ga'],
+ b: ['some other text'],
+ },
+ ],
+ };
});
test('applies selected-left on left side click', function() {
@@ -143,13 +159,17 @@
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;
+ };
var selection = window.getSelection();
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('ba\nzin\nga\n', element._getSelectedText('left'));
+ assert.equal(element._getSelectedText('left'), 'ba\nzin\nga');
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 36a18fc..4672b31 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -155,7 +155,7 @@
<style include="gr-theme-default"></style>
<div class$="[[_computeContainerClass(_loggedIn, viewMode)]]"
on-tap="_handleTap">
- <gr-diff-selection>
+ <gr-diff-selection diff="[[_diff]]">
<gr-diff-highlight
id="highlights"
logged-in="[[_loggedIn]]"
