Skip to content

Commit 24a9caa

Browse files
authored
Fix more HTMLURL in templates (#22831)
I haven't tested `runs_list.tmpl` but I think it could be right. After this PR, besides the `<meta .. HTMLURL>` in html head, the only explicit HTMLURL usage is in `pull_merge_instruction.tmpl`, which doesn't affect users too much and it's difficult to fix at the moment. There are still many usages of `AppUrl` in the templates (eg: the package help manual), they are similar problems as the HTMLURL in pull_merge_instruction, and they might be fixed together in the future. Diff without space: https://github.com/go-gitea/gitea/pull/22831/files?diff=unified&w=1
1 parent 0c190e3 commit 24a9caa

File tree

7 files changed

+85
-81
lines changed

7 files changed

+85
-81
lines changed

Diff for: models/issues/issue_xref.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -277,26 +277,26 @@ func CommentTypeIsRef(t CommentType) bool {
277277
return t == CommentTypeCommentRef || t == CommentTypePullRef || t == CommentTypeIssueRef
278278
}
279279

280-
// RefCommentHTMLURL returns the HTML URL for the comment that created this reference
281-
func (c *Comment) RefCommentHTMLURL() string {
280+
// RefCommentLink returns the relative URL for the comment that created this reference
281+
func (c *Comment) RefCommentLink() string {
282282
// Edge case for when the reference is inside the title or the description of the referring issue
283283
if c.RefCommentID == 0 {
284-
return c.RefIssueHTMLURL()
284+
return c.RefIssueLink()
285285
}
286286
if err := c.LoadRefComment(); err != nil { // Silently dropping errors :unamused:
287287
log.Error("LoadRefComment(%d): %v", c.RefCommentID, err)
288288
return ""
289289
}
290-
return c.RefComment.HTMLURL()
290+
return c.RefComment.Link()
291291
}
292292

293-
// RefIssueHTMLURL returns the HTML URL of the issue where this reference was created
294-
func (c *Comment) RefIssueHTMLURL() string {
293+
// RefIssueLink returns the relative URL of the issue where this reference was created
294+
func (c *Comment) RefIssueLink() string {
295295
if err := c.LoadRefIssue(); err != nil { // Silently dropping errors :unamused:
296296
log.Error("LoadRefIssue(%d): %v", c.RefCommentID, err)
297297
return ""
298298
}
299-
return c.RefIssue.HTMLURL()
299+
return c.RefIssue.Link()
300300
}
301301

302302
// RefIssueTitle returns the title of the issue where this reference was created

Diff for: modules/templates/helper.go

+4
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ func NewFuncMap() []template.FuncMap {
7272
return setting.StaticURLPrefix + "/assets"
7373
},
7474
"AppUrl": func() string {
75+
// The usage of AppUrl should be avoided as much as possible,
76+
// because the AppURL(ROOT_URL) may not match user's visiting site and the ROOT_URL in app.ini may be incorrect.
77+
// And it's difficult for Gitea to guess absolute URL correctly with zero configuration,
78+
// because Gitea doesn't know whether the scheme is HTTP or HTTPS unless the reverse proxy could tell Gitea.
7579
return setting.AppURL
7680
},
7781
"AppVer": func() string {

Diff for: templates/projects/view.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@
238238
{{end}}
239239
<div class="right floated">
240240
{{range .Assignees}}
241-
<a class="tooltip" target="_blank" href="{{.HTMLURL}}" data-content="{{$.locale.Tr "repo.projects.board.assigned_to"}} {{.Name}}">{{avatar . 28 "mini mr-3"}}</a>
241+
<a class="tooltip" target="_blank" href="{{.HomeLink}}" data-content="{{$.locale.Tr "repo.projects.board.assigned_to"}} {{.Name}}">{{avatar . 28 "mini mr-3"}}</a>
242242
{{end}}
243243
</div>
244244
</div>

Diff for: templates/repo/actions/runs_list.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
</div>
77
<div class="issue-item-main f1 fc df">
88
<div class="issue-item-top-row">
9-
<a class="index ml-0 mr-2" href="{{if .HTMLURL}}{{.HTMLURL}}{{else}}{{$.Link}}/{{.Index}}{{end}}">
9+
<a class="index ml-0 mr-2" href="{{if .Link}}{{.Link}}{{else}}{{$.Link}}/{{.Index}}{{end}}">
1010
{{.Title}}
1111
</a>
1212
<span class="ui label">

Diff for: templates/repo/issue/view_content/comments.tmpl

+2-2
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,12 @@
151151
{{if eq .RefAction 3}}<del>{{end}}
152152
<span class="text grey muted-links">
153153
{{template "shared/user/authorlink" .Poster}}
154-
{{$.locale.Tr $refTr (.EventTag|Escape) $createdStr (.RefCommentHTMLURL|Escape) $refFrom | Safe}}
154+
{{$.locale.Tr $refTr (.EventTag|Escape) $createdStr (.RefCommentLink|Escape) $refFrom | Safe}}
155155
</span>
156156
{{if eq .RefAction 3}}</del>{{end}}
157157

158158
<div class="detail">
159-
<span class="text grey muted-links"><a href="{{.RefIssueHTMLURL}}"><b>{{.RefIssueTitle}}</b> {{.RefIssueIdent}}</a></span>
159+
<span class="text grey muted-links"><a href="{{.RefIssueLink}}"><b>{{.RefIssueTitle}}</b> {{.RefIssueIdent}}</a></span>
160160
</div>
161161
</div>
162162
{{else if eq .Type 4}}

Diff for: templates/repo/issue/view_content/pull.tmpl

+69-70
Original file line numberDiff line numberDiff line change
@@ -303,79 +303,78 @@
303303
{{$hasPendingPullRequestMergeTip = $.locale.Tr "repo.pulls.auto_merge_has_pending_schedule" .PendingPullRequestMerge.Doer.Name $createdPRMergeStr}}
304304
{{end}}
305305
<div class="ui divider"></div>
306-
<script>
307-
(() => {
308-
const defaultMergeTitle = {{.DefaultMergeMessage}};
309-
const defaultSquashMergeTitle = {{.DefaultSquashMergeMessage}};
310-
const defaultMergeMessage = {{if .DefaultMergeBody}}{{.DefaultMergeBody}}{{else}}'Reviewed-on: ' + {{$.Issue.HTMLURL}} + '\n' + {{$approvers}}{{end}};
311-
const defaultSquashMergeMessage = {{if .DefaultSquashMergeBody}}{{.DefaultSquashMergeBody}}{{else}}'Reviewed-on: ' + {{$.Issue.HTMLURL}} + '\n' + {{$approvers}}{{end}};
312-
const mergeForm = {
313-
'baseLink': {{.Link}},
314-
'textCancel': {{$.locale.Tr "cancel"}},
315-
'textDeleteBranch': {{$.locale.Tr "repo.branch.delete" .HeadTarget}},
316-
'textAutoMergeButtonWhenSucceed': {{$.locale.Tr "repo.pulls.auto_merge_button_when_succeed"}},
317-
'textAutoMergeWhenSucceed': {{$.locale.Tr "repo.pulls.auto_merge_when_succeed"}},
318-
'textAutoMergeCancelSchedule': {{$.locale.Tr "repo.pulls.auto_merge_cancel_schedule"}},
319-
'textClearMergeMessage': {{$.locale.Tr "repo.pulls.clear_merge_message"}},
320-
'textClearMergeMessageHint': {{$.locale.Tr "repo.pulls.clear_merge_message_hint"}},
306+
<script type="module">
307+
const issueUrl = window.location.origin + {{$.Issue.Link}};
308+
const defaultMergeTitle = {{.DefaultMergeMessage}};
309+
const defaultSquashMergeTitle = {{.DefaultSquashMergeMessage}};
310+
const defaultMergeMessage = {{if .DefaultMergeBody}}{{.DefaultMergeBody}}{{else}}`Reviewed-on: ${issueUrl}\n` + {{$approvers}}{{end}};
311+
const defaultSquashMergeMessage = {{if .DefaultSquashMergeBody}}{{.DefaultSquashMergeBody}}{{else}}`Reviewed-on: ${issueUrl}\n` + {{$approvers}}{{end}};
312+
const mergeForm = {
313+
'baseLink': {{.Link}},
314+
'textCancel': {{$.locale.Tr "cancel"}},
315+
'textDeleteBranch': {{$.locale.Tr "repo.branch.delete" .HeadTarget}},
316+
'textAutoMergeButtonWhenSucceed': {{$.locale.Tr "repo.pulls.auto_merge_button_when_succeed"}},
317+
'textAutoMergeWhenSucceed': {{$.locale.Tr "repo.pulls.auto_merge_when_succeed"}},
318+
'textAutoMergeCancelSchedule': {{$.locale.Tr "repo.pulls.auto_merge_cancel_schedule"}},
319+
'textClearMergeMessage': {{$.locale.Tr "repo.pulls.clear_merge_message"}},
320+
'textClearMergeMessageHint': {{$.locale.Tr "repo.pulls.clear_merge_message_hint"}},
321321

322-
'canMergeNow': {{$canMergeNow}},
323-
'allOverridableChecksOk': {{not $notAllOverridableChecksOk}},
324-
'emptyCommit': {{.Issue.PullRequest.IsEmpty}},
325-
'pullHeadCommitID': {{.PullHeadCommitID}},
326-
'isPullBranchDeletable': {{.IsPullBranchDeletable}},
327-
'defaultMergeStyle': {{.MergeStyle}},
328-
'defaultDeleteBranchAfterMerge': {{$prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}},
329-
'mergeMessageFieldPlaceHolder': {{$.locale.Tr "repo.editor.commit_message_desc"}},
330-
'defaultMergeMessage': defaultMergeMessage,
322+
'canMergeNow': {{$canMergeNow}},
323+
'allOverridableChecksOk': {{not $notAllOverridableChecksOk}},
324+
'emptyCommit': {{.Issue.PullRequest.IsEmpty}},
325+
'pullHeadCommitID': {{.PullHeadCommitID}},
326+
'isPullBranchDeletable': {{.IsPullBranchDeletable}},
327+
'defaultMergeStyle': {{.MergeStyle}},
328+
'defaultDeleteBranchAfterMerge': {{$prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}},
329+
'mergeMessageFieldPlaceHolder': {{$.locale.Tr "repo.editor.commit_message_desc"}},
330+
'defaultMergeMessage': defaultMergeMessage,
331331

332-
'hasPendingPullRequestMerge': {{.HasPendingPullRequestMerge}},
333-
'hasPendingPullRequestMergeTip': {{$hasPendingPullRequestMergeTip}},
334-
};
332+
'hasPendingPullRequestMerge': {{.HasPendingPullRequestMerge}},
333+
'hasPendingPullRequestMergeTip': {{$hasPendingPullRequestMergeTip}},
334+
};
335335

336-
const generalHideAutoMerge = mergeForm.canMergeNow && mergeForm.allOverridableChecksOk; // if this PR can be merged now, then hide the auto merge
337-
mergeForm['mergeStyles'] = [
338-
{
339-
'name': 'merge',
340-
'allowed': {{$prUnit.PullRequestsConfig.AllowMerge}},
341-
'textDoMerge': {{$.locale.Tr "repo.pulls.merge_pull_request"}},
342-
'mergeTitleFieldText': defaultMergeTitle,
343-
'mergeMessageFieldText': defaultMergeMessage,
344-
'hideAutoMerge': generalHideAutoMerge,
345-
},
346-
{
347-
'name': 'rebase',
348-
'allowed': {{$prUnit.PullRequestsConfig.AllowRebase}},
349-
'textDoMerge': {{$.locale.Tr "repo.pulls.rebase_merge_pull_request"}},
350-
'hideMergeMessageTexts': true,
351-
'hideAutoMerge': generalHideAutoMerge,
352-
},
353-
{
354-
'name': 'rebase-merge',
355-
'allowed': {{$prUnit.PullRequestsConfig.AllowRebaseMerge}},
356-
'textDoMerge': {{$.locale.Tr "repo.pulls.rebase_merge_commit_pull_request"}},
357-
'mergeTitleFieldText': defaultMergeTitle,
358-
'mergeMessageFieldText': defaultMergeMessage,
359-
'hideAutoMerge': generalHideAutoMerge,
360-
},
361-
{
362-
'name': 'squash',
363-
'allowed': {{$prUnit.PullRequestsConfig.AllowSquash}},
364-
'textDoMerge': {{$.locale.Tr "repo.pulls.squash_merge_pull_request"}},
365-
'mergeTitleFieldText': defaultSquashMergeTitle,
366-
'mergeMessageFieldText': {{.GetCommitMessages}} + defaultSquashMergeMessage,
367-
'hideAutoMerge': generalHideAutoMerge,
368-
},
369-
{
370-
'name': 'manually-merged',
371-
'allowed': {{and $prUnit.PullRequestsConfig.AllowManualMerge $.IsRepoAdmin}},
372-
'textDoMerge': {{$.locale.Tr "repo.pulls.merge_manually"}},
373-
'hideMergeMessageTexts': true,
374-
'hideAutoMerge': true,
375-
}
376-
];
377-
window.config.pageData.pullRequestMergeForm = mergeForm;
378-
})();
336+
const generalHideAutoMerge = mergeForm.canMergeNow && mergeForm.allOverridableChecksOk; // if this PR can be merged now, then hide the auto merge
337+
mergeForm['mergeStyles'] = [
338+
{
339+
'name': 'merge',
340+
'allowed': {{$prUnit.PullRequestsConfig.AllowMerge}},
341+
'textDoMerge': {{$.locale.Tr "repo.pulls.merge_pull_request"}},
342+
'mergeTitleFieldText': defaultMergeTitle,
343+
'mergeMessageFieldText': defaultMergeMessage,
344+
'hideAutoMerge': generalHideAutoMerge,
345+
},
346+
{
347+
'name': 'rebase',
348+
'allowed': {{$prUnit.PullRequestsConfig.AllowRebase}},
349+
'textDoMerge': {{$.locale.Tr "repo.pulls.rebase_merge_pull_request"}},
350+
'hideMergeMessageTexts': true,
351+
'hideAutoMerge': generalHideAutoMerge,
352+
},
353+
{
354+
'name': 'rebase-merge',
355+
'allowed': {{$prUnit.PullRequestsConfig.AllowRebaseMerge}},
356+
'textDoMerge': {{$.locale.Tr "repo.pulls.rebase_merge_commit_pull_request"}},
357+
'mergeTitleFieldText': defaultMergeTitle,
358+
'mergeMessageFieldText': defaultMergeMessage,
359+
'hideAutoMerge': generalHideAutoMerge,
360+
},
361+
{
362+
'name': 'squash',
363+
'allowed': {{$prUnit.PullRequestsConfig.AllowSquash}},
364+
'textDoMerge': {{$.locale.Tr "repo.pulls.squash_merge_pull_request"}},
365+
'mergeTitleFieldText': defaultSquashMergeTitle,
366+
'mergeMessageFieldText': {{.GetCommitMessages}} + defaultSquashMergeMessage,
367+
'hideAutoMerge': generalHideAutoMerge,
368+
},
369+
{
370+
'name': 'manually-merged',
371+
'allowed': {{and $prUnit.PullRequestsConfig.AllowManualMerge $.IsRepoAdmin}},
372+
'textDoMerge': {{$.locale.Tr "repo.pulls.merge_manually"}},
373+
'hideMergeMessageTexts': true,
374+
'hideAutoMerge': true,
375+
}
376+
];
377+
window.config.pageData.pullRequestMergeForm = mergeForm;
379378
</script>
380379

381380
<div id="pull-request-merge-form"></div>

Diff for: templates/repo/issue/view_content/pull_merge_instruction.tmpl

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
<div class="ui secondary segment">
66
{{if eq $.Issue.PullRequest.Flow 0}}
77
<div>git checkout -b {{if ne $.Issue.PullRequest.HeadRepo.ID $.Issue.PullRequest.BaseRepo.ID}}{{$.Issue.PullRequest.HeadRepo.OwnerName}}-{{end}}{{$.Issue.PullRequest.HeadBranch}} {{$.Issue.PullRequest.BaseBranch}}</div>
8+
{{/* the only legacy HTMLURL used in template, which doesn't affect users too much and is very diffcult to fix, it should be fixed together with other AppUrl usages*/}}
89
<div>git pull {{if ne $.Issue.PullRequest.HeadRepo.ID $.Issue.PullRequest.BaseRepo.ID}}{{$.Issue.PullRequest.HeadRepo.HTMLURL}}{{else}}origin{{end}} {{$.Issue.PullRequest.HeadBranch}}</div>
910
{{else}}
1011
<div>git fetch origin {{$.Issue.PullRequest.GetGitRefName}}:{{$.Issue.PullRequest.HeadBranch}}</div>

0 commit comments

Comments
 (0)