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]]"