From 9fcec7437f932d608fb0eb05efe474dad34d8a6e Mon Sep 17 00:00:00 2001 From: Wyatt Allen Date: Thu, 8 Sep 2016 14:09:11 -0700 Subject: [PATCH] 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 --- .../gr-diff-selection/gr-diff-selection.html | 12 ++-- .../gr-diff-selection/gr-diff-selection.js | 62 ++++++++++++++----- .../gr-diff-selection_test.html | 34 +++++++--- .../app/elements/diff/gr-diff/gr-diff.html | 2 +- 4 files changed, 83 insertions(+), 27 deletions(-) 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 bc551299bc..3c24291b0d 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 @@ limitations under the License. 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 80e758ec7f..35da901974 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 17512f3f51..cdc7483857 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 @@ limitations under the License. - + - + - + - + - + - + @@ -85,6 +85,22 @@ limitations under the License. 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 @@ 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; + }; 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'); }); }); 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 36a18fc955..4672b318a4 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 @@ limitations under the License.
- +
11
ba ba
11
some other text
22
zin
22
more more more
23
ga ga
33
some other text