Skip to content

Refactor flash message and remove SanitizeHTML template func#37179

Merged
wxiaoguang merged 4 commits intogo-gitea:mainfrom
wxiaoguang:fix-flash-message
Apr 12, 2026
Merged

Refactor flash message and remove SanitizeHTML template func#37179
wxiaoguang merged 4 commits intogo-gitea:mainfrom
wxiaoguang:fix-flash-message

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented Apr 11, 2026

  1. Fix the "flash message" layout problem for different cases
    • I am sure most of the users should have ever seen the ugly center-aligned error message with multiple lines.
  2. Fix inconsistent "Details" flash message EOL handling, sometimes \n, sometimes <br>
    • Now, always use "\n" and use <pre> to render
  3. Remove SanitizeHTML template func because it is not useful and can be easily abused.
  4. Clarify PostProcessCommitMessage's behavior and add FIXME comment

By the way: cleaned up some devtest pages, move embedded style block to CSS file

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 11, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

image

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 refactors how flash messages are rendered across the web UI to improve layout consistency (single-line vs multi-line), standardize “details” rendering using \n + <pre>, and remove the SanitizeHTML template func from normal web templates to reduce misuse potential (while keeping compatibility for mail templates). It also adjusts commit-message post-processing APIs to make the HTML-in/HTML-out behavior explicit.

Changes:

  • Centralize flash message rendering via ctx.RenderUtils.RenderFlashMessage, with improved alignment and newline handling.
  • Replace SanitizeHTML usage in web templates/handlers with safer alternatives (markup.Sanitize, escaping helpers, or pre-rendered HTML).
  • Update PostProcessCommitMessage to accept/return template.HTML, and update affected call sites.

Reviewed changes

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

Show a summary per file
File Description
web_src/css/devtest.css Moves devtest <h1>/<h2> styling into the CSS file and adds demo form styling.
web_src/css/base.css Updates flash-message styling for <pre> and adds .flex-relaxed-list helper layout class.
templates/repo/issue/view_content/comments.tmpl Switches commit-ref comment rendering to a dedicated sanitized HTML getter.
templates/repo/commit_page.tmpl Removes SanitizeHTML from rendered git note content display.
templates/devtest/toast-and-message.tmpl Wraps devtest content in a container for consistent layout.
templates/devtest/fetch-action.tmpl Cleans up devtest markup, uses new layout helper class, and removes inline styles.
templates/devtest/devtest-header.tmpl Wraps alerts in a container and passes ctx.RootData to alert partial.
templates/base/alert.tmpl Refactors flash rendering to RenderFlashMessage and tweaks 2FA-required message markup/classes.
templates/base/alert_details.tmpl Renders summary/details via <pre> and removes SanitizeHTML usage.
routers/web/repo/pull.go Replaces <br>-based details concatenation with \n and switches to escaping helper.
routers/web/repo/issue_view.go Replaces templates.SanitizeHTML usage with markup.Sanitize and removes unused import.
routers/web/repo/issue_new.go Switches invalid-template flash “Details” to escaped \n-based content.
routers/web/repo/editor_error.go Uses escaping helper and changes JSON fallback error response to a generic message; improves logging.
routers/web/repo/commit.go Adapts to PostProcessCommitMessage HTML-in/HTML-out signature.
routers/web/repo/branch.go Switches push-rejected details to escaping helper.
routers/web/feed/convert.go Replaces templates.SanitizeHTML with markup.Sanitize for feed descriptions.
routers/web/devtest/devtest.go Updates devtest flash messages to use \n and adds mock data for toast/message page.
routers/utils/utils.go Replaces SanitizeFlashErrorString with EscapeFlashErrorString returning escaped template.HTML.
routers/utils/utils_test.go Updates the test to call the renamed function (but expected values still reflect old behavior).
modules/templates/util_render.go Updates commit-message rendering to match new post-processing API and adds RenderFlashMessage.
modules/templates/mail.go Keeps SanitizeHTML available for mail templates (deprecated) by mapping to internal sanitizer.
modules/templates/helper.go Removes SanitizeHTML from web func map; makes sanitizer helper unexported.
modules/templates/helper_test.go Updates sanitizer test to use the unexported helper.
modules/markup/html.go Changes PostProcessCommitMessage signature to template.HTML and adds clarifying FIXME.
models/issues/comment.go Adds a new method to return sanitized HTML for commit-ref comment content.

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

Comment thread routers/utils/utils_test.go
Comment thread modules/templates/util_render.go Outdated
Comment thread templates/base/alert.tmpl
Comment thread models/issues/comment.go Outdated
@wxiaoguang wxiaoguang mentioned this pull request Apr 11, 2026
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

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


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

Comment thread web_src/css/base.css
Comment thread routers/utils/utils_test.go
@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 11, 2026
Copy link
Copy Markdown
Member

@bircni bircni left a comment

Choose a reason for hiding this comment

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

worth a back port?

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 11, 2026
@bircni bircni added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 11, 2026
@wxiaoguang wxiaoguang merged commit 8fcbdf0 into go-gitea:main Apr 12, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 12, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 12, 2026
@wxiaoguang wxiaoguang deleted the fix-flash-message branch April 12, 2026 02:18
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

worth a back port?

Nope. No backport for a non-bug PR

zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 14, 2026
* main:
  Add comment for the design of "user activity time" (go-gitea#37195)
  fix(api): handle missing base branch in PR commits API (go-gitea#37193)
  Refactor htmx and fetch-action related code (go-gitea#37186)
  Fix encoding for Matrix Webhooks (go-gitea#37190)
  Always show owner/repo name in compare page dropdowns (go-gitea#37172)
  fix(api): handle fork-only commits in compare API (go-gitea#37185)
  Improve Contributing docs and set a release schedule (go-gitea#37109)
  Update Nix flake (go-gitea#37183)
  Remove outdated RunUser logic (go-gitea#37180)
  Refactor flash message and remove SanitizeHTML template func (go-gitea#37179)
  Indicate form field readonly via background (go-gitea#37175)
  Remove dead CSS rules (go-gitea#37173)
  Fix flaky `TestCatFileBatch/QueryTerminated` test (go-gitea#37159)
  Implement logout redirection for reverse proxy auth setups (go-gitea#36085)
  Add missing `//nolint:depguard` (go-gitea#37162)
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. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants