Skip to content

Fix various problems#37077

Merged
wxiaoguang merged 5 commits intogo-gitea:mainfrom
wxiaoguang:fix-various
Apr 3, 2026
Merged

Fix various problems#37077
wxiaoguang merged 5 commits intogo-gitea:mainfrom
wxiaoguang:fix-various

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented Apr 2, 2026

Quick fix for 1.26. Complex change is not in this PR's scope. Related to existing PRs which are still WIP or have various problems:


  • Slightly refactor NewComment to fix incorrect responses, remove incorrect defer (still far from ideal)
  • Avoid const causes js error in global scope
  • Don't process markup contents on user's home activity feed, to avoid js error due to broken math/mermaid code

Diff with ignoring spaces: https://github.com/go-gitea/gitea/pull/37077/files?diff=unified&w=1

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2026
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/frontend labels Apr 2, 2026
@wxiaoguang wxiaoguang force-pushed the fix-various branch 3 times, most recently from fc6e6dd to 31b5702 Compare April 2, 2026 05:09
@wxiaoguang wxiaoguang added this to the 1.26.0 milestone Apr 2, 2026
@wxiaoguang wxiaoguang force-pushed the fix-various branch 2 times, most recently from b12ca80 to acec5f3 Compare April 2, 2026 08:19
@wxiaoguang wxiaoguang requested a review from Copilot April 2, 2026 08:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR applies a set of targeted fixes for the 1.26 line, focusing on preventing frontend JS errors from truncated markup rendering, avoiding script-scope conflicts in the PR merge box, and refactoring the issue comment creation flow to avoid incorrect/mixed responses.

Changes:

  • Skip initializing markup features for truncated markup blocks (e.g. dashboard activity feed) while clearing backend “loading” state.
  • Wrap the pull merge box inline module script to avoid duplicate top-level declarations.
  • Refactor NewComment to remove the problematic deferred response write and tighten empty-comment validation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
web_src/js/markup/content.ts Skips markup feature initialization for .truncated-markup and clears .is-loading.
templates/user/dashboard/feeds.tmpl Marks dashboard feed comment markup as truncated via truncated-markup class.
templates/repo/issue/view_content/pull_merge_box.tmpl Wraps inline module script in an IIFE to avoid duplicate declarations.
routers/web/repo/issue_comment.go Refactors comment creation + close/reopen handling and redirect response flow.
options/locale/locale_en-US.json Adds a new i18n key for empty comment validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 2, 2026

Don't process markup contents on user's home activity feed, to avoid js error due to broken mermaid code

Seems ok, we can close the 2 (?) WIP PRs for that with that fix. I discontinued mine because the ask had gotten too complex. Or maybe just keep the simple approach of ommitting code blocks (my initial approach).

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Don't process markup contents on user's home activity feed, to avoid js error due to broken mermaid code

Seems ok, we can close the 2 (?) WIP PRs for that with that fix. I discontinued mine because the ask had gotten too complex. Or maybe just keep the simple approach of ommitting code blocks (my initial approach).

My thoughts about the related PRs:

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 2, 2026

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 2, 2026
@GiteaBot GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Apr 2, 2026
@GiteaBot GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Apr 2, 2026
@bircni bircni requested a review from silverwind April 2, 2026 21:34
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 2, 2026
@wxiaoguang wxiaoguang merged commit 7b17234 into go-gitea:main Apr 3, 2026
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 3, 2026
@wxiaoguang wxiaoguang deleted the fix-various branch April 3, 2026 02:25
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2026
* main:
  Fix RPM Registry 404 when package name contains 'package' (go-gitea#37087)
  Improve actions notifier for `workflow_run` (go-gitea#37088)
  Refactor code render and render control chars (go-gitea#37078)
  Fix various problems (go-gitea#37077)
  [skip ci] Updated translations via Crowdin
  Support legacy run/job index-based URLs and refactor migration 326 (go-gitea#37008)
  Fix a bug when forking a repository in an organization (go-gitea#36950)
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 3, 2026

So what is the fix for the frontpage? Still "Don't process markup contents on user's home activity feed"? I think it's bad to show unprocessed markup. I imagine we need a AbbevivateMarkup(markup: string, numChars: int) function which could useful in more than just this place (ContextPopup).

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

imagine we need a AbbevivateMarkup(markup: string, numChars: int) function which could useful in more than just this place (ContextPopup).

I think it needs to skip the failed rendering, don't show eye-catching errors:

image

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 3, 2026

I think it needs to skip the failed rendering, don't show eye-catching errors

Yes, no cut-off code blocks, math blocks, inline math, inline code. that makes no sense when being cut off in the middle. Only cut off in the middle inside plain text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mermaid code block truncated on homepage causes parse error

6 participants