From 29aea3642f5de5f1a8d4264f8360ddb5d072a861 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 9 Feb 2023 17:29:13 +0800 Subject: [PATCH 1/8] Make clone URL use current page's host (#22808) Follow #21986 Even if the ROOT_URL is incorrect, the clone URL on the UI should be correct. --------- Co-authored-by: Lunny Xiao --- templates/repo/clone_script.tmpl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/templates/repo/clone_script.tmpl b/templates/repo/clone_script.tmpl index afd90040fb4e7..88a67d82353b8 100644 --- a/templates/repo/clone_script.tmpl +++ b/templates/repo/clone_script.tmpl @@ -17,7 +17,14 @@ const btn = isSSH ? sshBtn : httpsBtn; if (!btn) return; - const link = btn.getAttribute('data-link'); + let link = btn.getAttribute('data-link'); + if (link.startsWith('http://') || link.startsWith('https://')) { + // use current protocol/host as the clone link + const url = new URL(link); + url.protocol = window.location.protocol; + url.host = window.location.host; + link = url.toString(); + } for (const el of document.getElementsByClassName('js-clone-url')) { el[el.nodeName === 'INPUT' ? 'value' : 'textContent'] = link; } From e253888a0e68d03da0616930427d7162eefa6aaf Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 9 Feb 2023 20:51:36 +0800 Subject: [PATCH 2/8] Fix isAllowed of escapeStreamer (#22814) The use of `sort.Search` is wrong: The slice should be sorted, and `return >= 0` doen't mean it exists, see the [manual](https://pkg.go.dev/sort#Search). Could be fixed like this if we really need it: ```diff diff --git a/modules/charset/escape_stream.go b/modules/charset/escape_stream.go index 823b63513..fcf1ffbc1 100644 --- a/modules/charset/escape_stream.go +++ b/modules/charset/escape_stream.go @@ -20,6 +20,9 @@ import ( var defaultWordRegexp = regexp.MustCompile(`(-?\d*\.\d\w*)|([^\` + "`" + `\~\!\@\#\$\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s\x00-\x1f]+)`) func NewEscapeStreamer(locale translation.Locale, next HTMLStreamer, allowed ...rune) HTMLStreamer { + sort.Slice(allowed, func(i, j int) bool { + return allowed[i] < allowed[j] + }) return &escapeStreamer{ escaped: &EscapeStatus{}, PassthroughHTMLStreamer: *NewPassthroughStreamer(next), @@ -284,14 +287,8 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables } func (e *escapeStreamer) isAllowed(r rune) bool { - if len(e.allowed) == 0 { - return false - } - if len(e.allowed) == 1 { - return e.allowed[0] == r - } - - return sort.Search(len(e.allowed), func(i int) bool { + i := sort.Search(len(e.allowed), func(i int) bool { return e.allowed[i] >= r - }) >= 0 + }) + return i < len(e.allowed) && e.allowed[i] == r } ``` But I don't think so, a map is better to do it. --- modules/charset/escape_stream.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/modules/charset/escape_stream.go b/modules/charset/escape_stream.go index 823b635137ebf..1b956bf4ab1a1 100644 --- a/modules/charset/escape_stream.go +++ b/modules/charset/escape_stream.go @@ -6,7 +6,6 @@ package charset import ( "fmt" "regexp" - "sort" "strings" "unicode" "unicode/utf8" @@ -20,12 +19,16 @@ import ( var defaultWordRegexp = regexp.MustCompile(`(-?\d*\.\d\w*)|([^\` + "`" + `\~\!\@\#\$\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s\x00-\x1f]+)`) func NewEscapeStreamer(locale translation.Locale, next HTMLStreamer, allowed ...rune) HTMLStreamer { + allowedM := make(map[rune]bool, len(allowed)) + for _, v := range allowed { + allowedM[v] = true + } return &escapeStreamer{ escaped: &EscapeStatus{}, PassthroughHTMLStreamer: *NewPassthroughStreamer(next), locale: locale, ambiguousTables: AmbiguousTablesForLocale(locale), - allowed: allowed, + allowed: allowedM, } } @@ -34,7 +37,7 @@ type escapeStreamer struct { escaped *EscapeStatus locale translation.Locale ambiguousTables []*AmbiguousTable - allowed []rune + allowed map[rune]bool } func (e *escapeStreamer) EscapeStatus() *EscapeStatus { @@ -256,7 +259,7 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables runeCounts.numBrokenRunes++ case r == ' ' || r == '\t' || r == '\n': runeCounts.numBasicRunes++ - case e.isAllowed(r): + case e.allowed[r]: if r > 0x7e || r < 0x20 { types[i] = nonBasicASCIIRuneType runeCounts.numNonConfusingNonBasicRunes++ @@ -282,16 +285,3 @@ func (e *escapeStreamer) runeTypes(runes ...rune) (types []runeType, confusables } return types, confusables, runeCounts } - -func (e *escapeStreamer) isAllowed(r rune) bool { - if len(e.allowed) == 0 { - return false - } - if len(e.allowed) == 1 { - return e.allowed[0] == r - } - - return sort.Search(len(e.allowed), func(i int) bool { - return e.allowed[i] >= r - }) >= 0 -} From cef8f50286ce1bc04c2e5989998d5a0774870de7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Feb 2023 00:14:45 +0800 Subject: [PATCH 3/8] Improve AppUrl/ROOT_URL checking (#22836) After some PRs: * #21986 * #22795 * #22808 * #22831 * #22839 Users won't be affected by the ROOT_URL problem in most cases. Close #19345 This PR improves AppUrl/ROOT_URL checking, only check it on the admin page, and the message is also updated. Feel free to suggest about more English-native messages. ![image](https://user-images.githubusercontent.com/2114189/217811809-7d44ddb7-2c4a-46d0-a5db-8ae6ee65f8c3.png) --- web_src/js/features/admin/common.js | 7 ++++++- web_src/js/features/common-global.js | 7 ++----- web_src/js/index.js | 2 -- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/web_src/js/features/admin/common.js b/web_src/js/features/admin/common.js index 2438fcf62b465..d0b3e461d54b6 100644 --- a/web_src/js/features/admin/common.js +++ b/web_src/js/features/admin/common.js @@ -1,12 +1,17 @@ import $ from 'jquery'; +import {checkAppUrl} from '../common-global.js'; const {csrfToken} = window.config; export function initAdminCommon() { - if ($('.admin').length === 0) { + if ($('.page-content.admin').length === 0) { return; } + // check whether appUrl(ROOT_URL) is correct, if not, show an error message + // only admin pages need this check because most templates are using relative URLs now + checkAppUrl(); + // New user if ($('.admin.new.user').length > 0 || $('.admin.edit.user').length > 0) { $('#login_type').on('change', function () { diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 4677eeac0c850..e655feec0bae6 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -381,9 +381,6 @@ export function checkAppUrl() { if (curUrl.startsWith(appUrl) || `${curUrl}/` === appUrl) { return; } - if (document.querySelector('.page-content.install')) { - return; // no need to show the message on the installation page - } - showGlobalErrorMessage(`Your ROOT_URL in app.ini is ${appUrl} but you are visiting ${curUrl} -You should set ROOT_URL correctly, otherwise the web may not work correctly.`); + showGlobalErrorMessage(`Your ROOT_URL in app.ini is "${appUrl}", it's unlikely matching the site you are visiting. +Mismatched ROOT_URL config causes wrong URL links for web UI/mail content/webhook notification.`); } diff --git a/web_src/js/index.js b/web_src/js/index.js index 74d80776b5ad0..611c09d2b8c16 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -48,7 +48,6 @@ import { initCommitStatuses, } from './features/repo-commit.js'; import { - checkAppUrl, initFootLanguageMenu, initGlobalButtonClickOnEnter, initGlobalButtons, @@ -199,5 +198,4 @@ $(document).ready(() => { initUserAuthWebAuthnRegister(); initUserSettings(); initViewedCheckboxListenerFor(); - checkAppUrl(); }); From 0c190e396df7025683b56f0e6f64356c907a8512 Mon Sep 17 00:00:00 2001 From: John Olheiser Date: Thu, 9 Feb 2023 10:15:07 -0600 Subject: [PATCH 4/8] Fix unmatched div in project filter (#22832) (Note that the below screenshots aren't the same repo, the former is try and the latter is local) Before ![div-before](https://user-images.githubusercontent.com/42128690/217723899-a15da77f-a196-4b23-a157-e7f1e1979610.png) After ![div-after](https://user-images.githubusercontent.com/42128690/217723878-e54235bc-a7d7-425e-bd0d-47d1814f18ba.png) Signed-off-by: jolheiser Co-authored-by: techknowlogick --- templates/repo/issue/list.tmpl | 1 - 1 file changed, 1 deletion(-) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index cf2a2a6bbabaa..4b7de641f1d2d 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -276,7 +276,6 @@ {{.Title}} {{end}} - {{end}} From 24a9caa2f33db2664510e79f184892c8cb266b62 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Feb 2023 00:31:30 +0800 Subject: [PATCH 5/8] 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 `` 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 --- models/issues/issue_xref.go | 14 +- modules/templates/helper.go | 4 + templates/projects/view.tmpl | 2 +- templates/repo/actions/runs_list.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 4 +- templates/repo/issue/view_content/pull.tmpl | 139 +++++++++--------- .../view_content/pull_merge_instruction.tmpl | 1 + 7 files changed, 85 insertions(+), 81 deletions(-) diff --git a/models/issues/issue_xref.go b/models/issues/issue_xref.go index 21ee24210f838..a1086f9e8120a 100644 --- a/models/issues/issue_xref.go +++ b/models/issues/issue_xref.go @@ -277,26 +277,26 @@ func CommentTypeIsRef(t CommentType) bool { return t == CommentTypeCommentRef || t == CommentTypePullRef || t == CommentTypeIssueRef } -// RefCommentHTMLURL returns the HTML URL for the comment that created this reference -func (c *Comment) RefCommentHTMLURL() string { +// RefCommentLink returns the relative URL for the comment that created this reference +func (c *Comment) RefCommentLink() string { // Edge case for when the reference is inside the title or the description of the referring issue if c.RefCommentID == 0 { - return c.RefIssueHTMLURL() + return c.RefIssueLink() } if err := c.LoadRefComment(); err != nil { // Silently dropping errors :unamused: log.Error("LoadRefComment(%d): %v", c.RefCommentID, err) return "" } - return c.RefComment.HTMLURL() + return c.RefComment.Link() } -// RefIssueHTMLURL returns the HTML URL of the issue where this reference was created -func (c *Comment) RefIssueHTMLURL() string { +// RefIssueLink returns the relative URL of the issue where this reference was created +func (c *Comment) RefIssueLink() string { if err := c.LoadRefIssue(); err != nil { // Silently dropping errors :unamused: log.Error("LoadRefIssue(%d): %v", c.RefCommentID, err) return "" } - return c.RefIssue.HTMLURL() + return c.RefIssue.Link() } // RefIssueTitle returns the title of the issue where this reference was created diff --git a/modules/templates/helper.go b/modules/templates/helper.go index a390d9459298d..7afc3aa59bc02 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -72,6 +72,10 @@ func NewFuncMap() []template.FuncMap { return setting.StaticURLPrefix + "/assets" }, "AppUrl": func() string { + // The usage of AppUrl should be avoided as much as possible, + // because the AppURL(ROOT_URL) may not match user's visiting site and the ROOT_URL in app.ini may be incorrect. + // And it's difficult for Gitea to guess absolute URL correctly with zero configuration, + // because Gitea doesn't know whether the scheme is HTTP or HTTPS unless the reverse proxy could tell Gitea. return setting.AppURL }, "AppVer": func() string { diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 42eb578074d83..72131e30bb79e 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -238,7 +238,7 @@ {{end}}
{{range .Assignees}} - {{avatar . 28 "mini mr-3"}} + {{avatar . 28 "mini mr-3"}} {{end}}
diff --git a/templates/repo/actions/runs_list.tmpl b/templates/repo/actions/runs_list.tmpl index d3eb12430f282..2a8522294318d 100644 --- a/templates/repo/actions/runs_list.tmpl +++ b/templates/repo/actions/runs_list.tmpl @@ -6,7 +6,7 @@
- + {{.Title}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 39fbc638ce12c..37aa82345fa58 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -151,12 +151,12 @@ {{if eq .RefAction 3}}{{end}} {{template "shared/user/authorlink" .Poster}} - {{$.locale.Tr $refTr (.EventTag|Escape) $createdStr (.RefCommentHTMLURL|Escape) $refFrom | Safe}} + {{$.locale.Tr $refTr (.EventTag|Escape) $createdStr (.RefCommentLink|Escape) $refFrom | Safe}} {{if eq .RefAction 3}}{{end}}
- {{.RefIssueTitle}} {{.RefIssueIdent}} + {{.RefIssueTitle}} {{.RefIssueIdent}}
{{else if eq .Type 4}} diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index c68a150832fe6..f32c278b62f6c 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -303,79 +303,78 @@ {{$hasPendingPullRequestMergeTip = $.locale.Tr "repo.pulls.auto_merge_has_pending_schedule" .PendingPullRequestMerge.Doer.Name $createdPRMergeStr}} {{end}}
-
diff --git a/templates/repo/issue/view_content/pull_merge_instruction.tmpl b/templates/repo/issue/view_content/pull_merge_instruction.tmpl index 816f25cbcf901..21bb3d8e79b90 100644 --- a/templates/repo/issue/view_content/pull_merge_instruction.tmpl +++ b/templates/repo/issue/view_content/pull_merge_instruction.tmpl @@ -5,6 +5,7 @@
{{if eq $.Issue.PullRequest.Flow 0}}
git checkout -b {{if ne $.Issue.PullRequest.HeadRepo.ID $.Issue.PullRequest.BaseRepo.ID}}{{$.Issue.PullRequest.HeadRepo.OwnerName}}-{{end}}{{$.Issue.PullRequest.HeadBranch}} {{$.Issue.PullRequest.BaseBranch}}
+ {{/* 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*/}}
git pull {{if ne $.Issue.PullRequest.HeadRepo.ID $.Issue.PullRequest.BaseRepo.ID}}{{$.Issue.PullRequest.HeadRepo.HTMLURL}}{{else}}origin{{end}} {{$.Issue.PullRequest.HeadBranch}}
{{else}}
git fetch origin {{$.Issue.PullRequest.GetGitRefName}}:{{$.Issue.PullRequest.HeadBranch}}
From 137fcc989bbde6dcddbbf53650e174c119d38068 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Thu, 9 Feb 2023 17:39:31 +0100 Subject: [PATCH 6/8] Fix inconsistent Filter Project name in issue list (#22827) Use Project instead of Filter Project like the other filter menus. --- templates/repo/issue/list.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 4b7de641f1d2d..c04b074221962 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -77,13 +77,13 @@
- {{.locale.Tr "repo.issues.filter_projects"}} + {{.locale.Tr "repo.issues.filter_project"}} {{svg "octicon-triangle-down" 14 "dropdown icon"}}
{{svg "octicon-search" 16}} - +
{{.locale.Tr "repo.issues.filter_project_all"}} {{.locale.Tr "repo.issues.filter_project_none"}} From bdd2c9d2eff76fed2cc200377723f519e8af64d0 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Thu, 9 Feb 2023 18:08:42 +0100 Subject: [PATCH 7/8] Fix update by rebase being wrongly disabled by protected base branch (#22825) The branch this is force pushing to is the head branch in the head repo, so it should be checking if that is protected, not the base. --- services/pull/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/update.go b/services/pull/update.go index ede89bcdfff3e..b9525cf0c980f 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -106,7 +106,7 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, BaseBranch: pull.HeadBranch, } - pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.BaseRepoID, pull.BaseBranch) + pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) if err != nil { return false, false, err } From affdd40296960a08a4223330ccbd1fb88c96ea1a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Feb 2023 01:11:16 +0800 Subject: [PATCH 8/8] Make issue title edit buttons focusable and fix incorrect ajax requests (#22807) Replace #19922 , which is stale since my last review: https://github.com/go-gitea/gitea/pull/19922#pullrequestreview-1003546506 and https://github.com/go-gitea/gitea/pull/19922#issuecomment-1153181546 Close #19769 Changes: 1. Use `
{{end}}

{{RenderIssueTitle $.Context .Issue.Title $.RepoLink $.Repository.ComposeMetas | RenderCodeBlock}} #{{.Issue.Index}} -

{{if and (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .Repository.IsArchived)}}
- - + +
{{end}}
diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index e655feec0bae6..f2edf31249ce6 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -60,6 +60,7 @@ export function initGlobalButtonClickOnEnter() { $(document).on('keypress', '.ui.button', (e) => { if (e.keyCode === 13 || e.keyCode === 32) { // enter key or space bar $(e.target).trigger('click'); + e.preventDefault(); } }); } diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 56d294e81afc2..f562584c1143f 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -605,6 +605,7 @@ export function initRepoIssueTitleEdit() { const targetBranch = $('#pull-target-branch').data('branch'); const $branchTarget = $('#branch_target'); if (targetBranch === $branchTarget.text()) { + window.location.reload(); return false; } $.post(update_url, { @@ -617,19 +618,22 @@ export function initRepoIssueTitleEdit() { }); }; - const pullrequest_target_update_url = $(this).data('target-update-url'); + const pullrequest_target_update_url = $(this).attr('data-target-update-url'); if ($editInput.val().length === 0 || $editInput.val() === $issueTitle.text()) { $editInput.val($issueTitle.text()); pullrequest_targetbranch_change(pullrequest_target_update_url); } else { - $.post($(this).data('update-url'), { + $.post($(this).attr('data-update-url'), { _csrf: csrfToken, title: $editInput.val() }, (data) => { $editInput.val(data.title); $issueTitle.text(data.title); - pullrequest_targetbranch_change(pullrequest_target_update_url); - window.location.reload(); + if (pullrequest_target_update_url) { + pullrequest_targetbranch_change(pullrequest_target_update_url); // it will reload the window + } else { + window.location.reload(); + } }); } return false; diff --git a/web_src/less/_base.less b/web_src/less/_base.less index 6f35a49ad8209..4a22b8af4b552 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -2316,6 +2316,13 @@ a.ui.label:hover { .ui.basic.secondary.buttons .button:active, .ui.basic.secondary.button:active { color: var(--color-secondary-dark-8) !important; + border-color: var(--color-secondary-dark-1) !important; +} + +.ui.basic.secondary.button:focus, +.ui.basic.secondary.buttons .button:focus { + color: var(--color-secondary-dark-8) !important; + border-color: var(--color-secondary-dark-3) !important; } .ui.tertiary.button {