fix: avoid nil panic and refactor some trivial problems (#39251)

models/git/branch.go

@@ -208,23 +208,11 @@ func AddBranches(ctx context.Context, branches []*Branch) error {
 
 func GetDeletedBranchByID(ctx context.Context, repoID, branchID int64) (*Branch, error) {
 	var branch Branch
-	has, err := db.GetEngine(ctx).ID(branchID).Get(&branch)
+	has, err := db.GetEngine(ctx).ID(branchID).Where("repo_id=? AND is_deleted=?", repoID, true).Get(&branch)
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, ErrBranchNotExist{
-			RepoID: repoID,
-		}
-	}
-	if branch.RepoID != repoID {
-		return nil, ErrBranchNotExist{
-			RepoID: repoID,
-		}
-	}
-	if !branch.IsDeleted {
-		return nil, ErrBranchNotExist{
-			RepoID: repoID,
-		}
+		return nil, ErrBranchNotExist{RepoID: repoID}
 	}
 	return &branch, nil
 }

modules/git/gitcmd/pipe.go

@@ -9,14 +9,6 @@ import (
 )
 
 type PipeBufferReader interface {
-	// Read should be used in the same goroutine as command's Wait
-	// When Reader in one goroutine, command's Wait in another goroutine, then the command exits, the pipe will be closed:
-	// * If the Reader goroutine reads faster, it will read all remaining data and then get io.EOF
-	//   * But this io.EOF doesn't mean the Reader has gotten complete data, the data might still be corrupted
-	// * If the Reader goroutine reads slower, it will get os.ErrClosed because the os.Pipe is closed ahead when the command exits
-	//
-	// When using 2 goroutines, no clear solution to distinguish these two cases or make Reader knows whether the data is complete
-	// It should avoid using Reader in a different goroutine than the command if the Read error needs to be handled.
 	Read(p []byte) (n int, err error)
 	Bytes() []byte
 }
@@ -26,6 +18,15 @@ type PipeBufferWriter interface {
 	Bytes() []byte
 }
 
+// PipeReader should be used in the same goroutine as command's Wait
+// When Reader in one goroutine, command's Wait in another goroutine, then the command exits, the pipe will be closed:
+// * If the Reader goroutine reads faster, it will read all remaining data and then get io.EOF
+//   - But this io.EOF doesn't mean the Reader has gotten complete data, the data might still be corrupted
+//
+// * If the Reader goroutine reads slower, it will get os.ErrClosed because the os.Pipe is closed ahead when the command exits
+//
+// When using 2 goroutines, no clear solution to distinguish these two cases or make Reader knows whether the data is complete
+// It should avoid using Reader in a different goroutine than the command if the Read error needs to be handled.
 type PipeReader interface {
 	io.ReadCloser
 	internalOnly()

routers/web/admin/packages.go

@@ -5,7 +5,6 @@ package admin
 
 import (
 	"net/http"
-	"net/url"
 	"time"
 
 	"gitea.dev/models/db"
@@ -93,7 +92,7 @@ func DeletePackageVersion(ctx *context.Context) {
 	}
 
 	ctx.Flash.Success(ctx.Tr("packages.settings.delete.version.success"))
-	ctx.JSONRedirect(setting.AppSubURL + "/-/admin/packages?page=" + url.QueryEscape(ctx.FormString("page")) + "&q=" + url.QueryEscape(ctx.FormString("q")) + "&type=" + url.QueryEscape(ctx.FormString("type")))
+	ctx.JSONRedirect("")
 }
 
 func CleanupExpiredData(ctx *context.Context) {

routers/web/admin/repos.go

@@ -47,10 +47,6 @@ func DeleteRepo(ctx *context.Context) {
 		return
 	}
 
-	if ctx.Repo != nil && ctx.Repo.GitRepo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == repo.ID {
-		ctx.Repo.GitRepo.Close()
-	}
-
 	if err := repo_service.DeleteRepository(ctx, ctx.Doer, repo, true); err != nil {
 		ctx.ServerError("DeleteRepository", err)
 		return
@@ -58,7 +54,7 @@ func DeleteRepo(ctx *context.Context) {
 	log.Trace("Repository deleted: %s", repo.FullName())
 
 	ctx.Flash.Success(ctx.Tr("repo.settings.deletion_success"))
-	ctx.JSONRedirect(setting.AppSubURL + "/-/admin/repos?page=" + url.QueryEscape(ctx.FormString("page")) + "&sort=" + url.QueryEscape(ctx.FormString("sort")))
+	ctx.JSONRedirect("")
 }
 
 // UnadoptedRepos lists the unadopted repositories

routers/web/repo/branch.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
 	"strings"
 
 	git_model "gitea.dev/models/git"
@@ -87,48 +86,31 @@ func Branches(ctx *context.Context) {
 	ctx.HTML(http.StatusOK, tplBranch)
 }
 
-// DeleteBranchPost responses for delete merged branch
 func DeleteBranchPost(ctx *context.Context) {
-	defer jsonRedirectBranches(ctx)
 	branchName := ctx.FormString("name")
-
-	if err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil {
-		switch {
-		case git.IsErrBranchNotExist(err):
-			log.Debug("DeleteBranch: Can't delete non existing branch '%s'", branchName)
-			ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
-		case errors.Is(err, repo_service.ErrBranchIsDefault):
-			log.Debug("DeleteBranch: Can't delete default branch '%s'", branchName)
-			ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName))
-		case errors.Is(err, git_model.ErrBranchIsProtected):
-			log.Debug("DeleteBranch: Can't delete protected branch '%s'", branchName)
-			ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
-		default:
-			log.Error("DeleteBranch: %v", err)
-			ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
-		}
-
-		return
+	err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName)
+	switch {
+	case err == nil:
+		ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", branchName))
+		ctx.JSONRedirect("")
+	case git.IsErrBranchNotExist(err):
+		ctx.JSONError(ctx.Tr("repo.branch.deletion_failed", branchName))
+	case errors.Is(err, repo_service.ErrBranchIsDefault):
+		ctx.JSONError(ctx.Tr("repo.branch.default_deletion_failed", branchName))
+	case errors.Is(err, git_model.ErrBranchIsProtected):
+		ctx.JSONError(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
+	default:
+		log.Error("DeleteBranch: %v", err)
+		ctx.JSONError(ctx.Tr("repo.branch.deletion_failed", branchName))
 	}
-
-	ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", branchName))
 }
 
-// RestoreBranchPost responses for delete merged branch
 func RestoreBranchPost(ctx *context.Context) {
-	defer jsonRedirectBranches(ctx)
-
 	branchID := ctx.FormInt64("branch_id")
-	branchName := ctx.FormString("name")
 
 	deletedBranch, err := git_model.GetDeletedBranchByID(ctx, ctx.Repo.Repository.ID, branchID)
 	if err != nil {
-		log.Error("GetDeletedBranchByID: %v", err)
-		ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", branchName))
-		return
-	} else if deletedBranch == nil {
-		log.Debug("RestoreBranch: Can't restore branch[%d] '%s', as it does not exist", branchID, branchName)
-		ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", branchName))
+		ctx.JSONErrorAuto(err)
 		return
 	}
 
@@ -138,11 +120,11 @@ func RestoreBranchPost(ctx *context.Context) {
 	}); err != nil {
 		if strings.Contains(err.Error(), "already exists") {
 			log.Debug("RestoreBranch: Can't restore branch '%s', since one with same name already exist", deletedBranch.Name)
-			ctx.Flash.Error(ctx.Tr("repo.branch.already_exists", deletedBranch.Name))
+			ctx.JSONError(ctx.Tr("repo.branch.already_exists", deletedBranch.Name))
 			return
 		}
 		log.Error("RestoreBranch: CreateBranch: %v", err)
-		ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name))
+		ctx.JSONError(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name))
 		return
 	}
 
@@ -163,10 +145,7 @@ func RestoreBranchPost(ctx *context.Context) {
 	}
 
 	ctx.Flash.Success(ctx.Tr("repo.branch.restore_success", deletedBranch.Name))
-}
-
-func jsonRedirectBranches(ctx *context.Context) {
-	ctx.JSONRedirect(ctx.Repo.RepoLink + "/branches?page=" + url.QueryEscape(ctx.FormString("page")))
+	ctx.JSONRedirect("")
 }
 
 // CreateBranch creates new branch in repository

routers/web/user/notification.go

@@ -140,6 +140,7 @@ func prepareUserNotificationsData(ctx *context.Context) {
 
 	pager.RemoveParam(container.SetOf("div-only", "sequence-number"))
 	ctx.Data["Page"] = pager
+	ctx.Data["PageQueryParams"] = templates.QueryBuild(pager.GetParams(), "page", page)
 }
 
 func filterNotificationsByRepoAccess(ctx stdCtx.Context, doer *user_model.User, notifications activities_model.NotificationList) (activities_model.NotificationList, []int, error) {

templates/admin/packages/list.tmpl

@@ -51,15 +51,10 @@
 					</tr>
 				</thead>
 				<tbody>
-					{{range .PackageDescriptors}}
+					{{range $pd := .PackageDescriptors}}
 						<tr>
 							<td>{{.Version.ID}}</td>
-							<td>
-								<a href="{{.Owner.HomeLink}}">{{.Owner.Name}}</a>
-								{{if .Owner.Visibility.IsPrivate}}
-									<span class="tw-text-gold">{{svg "octicon-lock"}}</span>
-								{{end}}
-							</td>
+							<td>{{template "admin/shared/owner" dict "OwnerUser" $pd.Owner}}</td>
 							<td>{{.Package.Type.Name}}</td>
 							<td class="gt-ellipsis tw-max-w-48">{{.Package.Name}}</td>
 							<td class="gt-ellipsis tw-max-w-48"><a href="{{.VersionWebLink}}">{{.Version.Version}}</a></td>
@@ -73,8 +68,8 @@
 							<td>{{DateUtils.AbsoluteShort .Version.CreatedUnix}}</td>
 							<td>
 								<a class="tw-text-red show-modal" href data-modal="#admin-package-delete-modal"
-									data-modal-form.url="{{$.Link}}/delete?page={{$.Page.Paginator.Current}}&sort={{$.SortType}}&id={{.Version.ID}}"
-									data-modal-package-name="{{.Package.Name}}" data-modal-package-version="{{.Version.Version}}"
+									data-modal-form.url="{{$.Link}}/delete?id={{$pd.Version.ID}}"
+									data-modal-package-name="{{$pd.Package.Name}}" data-modal-package-version="{{$pd.Version.Version}}"
 								>{{svg "octicon-trash"}}</a>
 							</td>
 						</tr>

templates/admin/repo/list.tmpl

@@ -43,15 +43,10 @@
 					</tr>
 				</thead>
 				<tbody>
-					{{range .Repos}}
+					{{range $repo := .Repos}}
 						<tr>
 							<td>{{.ID}}</td>
-							<td>
-								<a class="tw-break-anywhere" href="{{.Owner.HomeLink}}">{{.Owner.Name}}</a>
-								{{if .Owner.Visibility.IsPrivate}}
-									<span class="tw-text-gold">{{svg "octicon-lock"}}</span>
-								{{end}}
-							</td>
+							<td>{{template "admin/shared/owner" dict "OwnerUser" $repo.Owner}}</td>
 							<td>
 								<a class="tw-break-anywhere" href="{{.Link}}">{{.Name}}</a>
 								{{if .IsArchived}}
@@ -59,7 +54,7 @@
 								{{end}}
 								{{if .IsPrivate}}
 									<span class="ui basic label">{{ctx.Locale.Tr "repo.desc.private"}}</span>
-								{{else}}
+								{{else if .Owner}}
 									{{if .Owner.Visibility.IsPrivate}}
 										<span class="ui basic label">{{ctx.Locale.Tr "repo.desc.internal"}}</span>
 									{{end}}
@@ -86,8 +81,8 @@
 							<td>{{DateUtils.AbsoluteShort .CreatedUnix}}</td>
 							<td>
 								<a class="tw-text-red show-modal" href data-modal="#admin-repo-delete-modal"
-									data-modal-form.url="{{$.Link}}/delete?page={{$.Page.Paginator.Current}}&sort={{$.SortType}}&id={{.ID}}"
-									data-modal-repo-name="{{.Name}}"
+									data-modal-form.url="{{$.Link}}/delete?id={{$repo.ID}}"
+									data-modal-repo-name="{{$repo.Name}}"
 								>{{svg "octicon-trash"}}</a>
 							</td>
 						</tr>

templates/admin/shared/owner.tmpl

@@ -0,0 +1,5 @@
+{{$ownerUser := $.OwnerUser}}
+{{if $ownerUser}}
+	<a class="tw-break-anywhere" href="{{$ownerUser.HomeLink}}">{{$ownerUser.Name}}</a>
+	{{if $ownerUser.Visibility.IsPrivate}}<span class="tw-text-gold">{{svg "octicon-lock"}}</span>{{end}}
+{{end}}

templates/org/settings/options.tmpl

@@ -1,6 +1,6 @@
 {{template "org/settings/layout_head" (dict "pageClass" "organization settings options")}}
 
-<div class="ui segments org-setting-content">
+<div class="org-setting-content">
 	<h4 class="ui top attached header">
 		{{ctx.Locale.Tr "org.settings.options"}}
 	</h4>
@@ -65,8 +65,8 @@
 			</div>
 		</form>
 	</div>
-</div>
 
 {{template "org/settings/options_dangerzone" .}}
+</div>
 
 {{template "org/settings/layout_footer" .}}

templates/repo/branch/list.tmpl

@@ -54,7 +54,7 @@
 									</div>
 								{{end}}
 								{{if and $.IsWriter $.Repository.CanContentChange (not .IsDeleted)}}
-									<button class="btn interact-bg tw-p-2 show-modal show-rename-branch-modal"
+									<button class="btn interact-bg tw-p-2 show-modal" data-global-click="showRenameBranchModal"
 										data-is-default-branch="true"
 										data-modal="#rename-branch-modal"
 										data-old-branch-name="{{$.DefaultBranchBranch.DBBranch.Name}}"
@@ -183,7 +183,7 @@
 									</div>
 								{{end}}
 								{{if and $.IsWriter $.Repository.CanContentChange (not .DBBranch.IsDeleted)}}
-									<button class="btn interact-bg tw-p-2 show-modal show-rename-branch-modal"
+									<button class="btn interact-bg tw-p-2 show-modal" data-global-click="showRenameBranchModal"
 										data-is-default-branch="false"
 										data-old-branch-name="{{.DBBranch.Name}}"
 										data-modal="#rename-branch-modal"
@@ -194,13 +194,13 @@
 								{{end}}
 								{{if and $.IsWriter $.Repository.CanContentChange (not .IsProtected)}}
 									{{if .DBBranch.IsDeleted}}
-										<button class="btn interact-bg tw-p-2 link-action restore-branch-button" data-url="{{$.Link}}/restore?branch_id={{.DBBranch.ID}}&name={{.DBBranch.Name}}&page={{$.Page.Paginator.Current}}" data-tooltip-content="{{ctx.Locale.Tr "repo.branch.restore" (.DBBranch.Name)}}">
+										<button class="btn interact-bg tw-p-2 link-action restore-branch-button" data-url="{{$.Link}}/restore?branch_id={{.DBBranch.ID}}&name={{.DBBranch.Name}}" data-tooltip-content="{{ctx.Locale.Tr "repo.branch.restore" (.DBBranch.Name)}}">
 											<span class="tw-text-blue">
 												{{svg "octicon-reply"}}
 											</span>
 										</button>
 									{{else}}
-										<button class="btn interact-bg tw-p-2 show-modal delete-branch-button tw-text-red" data-modal="#delete-branch-modal" data-modal-form.url="{{$.Link}}/delete?name={{.DBBranch.Name}}&page={{$.Page.Paginator.Current}}" data-tooltip-content="{{ctx.Locale.Tr "repo.branch.delete" (.DBBranch.Name)}}" data-modal-name="{{.DBBranch.Name}}">
+										<button class="btn interact-bg tw-p-2 show-modal delete-branch-button tw-text-red" data-modal="#delete-branch-modal" data-modal-form.url="{{$.Link}}/delete?name={{.DBBranch.Name}}" data-tooltip-content="{{ctx.Locale.Tr "repo.branch.delete" (.DBBranch.Name)}}" data-modal-name="{{.DBBranch.Name}}">
 											{{svg "octicon-trash"}}
 										</button>
 									{{end}}
@@ -256,11 +256,9 @@
 			<div class="field default-branch-warning">
 				<span class="tw-text-red">{{ctx.Locale.Tr "repo.branch.warning_rename_default_branch"}}</span>
 			</div>
+			<input name="from" type="hidden">
 			<div class="field">
-				<span class="text" data-rename-branch-to="{{ctx.Locale.Tr "repo.branch.rename_branch_to"}}"></span>
-			</div>
-			<input name="from" type="hidden" required>
-			<div class="required field">
+				<label class="text" data-rename-branch-to="{{ctx.Locale.Tr "repo.branch.rename_branch_to"}}"></label>
 				<input name="to" required>
 			</div>
 		</div>

templates/user/notification/notification_div.tmpl

@@ -53,7 +53,7 @@
 							{{DateUtils.TimeSince $one.UpdatedUnix}}
 						{{end}}
 					</div>
-					<form class="notifications-buttons form-fetch-action" action="{{AppSubUrl}}/notifications/status?type={{$.PageType}}&page={{$.Page.Paginator.Current}}&perPage={{$.Page.Paginator.PagingNum}}" method="post"
+					<form class="notifications-buttons form-fetch-action" action="{{AppSubUrl}}/notifications/status?{{$.PageQueryParams}}" method="post"
 								data-fetch-sync="$body #notification_div"
 					>
 						<input type="hidden" name="notification_id" value="{{$one.ID}}">

tests/integration/api_helper_for_declarative_test.go

@@ -212,12 +212,12 @@ func doAPICreateDeployKey(ctx APITestContext, keyname, keyFile string, readOnly
 	}
 }
 
