diff options
author | viktard <viktard@google.com> | 2018-10-17 14:42:50 -0700 |
---|---|---|
committer | Paladox none <thomasmulhall410@yahoo.com> | 2018-10-19 16:32:09 +0000 |
commit | 0f9c0c739b6fcd50bd8126ec0dd86562b314a148 (patch) | |
tree | a48780fc2a68b5c7e54786dccec71d3f25db606f | |
parent | 9abd4415cda61758e59cd680d0dbe6876628a496 (diff) |
Fix triple click select and copy when other diff side is empty
Triple click in side-by-side mode with other side empty results in DOM
selection ending on the empty div line number tag. Normalizing such
selection results in null element for the end container.
This is handled on ad-hoc basis by validating where DOM selection landed
and is handled properly.
Previous fix (https://gerrit-review.googlesource.com/c/gerrit/+/193470)
was only addressing the case when the other diff side is populated.
Bug: Issue 8431
Change-Id: I17964db98c217d30dd266e01d0a03a11ed9c42b9
(cherry picked from commit 05e0f2060028c5cd34b9418678050d2067238441)
3 files changed, 35 insertions, 5 deletions
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js index cee3caddf5..af8725e772 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js @@ -171,13 +171,19 @@ } const start = range.start; const end = range.end; + // Happens when triple click in side-by-side mode with other side empty. + const endsAtOtherEmptySide = !end && + domRange.endOffset === 0 && + domRange.endContainer.nodeName === 'TD' && + (domRange.endContainer.classList.contains('left') || + domRange.endContainer.classList.contains('right')); const endsAtBeginningOfNextLine = end && start.column === 0 && end.column === 0 && end.line === start.line + 1; const content = domRange.cloneContents().querySelector('.contentText'); const lineLength = content && this._getLength(content) || 0; - if (lineLength && endsAtBeginningOfNextLine) { + if (lineLength && (endsAtBeginningOfNextLine || endsAtOtherEmptySide)) { // Move the selection to the end of the previous line. range.end = { node: start.node, diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html index 7b19338db7..b10e3cc167 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html @@ -123,7 +123,7 @@ limitations under the License. <tbody class="section both"> <tr class="diff-row side-by-side" left-type="both" right-type="both"> <td class="left lineNum" data-value="165"></td> - <td class="content both"><div class="contentText">in physicis, quibus maxime gloriatur, primum totus est alienus. Democritea dicit</div></td> + <td class="content both"><div class="contentText"></div></td> <td class="right lineNum" data-value="147"></td> <td class="content both"><div class="contentText">in physicis, <hl><span class="tab-indicator" style="tab-size:8;"> </span></hl> quibus maxime gloriatur, primum totus est alienus. Democritea dicit</div></td> </tr> @@ -589,6 +589,21 @@ limitations under the License. }); assert.equal(getActionSide(), 'right'); }); + + test('_fixTripleClickSelection empty line', () => { + const startContent = stubContent(146, 'right'); + const endContent = stubContent(165, 'left'); + emulateSelection(startContent.firstChild, 0, + endContent.parentElement.previousElementSibling, 0); + assert.isTrue(element.isRangeSelected()); + assert.deepEqual(getActionRange(), { + startLine: 146, + startChar: 0, + endLine: 146, + endChar: 84, + }); + assert.equal(getActionSide(), 'right'); + }); }); }); </script> 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 35e2fe1a23..27e467dcf8 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 @@ -179,10 +179,19 @@ const startLineEl = this.diffBuilder.getLineElByChild(range.startContainer); const endLineEl = this.diffBuilder.getLineElByChild(range.endContainer); + // Happens when triple click in side-by-side mode with other side empty. + const endsAtOtherEmptySide = !endLineEl && + range.endOffset === 0 && + range.endContainer.nodeName === 'TD' && + (range.endContainer.classList.contains('left') || + range.endContainer.classList.contains('right')); const startLineNum = parseInt(startLineEl.getAttribute('data-value'), 10); - const endLineNum = endLineEl === null ? - undefined : - parseInt(endLineEl.getAttribute('data-value'), 10); + let endLineNum; + if (endsAtOtherEmptySide) { + endLineNum = startLineNum + 1; + } else if (endLineEl) { + endLineNum = parseInt(endLineEl.getAttribute('data-value'), 10); + } return this._getRangeFromDiff(startLineNum, range.startOffset, endLineNum, range.endOffset, side); |