Fix copy/paste in diff view

The addition of syntax highlighting silently broke copy/paste
functionality due to the addition of another layer of div nesting.

Related to this bug are some issues with correct text selection in
unified diff view, so this patch addresses them as well.

Bug: Issue 4317
Change-Id: Iac7379de4131ab4e44905a54218d42fcfe67ce62

polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js

@@ -340,6 +340,9 @@
         side,
         this._comments.meta.projectConfig);
     threadEl.comments = comments;
+    if (opt_side) {
+      threadEl.setAttribute('data-side', opt_side);
+    }
     return threadEl;
   };
 
@@ -363,11 +366,14 @@
 
   GrDiffBuilder.prototype._createTextEl = function(line, opt_side) {
     var td = this._createElement('td');
+    var text = line.text;
     if (line.type !== GrDiffLine.Type.BLANK) {
       td.classList.add('content');
+      if (!text) {
+        text = '\xa0';
+      }
     }
     td.classList.add(line.type);
-    var text = line.text;
     var html = util.escapeHTML(text);
     html = this._addTabWrappers(html, this._prefs.tab_size);
 

polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.html

@@ -18,17 +18,19 @@ limitations under the License.
 <dom-module id="gr-diff-selection">
   <template>
     <style>
-      .contentWrapper ::content .content {
+      .contentWrapper ::content .content,
+      .contentWrapper ::content .contextControl {
         -webkit-user-select: none;
         -moz-user-select: none;
         -ms-user-select: none;
         user-select: none;
       }
 
-      :host.selected-right .contentWrapper ::content .right + .content,
+      :host.selected-right .contentWrapper ::content .side-by-side .right + .content,
-      :host.selected-left .contentWrapper ::content .left + .content,
+      :host.selected-left .contentWrapper ::content .side-by-side .left + .content,
-      :host.selected-right .contentWrapper ::content .unified .right ~ .content,
+      :host.selected-right .contentWrapper ::content .unified .right.lineNum ~ .content,
-      :host.selected-left .contentWrapper ::content .unified .left ~ .content {
+      :host.selected-left .contentWrapper ::content .unified .left.lineNum ~ .content:not(.both),
+      :host .contentWrapper ::content .unified .gr-diff-comment .message {
         -webkit-user-select: text;
         -moz-user-select: text;
         -ms-user-select: text;

polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js

@@ -56,7 +56,8 @@
     },
 
     _handleCopy: function(e) {
-      if (!e.target.classList.contains('content')) {
+      if (!e.target.classList.contains('contentText') &&
+          !e.target.classList.contains('gr-syntax')) {
         return;
       }
       var lineEl = this.diffBuilder.getLineElByChild(e.target);
@@ -69,17 +70,17 @@
       e.preventDefault();
     },
 
-    _getSelectedText: function(opt_side) {
+    _getSelectedText: function(side) {
       var sel = window.getSelection();
       if (sel.rangeCount != 1) {
         return; // No multi-select support yet.
       }
       var range = sel.getRangeAt(0);
       var fragment = range.cloneContents();
-      var selector = '.content,td.content:nth-of-type(1)';
+      var selector = '.contentText';
-      if (opt_side) {
+      selector += '[data-side="' + side + '"]';
-        selector = '.' + opt_side + ' + ' + selector;
+      selector += ':not(:empty)';
-      }
+
       var contentEls = Polymer.dom(fragment).querySelectorAll(selector);
       if (contentEls.length === 0) {
         return fragment.textContent;

polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html

@@ -30,21 +30,33 @@ limitations under the License.
       <table>
         <tr>
           <td class="lineNum left">1</td>
-          <td class="content">ba ba</td>
+          <td class="content">
+            <div class="contentText" data-side="left">ba ba</div>
+          </td>
           <td class="lineNum right">1</td>
-          <td class="content">some other text</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="content">zin</td>
+          <td class="content">
+            <div class="contentText" data-side="left">zin</div>
+          </td>
           <td class="lineNum right">2</td>
-          <td class="content">more more more</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="content">ga ga</td>
+          <td class="content">
+            <div class="contentText" data-side="left">ga ga</div>
+          </td>
           <td class="lineNum right">3</td>
-          <td class="other">some other text</td>
+          <td class="other">
+            <div class="contentText" data-side="right">some other text</div>
+          </td>
         </tr>
       </table>
     </gr-diff-selection>
@@ -105,36 +117,37 @@ limitations under the License.
     test('asks for text for right side Elements', function() {
       element._cachedDiffBuilder.getSideByLineEl.returns('left');
       sinon.stub(element, '_getSelectedText');
-      emulateCopyOn(element.querySelector('td.content'));
+      emulateCopyOn(element.querySelector('div.contentText'));
       assert.deepEqual(['left'], element._getSelectedText.lastCall.args);
     });
 
     test('reacts to copy for content Elements', function() {
       sinon.stub(element, '_getSelectedText');
-      emulateCopyOn(element.querySelector('td.content'));
+      emulateCopyOn(element.querySelector('div.contentText'));
       assert.isTrue(element._getSelectedText.called);
     });
 
     test('copy event is prevented for content Elements', function() {
       sinon.stub(element, '_getSelectedText');
-      var event = emulateCopyOn(element.querySelector('td.content'));
+      var event = emulateCopyOn(element.querySelector('div.contentText'));
       assert.isTrue(event.preventDefault.called);
     });
 
     test('inserts text into clipboard on copy', function() {
       sinon.stub(element, '_getSelectedText').returns('the text');
-      var event = emulateCopyOn(element.querySelector('td.content'));
+      var event = emulateCopyOn(element.querySelector('div.contentText'));
       assert.deepEqual(
           ['Text', 'the text'], event.clipboardData.setData.lastCall.args);
     });
 
     test('copies content correctly', function() {
       element.classList.add('selected-left');
+      element.classList.remove('selected-right');
       var selection = window.getSelection();
       var range = document.createRange();
-      range.setStart(element.querySelector('td.content').firstChild, 3);
+      range.setStart(element.querySelector('div.contentText').firstChild, 3);
       range.setEnd(
-          element.querySelectorAll('td.content')[4].firstChild, 2);
+          element.querySelectorAll('div.contentText')[4].firstChild, 2);
       selection.addRange(range);
       assert.equal('ba\nzin\nga\n', element._getSelectedText('left'));
     });

polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html

@@ -91,13 +91,6 @@ limitations under the License.
         vertical-align: top;
         white-space: pre;
       }
-      .contentText:empty:before {
-        /**
-         * Insert glyph to prevent empty diff content from collapsing.
-         * "\200B" is a 'ZERO WIDTH SPACE' (U+200B)
-         */
-        content: "\200B";
-      }
       .contextLineNum:before,
       .lineNum:before {
         display: inline-block;