-func doAPICreatePullRequest(ctx APITestContext, owner, repo, baseBranch, headBranch string) func(*testing.T) (api.PullRequest, error) {
+func doAPICreatePullRequest(ctx APITestContext, owner, repo, baseBranch, headOwnerBranch string) func(*testing.T) (api.PullRequest, error) {
 	return func(t *testing.T) (api.PullRequest, error) {
 		req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner, repo), &api.CreatePullRequestOption{
-			Head:  headBranch,
+			Head:  headOwnerBranch,
 			Base:  baseBranch,
-			Title: fmt.Sprintf("create a pr from %s to %s", headBranch, baseBranch),
+			Title: fmt.Sprintf("create a pr from %s to %s", headOwnerBranch, baseBranch),
 		}).AddTokenAuth(ctx.Token)
 
 		expected := http.StatusCreated

tests/integration/git_general_test.go

@@ -32,6 +32,7 @@ import (
 	"gitea.dev/modules/lfs"
 	"gitea.dev/modules/setting"
 	api "gitea.dev/modules/structs"
+	"gitea.dev/modules/test"
 	"gitea.dev/tests"
 
 	"github.com/kballard/go-shellquote"
@@ -543,15 +544,15 @@ func doProtectBranchExt(ctx APITestContext, ruleName string, opts doProtectBranc
 	}
 }
 
-func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) func(t *testing.T) {
+func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headOwnerBranch string) func(t *testing.T) {
 	return func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 		var pr api.PullRequest
 		var err error
 
-		// Create a test pullrequest
+		// Create a test pull request
 		t.Run("CreatePullRequest", func(t *testing.T) {
-			pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t)
+			pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headOwnerBranch)(t)
 			assert.NoError(t, err)
 		})
 
