Skip to content

Edit comment bug: can not save empty comment #29986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
yp05327 opened this issue Mar 22, 2024 · 9 comments · Fixed by #30706
Closed

Edit comment bug: can not save empty comment #29986

yp05327 opened this issue Mar 22, 2024 · 9 comments · Fixed by #30706
Labels
type/bug
Milestone
1.22.0

Comments

@yp05327
Copy link
Contributor

yp05327 commented Mar 22, 2024 •
edited by wxiaoguang
Loading

Description

  1. can not save empty comment
    I think we need to check oldContent == comment.Content instead of len(comment.Content) == 0
    and return an empty json, then check data.content === undefined in js.
    image

(the second one has been fixed)

  1. edit comment regression

Caused by #29843
after the edition you will see this strange code:
image
If I revert the changes from #29843, it works
Have no idea how to fix this, as lint not allow using append.
maybe other changes in #29843 will also have this issue?
the appended data will be text not DOM element.
image

Gitea Version

latest
@wxiaoguang wxiaoguang added the issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP label Mar 22, 2024
@wxiaoguang wxiaoguang mentioned this issue Mar 22, 2024
Merged
@wxiaoguang wxiaoguang added this to the 1.22.0 milestone Mar 22, 2024
@HEREYUA HEREYUA mentioned this issue Mar 22, 2024
Merged
@yardenshoham
Copy link
Member

silverwind pushed a commit that referenced this issue Mar 22, 2024
@HEREYUA
Fix: Abnormal strings appear when comments are saved after editing (#…
bf34723 
…29991)

Partially resolved（The second problem）：
[#29986](#29986)

**Before**
HTML strings appear when comments are saved after editing


![image](https://github.com/go-gitea/gitea/assets/37935145/c356d99a-8473-4cc5-8e38-1b207ccd8b12)


**After**



https://github.com/go-gitea/gitea/assets/37935145/525601f9-3ee1-4266-9105-36d82b91b1c8
@yardenshoham
Copy link
Member

@wxiaoguang should we remove issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP now?

@wxiaoguang wxiaoguang removed the issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP label Mar 22, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 22, 2024 •
edited
Loading

Oh yes, I just added it for remembering (to make sure it could be fixed in 1.22 milestone). It gets fixed pretty fast. Thank you.

And I edited the description to make it clear. 😁

@wxiaoguang wxiaoguang changed the title Edit comment bug Edit comment bug: can not save empty comment Mar 22, 2024
@wxiaoguang wxiaoguang added issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail and removed type/bug labels Mar 22, 2024
@wxiaoguang
Copy link
Contributor

For the first one: "can not save empty comment"

I think it could be considered as not a bug. I think it doesn't make sense to update to an empty content. Especially in some templates, it checks if .Content to use different layouts.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 25, 2024

For the first one, actually what I want to say is that the JSON returns empty content, and we have no empty check in JS, so this will cause a bug here. Also for the dropzone-attachments. Although the second one was fixed, but you will still see it, if you want to clear the comment.

In the latest version (1.22.0+dev-1175-g428e05662f)
image
image

@wxiaoguang wxiaoguang added type/bug and removed issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail labels Mar 25, 2024
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this issue Mar 26, 2024
@HEREYUA @earl-warren
Fix: Abnormal strings appear when comments are saved after editing (#…
9cf4ab0 
…29991)

Partially resolved（The second problem）：
[#29986](go-gitea/gitea#29986)

**Before**
HTML strings appear when comments are saved after editing

![image](https://github.com/go-gitea/gitea/assets/37935145/c356d99a-8473-4cc5-8e38-1b207ccd8b12)

**After**

https://github.com/go-gitea/gitea/assets/37935145/525601f9-3ee1-4266-9105-36d82b91b1c8
(cherry picked from commit bf34723491dcbb45dee7888c574e295cae6096be)
@yp05327
Copy link
Contributor Author

yp05327 commented Apr 9, 2024

I noticed that we have no empty checking in API, so users can "save empty comment" by using API but not from web UI.

API:
image
Web UI:
image

@silverwind
Copy link
Member

I think we should do this (both UI and API):

  • Allow new Issue/PR creation with empty content
  • Allow editing of comments to become empty
  • Forbid creation of comment with empty content

@wxiaoguang wxiaoguang mentioned this issue Apr 25, 2024
Merged
@wxiaoguang
Copy link
Contributor

A quick fix: Allow to save empty comment #30706

@yp05327
Copy link
Contributor Author

yp05327 commented Apr 26, 2024

I think we should do this (both UI and API):

  • Allow new Issue/PR creation with empty content
  • Allow editing of comments to become empty
  • Forbid creation of comment with empty content

It seems that all of them will be finished after #30706 merged.

wxiaoguang added a commit that referenced this issue Apr 26, 2024
@wxiaoguang
Allow to save empty comment (#30706)
6a07501 
Fix #29986
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
1.22.0
Development

Successfully merging a pull request may close this issue.

Allow to save empty comment wxiaoguang/gitea
4 participants
@silverwind @wxiaoguang @yp05327 @yardenshoham