From da230412574daa9697b4cef24c7be6209b8884dc Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 26 Jun 2019 23:35:07 +0200 Subject: [PATCH] fix extra newlines when copying from diff in Firefox (#7288) * fix extra newlines when copying from diff See https://bugzilla.mozilla.org/show_bug.cgi?id=1273836 Basically, the
 seems to add a forced newline that is not
possible to get rid of via CSS, so I replaced it with just a .

Secondly, .lines-type-marker also forced a newline in the copied text,
but that was possible to get rid of via user-select.

Safari still has a extraneous newline in the copied text of unknown
origin, but this should not block stop this PR.

* simplify .line-type-marker

* fix selector

* remove erronous ^^^

* Fix empty split diff

* Fix arc-theme-green

* fix add comment

* ensure line-num is copied too

* Update templates/repo/diff/box.tmpl

Co-Authored-By: zeripath 

* attempt to fix safari via removing 

* remove useless whitespace at the end of 'class'

* remove inter-tag whitespace for code s

* more inter-tag removal

* final inter-tag removal

* attempt to fix empty line copy

* move and comment getLineContent

* fix golint

* make background grey for missing added code
---
 models/git_diff.go                       | 24 ++++++++-----
 public/css/index.css                     | 15 ++++-----
 public/css/theme-arc-green.css           |  6 ++--
 public/js/index.js                       |  4 +--
 public/less/_base.less                   |  3 +-
 public/less/_repository.less             | 11 +++---
 public/less/themes/arc-green.less        |  8 +++--
 templates/repo/diff/box.tmpl             | 43 +++++++++---------------
 templates/repo/diff/section_unified.tmpl | 19 +++--------
 9 files changed, 59 insertions(+), 74 deletions(-)

diff --git a/models/git_diff.go b/models/git_diff.go
index bc79a73a4e..2f48f1b6fa 100644
--- a/models/git_diff.go
+++ b/models/git_diff.go
@@ -88,6 +88,14 @@ func (d *DiffLine) GetLineTypeMarker() string {
 	return ""
 }
 
+// escape a line's content or return 
needed for copy/paste purposes +func getLineContent(content string) string { + if len(content) > 0 { + return html.EscapeString(content) + } + return "
" +} + // DiffSection represents a section of a DiffFile. type DiffSection struct { Name string @@ -107,14 +115,14 @@ func diffToHTML(diffs []diffmatchpatch.Diff, lineType DiffLineType) template.HTM switch { case diffs[i].Type == diffmatchpatch.DiffInsert && lineType == DiffLineAdd: buf.Write(addedCodePrefix) - buf.WriteString(html.EscapeString(diffs[i].Text)) + buf.WriteString(getLineContent(diffs[i].Text)) buf.Write(codeTagSuffix) case diffs[i].Type == diffmatchpatch.DiffDelete && lineType == DiffLineDel: buf.Write(removedCodePrefix) - buf.WriteString(html.EscapeString(diffs[i].Text)) + buf.WriteString(getLineContent(diffs[i].Text)) buf.Write(codeTagSuffix) case diffs[i].Type == diffmatchpatch.DiffEqual: - buf.WriteString(html.EscapeString(diffs[i].Text)) + buf.WriteString(getLineContent(diffs[i].Text)) } } @@ -173,7 +181,7 @@ func init() { // GetComputedInlineDiffFor computes inline diff for the given line. func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine) template.HTML { if setting.Git.DisableDiffHighlight { - return template.HTML(html.EscapeString(diffLine.Content[1:])) + return template.HTML(getLineContent(diffLine.Content[1:])) } var ( compareDiffLine *DiffLine @@ -186,22 +194,22 @@ func (diffSection *DiffSection) GetComputedInlineDiffFor(diffLine *DiffLine) tem case DiffLineAdd: compareDiffLine = diffSection.GetLine(DiffLineDel, diffLine.RightIdx) if compareDiffLine == nil { - return template.HTML(html.EscapeString(diffLine.Content[1:])) + return template.HTML(getLineContent(diffLine.Content[1:])) } diff1 = compareDiffLine.Content diff2 = diffLine.Content case DiffLineDel: compareDiffLine = diffSection.GetLine(DiffLineAdd, diffLine.LeftIdx) if compareDiffLine == nil { - return template.HTML(html.EscapeString(diffLine.Content[1:])) + return template.HTML(getLineContent(diffLine.Content[1:])) } diff1 = diffLine.Content diff2 = compareDiffLine.Content default: if strings.IndexByte(" +-", diffLine.Content[0]) > -1 { - return template.HTML(html.EscapeString(diffLine.Content[1:])) + return template.HTML(getLineContent(diffLine.Content[1:])) } - return template.HTML(html.EscapeString(diffLine.Content)) + return template.HTML(getLineContent(diffLine.Content)) } diffRecord := diffMatchPatch.DiffMain(diff1[1:], diff2[1:], true) diff --git a/public/css/index.css b/public/css/index.css index 475a54f75f..5e2c7908e9 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -47,9 +47,9 @@ img{border-radius:3px} table{border-collapse:collapse} a{cursor:pointer} .rounded{border-radius:.28571429rem!important} -code,pre{font:12px 'SF Mono',Consolas,Menlo,'Liberation Mono',Monaco,'Lucida Console',monospace} -code.raw,pre.raw{padding:7px 12px;margin:10px 0;background-color:#f8f8f8;border:1px solid #ddd;border-radius:3px;font-size:13px;line-height:1.5;overflow:auto} -code.wrap,pre.wrap{white-space:pre-wrap;word-break:break-all;overflow-wrap:break-word;word-wrap:break-word} +.mono,code,pre{font:12px 'SF Mono',Consolas,Menlo,'Liberation Mono',Monaco,'Lucida Console',monospace} +.mono.raw,code.raw,pre.raw{padding:7px 12px;margin:10px 0;background-color:#f8f8f8;border:1px solid #ddd;border-radius:3px;font-size:13px;line-height:1.5;overflow:auto} +.mono.wrap,code.wrap,pre.wrap{white-space:pre-wrap;word-break:break-all;overflow-wrap:break-word;word-wrap:break-word} .dont-break-out{overflow-wrap:break-word;word-wrap:break-word;word-break:break-all;-webkit-hyphens:auto;-ms-hyphens:auto;hyphens:auto} .full.height{flex-grow:1;padding-bottom:80px} .following.bar{z-index:900;left:0;margin:0!important} @@ -638,19 +638,18 @@ footer .ui.left,footer .ui.right{line-height:40px} .repository .diff-file-box .file-body.file-code .lines-num-old{border-right:1px solid #ddd} .repository .diff-file-box .code-diff{font-size:12px} .repository .diff-file-box .code-diff td{padding:0 0 0 10px;border-top:0} -.repository .diff-file-box .code-diff pre{margin:0} .repository .diff-file-box .code-diff .lines-num{border-color:#d4d4d5;border-right-width:1px;border-right-style:solid;padding:0 5px} .repository .diff-file-box .code-diff tbody tr td.halfwidth{width:49%} .repository .diff-file-box .code-diff tbody tr td.tag-code,.repository .diff-file-box .code-diff tbody tr.tag-code td{background-color:#f0f0f0!important;border-color:#d3cfcf!important;padding-top:8px;padding-bottom:8px} .repository .diff-file-box .code-diff tbody tr .removed-code{background-color:#f99} .repository .diff-file-box .code-diff tbody tr .added-code{background-color:#9f9} -.repository .diff-file-box .code-diff tbody tr .lines-num[data-line-num]::before{content:attr(data-line-num);text-align:right} -.repository .diff-file-box .code-diff tbody tr .lines-type-marker{width:10px;min-width:10px} -.repository .diff-file-box .code-diff tbody tr .line-type-marker[data-type-marker]::before{content:attr(data-type-marker);text-align:right;display:inline-block} +.repository .diff-file-box .code-diff tbody tr [data-line-num]::before{content:attr(data-line-num);text-align:right} +.repository .diff-file-box .code-diff tbody tr .lines-type-marker{width:10px;min-width:10px;-webkit-user-select:none;-moz-user-select:none;-ms-user-select:none;user-select:none} +.repository .diff-file-box .code-diff tbody tr [data-type-marker]::before{content:attr(data-type-marker);text-align:right;display:inline-block} .repository .diff-file-box .code-diff-unified tbody tr.del-code td{background-color:#ffe0e0!important;border-color:#f1c0c0!important} .repository .diff-file-box .code-diff-unified tbody tr.add-code td{background-color:#d6fcd6!important;border-color:#c1e9c1!important} .repository .diff-file-box .code-diff-split table,.repository .diff-file-box .code-diff-split tbody{width:100%} -.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(2),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(5),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(6){background-color:#fafafa} +.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(2),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(3),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(5),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(6){background-color:#fafafa} .repository .diff-file-box .code-diff-split tbody tr td.del-code,.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(2),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3){background-color:#ffe0e0!important;border-color:#f1c0c0!important} .repository .diff-file-box .code-diff-split tbody tr td.add-code,.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(4),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(5),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(6){background-color:#d6fcd6!important;border-color:#c1e9c1!important} .repository .diff-file-box .code-diff-split tbody tr td:nth-child(4){border-left-width:1px;border-left-style:solid} diff --git a/public/css/theme-arc-green.css b/public/css/theme-arc-green.css index 05cc6e5ebb..f61fd2c96e 100644 --- a/public/css/theme-arc-green.css +++ b/public/css/theme-arc-green.css @@ -216,9 +216,9 @@ a.ui.label:hover,a.ui.labels .label:hover{background-color:#505667;color:#dbdbdb .ui.basic.blue.button,.ui.basic.blue.buttons .button{box-shadow:0 0 0 1px #a27558 inset!important;color:#a27558!important} .repository.file.list #file-content .code-view .hljs,.repository.file.list #file-content .code-view .lines-code ol,.repository.file.list #file-content .code-view .lines-code pre,.repository.file.list #file-content .code-view .lines-num .hljs,.repository.file.list #file-content .code-view .lines-num ol,.repository.file.list #file-content .code-view .lines-num pre{background-color:#2a2e3a} a.ui.label:hover,a.ui.labels .label:hover{background-color:#505667;color:#dbdbdb} -.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(2),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4){background-color:#2a2e3a} -.repository .diff-file-box .code-diff-split tbody tr td.add-code,.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(3),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(4){background-color:#283e2d!important;border-color:#314a37!important} -.repository .diff-file-box .code-diff-split tbody tr td.del-code,.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(2){background-color:#3c2626!important;border-color:#634343!important} +.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(2),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(5),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(6){background-color:#2a2e3a} +.repository .diff-file-box .code-diff-split tbody tr td.add-code,.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(4),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(5),.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(6){background-color:#283e2d!important;border-color:#314a37!important} +.repository .diff-file-box .code-diff-split tbody tr td.del-code,.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(1),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(2),.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3){background-color:#3c2626!important;border-color:#634343!important} .ui.blue.button:focus,.ui.blue.buttons .button:focus{background-color:#a27558} .ui.blue.button:active,.ui.blue.buttons .button:active{background-color:#a27558} #git-graph-container li a{color:#c79575} diff --git a/public/js/index.js b/public/js/index.js index 3b2527d986..53fcaa8ba1 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -1069,8 +1069,8 @@ function initPullRequestReview() { var ntr = tr.next(); if (!ntr.hasClass('add-comment')) { ntr = $('' - + (isSplit ? '' - : '') + + (isSplit ? '' + : '') + ''); tr.after(ntr); } diff --git a/public/less/_base.less b/public/less/_base.less index 13ae1ad665..213ae2d2e6 100644 --- a/public/less/_base.less +++ b/public/less/_base.less @@ -160,7 +160,8 @@ a { } pre, -code { +code, +.mono { font: 12px @monospaced-fonts, monospace; &.raw { diff --git a/public/less/_repository.less b/public/less/_repository.less index 8d38faf50c..681296b74d 100644 --- a/public/less/_repository.less +++ b/public/less/_repository.less @@ -1362,10 +1362,6 @@ border-top: 0; } - pre { - margin: 0; - } - .lines-num { border-color: #d4d4d5; border-right-width: 1px; @@ -1405,7 +1401,7 @@ background-color: #99ff99; } - .lines-num[data-line-num]::before { + [data-line-num]::before { content: attr(data-line-num); text-align: right; } @@ -1413,9 +1409,10 @@ .lines-type-marker { width: 10px; min-width: 10px; + user-select: none; } - .line-type-marker[data-type-marker]::before { + [data-type-marker]::before { content: attr(data-type-marker); text-align: right; display: inline-block; @@ -1448,7 +1445,7 @@ // light gray for empty lines before / after commit &.add-code td:nth-child(1), &.add-code td:nth-child(2), - &.del-code td:nth-child(3), + &.add-code td:nth-child(3), &.del-code td:nth-child(4), &.del-code td:nth-child(5), &.del-code td:nth-child(6) { diff --git a/public/less/themes/arc-green.less b/public/less/themes/arc-green.less index d2fe2982d7..463a979434 100644 --- a/public/less/themes/arc-green.less +++ b/public/less/themes/arc-green.less @@ -1115,12 +1115,15 @@ a.ui.labels .label:hover { .repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(1), .repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(2), .repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3), -.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4) { +.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4), +.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(5), +.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(6) { background-color: #2a2e3a; } -.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(3), .repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(4), +.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(5), +.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(6), .repository .diff-file-box .code-diff-split tbody tr td.add-code { background-color: #283e2d !important; border-color: #314a37 !important; @@ -1128,6 +1131,7 @@ a.ui.labels .label:hover { .repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(1), .repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(2), +.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3), .repository .diff-file-box .code-diff-split tbody tr td.del-code { background-color: #3c2626 !important; border-color: #634343 !important; diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 94ac094fa4..056b8aea20 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -120,30 +120,12 @@ {{range $j, $section := $file.Sections}} {{range $k, $line := $section.Lines}} - - - - -
{{if $line.LeftIdx}}{{end}}
- - - {{if and $.SignedUserID $line.CanComment $.PageIsPullFiles (not (eq .GetType 2))}} - + - {{end}} -
{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}
- - - - - -
{{if $line.RightIdx}}{{end}}
- - - {{if and $.SignedUserID $line.CanComment $.PageIsPullFiles (not (eq .GetType 3))}} - + - {{end}} -
{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}
- + + {{if $line.LeftIdx}}{{end}} + {{if and $.SignedUserID $line.CanComment $.PageIsPullFiles (not (eq .GetType 2))}}+{{end}}{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}} + + {{if $line.RightIdx}}{{end}} + {{if and $.SignedUserID $line.CanComment $.PageIsPullFiles (not (eq .GetType 3))}}+{{end}}{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}} {{if gt (len $line.Comments) 0}} @@ -230,17 +212,22 @@ document.addEventListener('DOMContentLoaded', function() { $('tr.add-code').each(function() { var prev = $(this).prev(); - if(prev.is('.del-code') && prev.children().eq(3).text().trim() === '') { - while(prev.prev().is('.del-code') && prev.prev().children().eq(3).text().trim() === '') { + if(prev.is('.del-code') && prev.children().eq(5).text().trim() === '') { + while(prev.prev().is('.del-code') && prev.prev().children().eq(5).text().trim() === '') { prev = prev.prev(); } - prev.children().eq(2).html($(this).children().eq(2).html()); + prev.children().eq(3).attr("data-line-num", $(this).children().eq(3).attr("data-line-num")); prev.children().eq(3).html($(this).children().eq(3).html()); + prev.children().eq(4).html($(this).children().eq(4).html()); + prev.children().eq(5).html($(this).children().eq(5).html()); prev.children().eq(0).addClass('del-code'); prev.children().eq(1).addClass('del-code'); - prev.children().eq(2).addClass('add-code'); + prev.children().eq(2).addClass('del-code'); prev.children().eq(3).addClass('add-code'); + prev.children().eq(4).addClass('add-code'); + prev.children().eq(5).addClass('add-code'); + $(this).remove(); } }); diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index 5706e4cdee..f33381a1a2 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -8,22 +8,11 @@ {{/* {{if gt $j 0}}{{end}} */}} {{else}} - - - - - - + + {{end}} - -
- - - {{if and $.root.SignedUserID $line.CanComment $.root.PageIsPullFiles}} - + - {{end}} -
{{$section.GetComputedInlineDiffFor $line}}
- + + {{if and $.root.SignedUserID $line.CanComment $.root.PageIsPullFiles}}+{{end}}{{$section.GetComputedInlineDiffFor $line}} {{if gt (len $line.Comments) 0}}