@@ -578,10 +579,14 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
 		t.Run("EnsurDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffContent))
 
 		// Then: Delete the head branch & make sure that doesn't break the PR page or change its diff
-		t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch))
-		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
-		t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffContent))
-
+		// FIXME: this test (from #10936) is not right, the "master" branch can't be deleted
+		_ = doBranchDelete
+		/*
+			_, headBranch, _ := strings.Cut(headOwnerBranch, ":")
+			t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch))
+			t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
+			t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffContent))
+		*/
 		// Delete the head repository & make sure that doesn't break the PR page or change its diff
 		t.Run("DeleteHeadRepository", doAPIDeleteRepository(ctx))
 		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
@@ -621,11 +626,14 @@ func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBr
 func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest) func(t *testing.T) {
 	return func(t *testing.T) {
 		req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
-		ctx.Session.MakeRequest(t, req, http.StatusOK)
+		resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
+		assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
 		req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
-		ctx.Session.MakeRequest(t, req, http.StatusOK)
+		resp = ctx.Session.MakeRequest(t, req, http.StatusOK)
+		assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
 		req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
-		ctx.Session.MakeRequest(t, req, http.StatusOK)
+		resp = ctx.Session.MakeRequest(t, req, http.StatusOK)
+		assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
 	}
 }
 

