Merge remote-tracking branch 'giteaofficial/main'

* giteaofficial/main:
  Disable issue/PR comment button given empty input (go-gitea#31463)
  Simplify 404/500 page (go-gitea#31409)
  Fix web notification icon not updated once you read all notifications (go-gitea#31447)
  Switch to "Write" tab when edit comment again (go-gitea#31445)
  Add simple JS init performance trace (go-gitea#31459)

models/activities/notification.go

@@ -286,13 +286,14 @@ type UserIDCount struct {
 	Count  int64
 }
 
-// GetUIDsAndNotificationCounts between the two provided times
+// GetUIDsAndNotificationCounts returns the unread counts for every user between the two provided times.
+// It must return all user IDs which appear during the period, including count=0 for users who have read all.
 func GetUIDsAndNotificationCounts(ctx context.Context, since, until timeutil.TimeStamp) ([]UserIDCount, error) {
-	sql := `SELECT user_id, count(*) AS count FROM notification ` +
+	sql := `SELECT user_id, sum(case when status= ? then 1 else 0 end) AS count FROM notification ` +
 		`WHERE user_id IN (SELECT user_id FROM notification WHERE updated_unix >= ? AND ` +
-		`updated_unix < ?) AND status = ? GROUP BY user_id`
+		`updated_unix < ?) GROUP BY user_id`
 	var res []UserIDCount
-	return res, db.GetEngine(ctx).SQL(sql, since, until, NotificationStatusUnread).Find(&res)
+	return res, db.GetEngine(ctx).SQL(sql, NotificationStatusUnread, since, until).Find(&res)
 }
 
 // SetIssueReadBy sets issue to be read by given user.

templates/repo/issue/view_content.tmpl

@@ -106,7 +106,7 @@
 												</button>
 											{{end}}
 										{{end}}
-										<button class="ui primary button">
+										<button id="comment-button" class="ui primary button">
 											{{ctx.Locale.Tr "repo.issues.create_comment"}}
 										</button>
 									</div>

templates/status/404.tmpl

@@ -1,10 +1,16 @@
 {{template "base/head" .}}
 <div role="main" aria-label="{{.Title}}" class="page-content {{if .IsRepo}}repository{{end}}">
 	{{if .IsRepo}}{{template "repo/header" .}}{{end}}
-	<div class="ui container tw-text-center">
-		<img class="tw-max-w-[80vw] tw-py-16" src="{{AssetUrlPrefix}}/img/404.png" alt="404">
-		<p>{{if .NotFoundPrompt}}{{.NotFoundPrompt}}{{else}}{{ctx.Locale.Tr "error404"}}{{end}}</p>
-		{{if .NotFoundGoBackURL}}<a class="ui button" href="{{.NotFoundGoBackURL}}">{{ctx.Locale.Tr "go_back"}}</a>{{end}}
+	<div class="ui container">
+		<div class="status-page-error">
+			<div class="status-page-error-title">404 Not Found</div>
+			<div class="tw-text-center">
+				<div class="tw-my-4">{{if .NotFoundPrompt}}{{.NotFoundPrompt}}{{else}}{{ctx.Locale.Tr "error404"}}{{end}}</div>
+				{{if .NotFoundGoBackURL}}
+					<a class="tw-block tw-my-4" href="{{.NotFoundGoBackURL}}">{{ctx.Locale.Tr "go_back"}}</a>
+				{{end}}
+			</div>
+		</div>
 	</div>
 </div>
 {{template "base/footer" .}}

templates/status/500.tmpl

@@ -33,17 +33,18 @@
 			<div class="ui container" >
 				<style> .ui.message.flash-message { text-align: left; } </style>
 				{{template "base/alert" .}}
-			</div>
-			<p class="tw-mt-8 center"><img src="{{AssetUrlPrefix}}/img/500.png" alt="Internal Server Error"></p>
-			<div class="divider"></div>
-			<div class="ui container tw-my-8">
-				{{if .ErrorMsg}}
-					<p>{{ctx.Locale.Tr "error.occurred"}}:</p>
-					<pre class="tw-whitespace-pre-wrap tw-break-all">{{.ErrorMsg}}</pre>
-				{{end}}
-				<div class="center tw-mt-8">
-					{{if or .SignedUser.IsAdmin .ShowFooterVersion}}<p>{{ctx.Locale.Tr "admin.config.app_ver"}}: {{AppVer}}</p>{{end}}
-					{{if .SignedUser.IsAdmin}}<p>{{ctx.Locale.Tr "error.report_message"}}</p>{{end}}
+				<div class="status-page-error">
+					<div class="status-page-error-title">500 Internal Server Error</div>
+					{{if .ErrorMsg}}
+					<div class="tw-mt-8">
+						<p>{{ctx.Locale.Tr "error.occurred"}}:</p>
+						<pre class="tw-whitespace-pre-wrap tw-break-all">{{.ErrorMsg}}</pre>
+					</div>
+					{{end}}
+					<div class="tw-mt-8 tw-text-center">
+						{{if or .SignedUser.IsAdmin .ShowFooterVersion}}<p>{{ctx.Locale.Tr "admin.config.app_ver"}}: {{AppVer}}</p>{{end}}
+						{{if .SignedUser.IsAdmin}}<p>{{ctx.Locale.Tr "error.report_message"}}</p>{{end}}
+					</div>
 				</div>
 			</div>
 		</div>

tests/integration/compare_test.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/test"
 	repo_service "code.gitea.io/gitea/services/repository"
 	"code.gitea.io/gitea/tests"
 
@@ -30,9 +31,9 @@ func TestCompareTag(t *testing.T) {
 	// A dropdown for both base and head.
 	assert.Lenf(t, selection.Nodes, 2, "The template has changed")
 
-	req = NewRequest(t, "GET", "/user2/repo1/compare/invalid")
+	req = NewRequest(t, "GET", "/user2/repo1/compare/invalid").SetHeader("Accept", "text/html")
 	resp = session.MakeRequest(t, req, http.StatusNotFound)
-	assert.False(t, strings.Contains(resp.Body.String(), "/assets/img/500.png"), "expect 404 page not 500")
+	assert.True(t, test.IsNormalPageCompleted(resp.Body.String()), "expect 404 page not 500")
 }
 
 // Compare with inferred default branch (master)

tests/integration/links_test.go

@@ -37,8 +37,6 @@ func TestLinksNoLogin(t *testing.T) {
 		"/user2/repo1/projects",
 		"/user2/repo1/projects/1",
 		"/user2/repo1/releases/tag/delete-tag", // It's the only one existing record on release.yml which has is_tag: true
-		"/assets/img/404.png",
-		"/assets/img/500.png",
 		"/.well-known/security.txt",
 	}
 

web_src/css/base.css

@@ -477,6 +477,20 @@ img.ui.avatar,
   padding-bottom: 80px;
 }
 
+.status-page-error {
+  margin-top: max(45vh - 90px, 80px);
+  margin-bottom: 80px;
+}
+
+.status-page-error-title {
+  font-size: 48px;
+  margin-bottom: 14px; /* some elements below may use tw-my-4 or tw-my-8, so use 14px as a minimal margin */
+  line-height: initial;
+  text-align: center;
+  font-weight: var(--font-weight-bold);
+  color: var(--color-text-light-2);
+}
+
 /* add margin below .secondary nav when it is the first child */
 .page-content > :first-child.secondary-nav {
   margin-bottom: 14px;

web_src/js/features/comp/ComboMarkdownEditor.js

@@ -122,34 +122,34 @@ class ComboMarkdownEditor {
   }
 
   setupTab() {
-    const $container = $(this.container);
-    const tabs = $container[0].querySelectorAll('.tabular.menu > .item');
+    const tabs = this.container.querySelectorAll('.tabular.menu > .item');
 
     // Fomantic Tab requires the "data-tab" to be globally unique.
     // So here it uses our defined "data-tab-for" and "data-tab-panel" to generate the "data-tab" attribute for Fomantic.
-    const tabEditor = Array.from(tabs).find((tab) => tab.getAttribute('data-tab-for') === 'markdown-writer');
-    const tabPreviewer = Array.from(tabs).find((tab) => tab.getAttribute('data-tab-for') === 'markdown-previewer');
-    tabEditor.setAttribute('data-tab', `markdown-writer-${elementIdCounter}`);
-    tabPreviewer.setAttribute('data-tab', `markdown-previewer-${elementIdCounter}`);
-    const panelEditor = $container[0].querySelector('.ui.tab[data-tab-panel="markdown-writer"]');
-    const panelPreviewer = $container[0].querySelector('.ui.tab[data-tab-panel="markdown-previewer"]');
+    this.tabEditor = Array.from(tabs).find((tab) => tab.getAttribute('data-tab-for') === 'markdown-writer');
+    this.tabPreviewer = Array.from(tabs).find((tab) => tab.getAttribute('data-tab-for') === 'markdown-previewer');
+    this.tabEditor.setAttribute('data-tab', `markdown-writer-${elementIdCounter}`);
+    this.tabPreviewer.setAttribute('data-tab', `markdown-previewer-${elementIdCounter}`);
+
+    const panelEditor = this.container.querySelector('.ui.tab[data-tab-panel="markdown-writer"]');
+    const panelPreviewer = this.container.querySelector('.ui.tab[data-tab-panel="markdown-previewer"]');
     panelEditor.setAttribute('data-tab', `markdown-writer-${elementIdCounter}`);
     panelPreviewer.setAttribute('data-tab', `markdown-previewer-${elementIdCounter}`);
     elementIdCounter++;
 
-    tabEditor.addEventListener('click', () => {
+    this.tabEditor.addEventListener('click', () => {
       requestAnimationFrame(() => {
         this.focus();
       });
     });
 
     $(tabs).tab();
 
-    this.previewUrl = tabPreviewer.getAttribute('data-preview-url');
-    this.previewContext = tabPreviewer.getAttribute('data-preview-context');
+    this.previewUrl = this.tabPreviewer.getAttribute('data-preview-url');
+    this.previewContext = this.tabPreviewer.getAttribute('data-preview-context');
     this.previewMode = this.options.previewMode ?? 'comment';
     this.previewWiki = this.options.previewWiki ?? false;
-    tabPreviewer.addEventListener('click', async () => {
+    this.tabPreviewer.addEventListener('click', async () => {
       const formData = new FormData();
       formData.append('mode', this.previewMode);
       formData.append('context', this.previewContext);
@@ -161,6 +161,10 @@ class ComboMarkdownEditor {
     });
   }
 
+  switchTabToEditor() {
+    this.tabEditor.click();
+  }
+
   prepareEasyMDEToolbarActions() {
     this.easyMDEToolbarDefault = [
       'bold', 'italic', 'strikethrough', '|', 'heading-1', 'heading-2', 'heading-3',

web_src/js/features/repo-issue-edit.js

@@ -179,6 +179,7 @@ async function onEditContent(event) {
   if (!comboMarkdownEditor.value()) {
     comboMarkdownEditor.value(rawContent.textContent);
   }
+  comboMarkdownEditor.switchTabToEditor();
   comboMarkdownEditor.focus();
 }
 

web_src/js/features/repo-issue.js

@@ -1,12 +1,12 @@
 import $ from 'jquery';
 import {htmlEscape} from 'escape-goat';
-import {showTemporaryTooltip, createTippy} from '../modules/tippy.js';
+import {createTippy, showTemporaryTooltip} from '../modules/tippy.js';
 import {hideElem, showElem, toggleElem} from '../utils/dom.js';
 import {setFileFolding} from './file-fold.js';
 import {getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.js';
 import {toAbsoluteUrl} from '../utils.js';
 import {initDropzone} from './dropzone.js';
-import {POST, GET} from '../modules/fetch.js';
+import {GET, POST} from '../modules/fetch.js';
 import {showErrorToast} from '../modules/toast.js';
 
 const {appSubUrl} = window.config;
@@ -673,19 +673,24 @@ export function initRepoIssueBranchSelect() {
   });
 }
 
-export function initSingleCommentEditor($commentForm) {
+export async function initSingleCommentEditor($commentForm) {
   // pages:
-  // * normal new issue/pr page, no status-button
-  // * issue/pr view page, with comment form, has status-button
+  // * normal new issue/pr page: no status-button, no comment-button (there is only a normal submit button which can submit empty content)
+  // * issue/pr view page: with comment form, has status-button and comment-button
   const opts = {};
   const statusButton = document.querySelector('#status-button');
-  if (statusButton) {
-    opts.onContentChanged = (editor) => {
-      const statusText = statusButton.getAttribute(editor.value().trim() ? 'data-status-and-comment' : 'data-status');
-      statusButton.textContent = statusText;
-    };
-  }
-  initComboMarkdownEditor($commentForm.find('.combo-markdown-editor'), opts);
+  const commentButton = document.querySelector('#comment-button');
+  opts.onContentChanged = (editor) => {
+    const editorText = editor.value().trim();
+    if (statusButton) {
+      statusButton.textContent = statusButton.getAttribute(editorText ? 'data-status-and-comment' : 'data-status');
+    }
+    if (commentButton) {
+      commentButton.disabled = !editorText;
+    }
+  };
+  const editor = await initComboMarkdownEditor($commentForm.find('.combo-markdown-editor'), opts);
+  opts.onContentChanged(editor); // sync state of buttons with the initial content
 }
 
 export function initIssueTemplateCommentEditors($commentForm) {