Merge "Handle diff selection edge case"

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

@@ -156,11 +156,10 @@
 
     /**
      * Adjust triple click selection for the whole line.
-     * domRange.endContainer may be one of the following:
+     * A triple click always results in:
-     * 1) 0 offset at right column's line number cell, or
+     * - start.column == end.column == 0
-     * 2) 0 offset at left column's line number at the next line.
+     * - end.line == start.line + 1
-     * Case 1 means left column was triple clicked.
+     *
-     * Case 2 means right column or unified view triple clicked.
      * @param {!Object} range Normalized range, ie column/line numbers
      * @param {!Range} domRange DOM Range object
      * @return {!Object} fixed normalized range
@@ -172,20 +171,13 @@
       }
       const start = range.start;
       const end = range.end;
-      const endsAtOtherSideLineNum =
+      const endsAtBeginningOfNextLine = end &&
-          domRange.endOffset === 0 &&
+          start.column === 0 &&
-          domRange.endContainer.nodeName === 'TD' &&
-          (domRange.endContainer.classList.contains('left') ||
-              domRange.endContainer.classList.contains('right'));
-      const endsOnOtherSideStart = endsAtOtherSideLineNum ||
-          end &&
           end.column === 0 &&
-          end.line === start.line &&
+          end.line === start.line + 1;
-          end.side != start.side;
       const content = domRange.cloneContents().querySelector('.contentText');
       const lineLength = content && this._getLength(content) || 0;
-      if (lineLength && endsOnOtherSideStart || endsAtOtherSideLineNum) {
+      if (lineLength && endsAtBeginningOfNextLine) {
-        // Selection ends at the beginning of the next line.
         // Move the selection to the end of the previous line.
         range.end = {
           node: start.node,

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

@@ -576,54 +576,18 @@ limitations under the License.
         assert.equal(result, 0);
       });
 
-      // TODO (viktard): Selection starts in line number.
-      // TODO (viktard): Empty lines in selection start.
-      // TODO (viktard): Empty lines in selection end.
-      // TODO (viktard): Only empty lines selected.
-      // TODO (viktard): Unified mode.
-
-      suite('triple click', () => {
       test('_fixTripleClickSelection', () => {
-          const fakeRange = {
+        const startContent = stubContent(119, 'right');
-            startContainer: '',
+        const endContent = stubContent(120, 'right');
-            startOffset: '',
+        emulateSelection(startContent.firstChild, 0, endContent.firstChild, 0);
-            endContainer: '',
+        assert.isTrue(element.isRangeSelected());
-            endOffset: '',
-          };
-          const fixedRange = {};
-          sandbox.stub(GrRangeNormalizer, 'normalize').returns(fakeRange);
-          sandbox.stub(element, '_normalizeSelectionSide');
-          sandbox.stub(element, '_fixTripleClickSelection').returns(fixedRange);
-          assert.strictEqual(element._normalizeRange({}), fixedRange);
-          assert.isTrue(element._fixTripleClickSelection.called);
-        });
-
-        test('left pane', () => {
-          const startNode = stubContent(138, 'left');
-          const endNode =
-              stubContent(119, 'right').parentElement.previousElementSibling;
-          builder.getLineNumberByChild.withArgs(endNode).returns(119);
-          emulateSelection(startNode, 0, endNode, 0);
-          assert.deepEqual(getActionRange(), {
-            startLine: 138,
-            startChar: 0,
-            endLine: 138,
-            endChar: 63,
-          });
-        });
-
-        test('right pane', () => {
-          const startNode = stubContent(119, 'right');
-          const endNode =
-              stubContent(140, 'left').parentElement.previousElementSibling;
-          emulateSelection(startNode, 0, endNode, 0);
         assert.deepEqual(getActionRange(), {
           startLine: 119,
           startChar: 0,
           endLine: 119,
-            endChar: 63,
+          endChar: element._getLength(startContent),
-          });
         });
+        assert.equal(getActionSide(), 'right');
       });
     });
   });