web_src/js/features/repo-branch.ts

@@ -1,6 +1,7 @@
 import {toggleElem} from '../utils/dom.ts';
 import {showFomanticModal} from '../modules/fomantic/modal.ts';
 import {trString} from '../modules/i18n.ts';
+import {registerGlobalEventFunc} from '../modules/observer.ts';
 
 export function initRepoBranchButton() {
   initRepoCreateBranchButton();
@@ -25,19 +26,17 @@ function initRepoCreateBranchButton() {
 }
 
 function initRepoRenameBranchButton() {
-  for (const el of document.querySelectorAll('.show-rename-branch-modal')) {
-    el.addEventListener('click', () => {
-      const target = el.getAttribute('data-modal')!;
-      const modal = document.querySelector(target)!;
-      const oldBranchName = el.getAttribute('data-old-branch-name')!;
-      modal.querySelector<HTMLInputElement>('input[name=from]')!.value = oldBranchName;
+  registerGlobalEventFunc('click', 'showRenameBranchModal', (el) => {
+    const target = el.getAttribute('data-modal')!;
+    const modal = document.querySelector(target)!;
+    const oldBranchName = el.getAttribute('data-old-branch-name')!;
+    modal.querySelector<HTMLInputElement>('input[name=from]')!.value = oldBranchName;
 
-      // display the warning that the branch which is chosen is the default branch
-      const warn = modal.querySelector('.default-branch-warning')!;
-      toggleElem(warn, el.getAttribute('data-is-default-branch') === 'true');
+    // display the warning that the branch which is chosen is the default branch
+    const warn = modal.querySelector('.default-branch-warning')!;
+    toggleElem(warn, el.getAttribute('data-is-default-branch') === 'true');
 
-      const text = modal.querySelector('[data-rename-branch-to]')!;
-      text.textContent = trString(text.getAttribute('data-rename-branch-to')!, oldBranchName);
-    });
-  }
+    const text = modal.querySelector('[data-rename-branch-to]')!;
+    text.textContent = trString(text.getAttribute('data-rename-branch-to')!, oldBranchName);
+  });
 }

web_src/js/modules/fomantic/modal.ts

@@ -67,8 +67,8 @@ function onModalApproveDefault(this: HTMLElement) {
   const $modal = $(this);
   const selectors = $modal.modal('setting', 'selector');
   const elModal = $modal[0];
-  const elApprove = elModal.querySelector(selectors.approve);
-  const elForm = elApprove?.closest('form');
+  const elApprove = elModal.querySelector<HTMLElement>(selectors.approve);
+  const elForm = elApprove?.closest<HTMLFormElement>('form');
   if (!elForm) return true; // no form, just allow closing the modal
 
   // "form-fetch-action" can handle network errors gracefully,
@@ -78,6 +78,7 @@ function onModalApproveDefault(this: HTMLElement) {
   // There is an abuse for the "modal" + "form" combination, the "Approve" button is a traditional form submit button in the form.
   // Then "approve" and "submit" occur at the same time, the modal will be closed immediately before the form is submitted.
   // So here we prevent the modal from closing automatically by returning false, add the "is-loading" class to the form element.
+  if (!elForm.reportValidity()) return false;
   elForm.classList.add('is-loading');
   return false;
 }