Unload handler for inline replies to comments displays "Are you sure.." alert when nothing has been entered

[davidbaumwald]

While investigating #54621, there's a similar(perhaps related) issue happening in wp-admin/edit-comments.php.

To reproduce:

1. Click "Reply" on an existing comment.
2. Don't enter anything in the "Reply to Comment" textarea.
3. Click on "Reply" or "Quick Edit" on any other comment.
4. Observe the "Are you sure..." alert.

Resetting the originalContent property to an empty string when the action is not edit seems to fix the issue.

[davidbaumwald]

Screenshot of the alert

[davidbaumwald]

Related #54621.

[cu121]

Fix provided for ticket #54990

[mkox]

I'm not sure if this a fix indeed. It should be fixed in the plain source code an not in the minified js, I guess.

I think a part of the problem is that the replycomment area is used for the edit of comments as well - at least the id.

I will have a look into it.

[azouamauriac]

hi @davidbaumwald when i'm trying to upload the fix i get the following error :
svn: E155010: The node 'src/wp-admin/js/edit-comments.js' was not found.

could you please explain me what happen?

[but when i made changes in src/js/_enqueues/admin/edit-comments.js svn show and handle those changes.] thanks.

[mkox]

Hi @azouamauriac,

'/src/wp-admin/js' is ignored by .gitignore file. It will be built from '/src/js/_enqueues/admin/edit-comments.js' during build process instead.

[azouamauriac]

set originalContent to empty when the action is reply

[azouamauriac]

Replying to mkox:

Hi @azouamauriac,

'/src/wp-admin/js' is ignored by .gitignore file. It will be built from '/src/js/_enqueues/admin/edit-comments.js' during build process instead.

@mkox thank you i've submitted the patch

[mkox]

The patch seems to solve the explained problem to me.

Anyway the behavior could be somehow improved for consistent behavior. At the moment even with this fix the check only works, if reply or quick edit are selected on another comment. If a user chooses to (slow) edit another comment, there is no check and the link is just opened. Maybe worth a new ticket.

[cu121]

@mkox can I upload the patch for this still? I am new here I am not aware of the process for uploading the patch. Could you please inform me, as i can see the patch is uploaded by another user

[azouamauriac]

Replying to cu121:

@mkox can I upload the patch for this still? I am new here I am not aware of the process for uploading the patch. Could you please inform me, as i can see the patch is uploaded by another user

hi i'm new here too, to submit a patch read this​https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/.

feel free to submit a new patch.

regards

[cu121]

Fixed #54990, This patch fixed both issues consists with QuickEdit and Reply comment if the reply content is empty.

[hasanuzzamanshamim]

I tested this issue with the provided patch; this will logically work for this case only:

1. click on "reply"
2. click on "quick edit".

it works perfectly cause this time the reply content is empty.
But if somehow I trigger the "reply" again without changing any value this will throw the alert. cause the value we checked is not empty now.

$( '#replycontent', editRow ).val() // not empty when quick edit

I think this hardcoded empty string check will not solve this issue for edit reply.

        discardCommentChanges: function() {
                var editRow = $( '#replyrow' );
                
                if  ( '' === $( '#replycontent', editRow ).val() ) {
                        return true;
                }

                return window.confirm( __( 'Are you sure you want to do this?\nThe comment changes you made will be lost.' ) );
        }

[cu121]

@hasanuzzamanshamim edit comment is synchronous action, based on the issue explained this fix was provided specifically for asynchronous actions like reply, quick edit, etc. I guess creating a separate ticket for that will be awesome as well :)

[pls78]

Hi! I have tested the patch #2: https://core.trac.wordpress.org/attachment/ticket/54990/54990.2.diff and it works! Great job!
I'm linking a short video of the test: ​https://www.loom.com/share/cc643ad41b4248f59fb370143968572e?sharedAppSource=personal_library

[hasanuzzamanshamim]

Replying to cu121:

@hasanuzzamanshamim edit comment is synchronous action, based on the issue explained this fix was provided specifically for asynchronous actions like reply, quick edit, etc. I guess creating a separate ticket for that will be awesome as well :)

I think you missed something, see this test case you will get the issue.
​https://drive.google.com/file/d/1EHXYs5REL8llvs4hGktzi1w_ittIS8v3/view?usp=sharing

[cu121]

@hasanuzzamanshamim since the field content is not empty, it will definitely show a "Are you sure?" alert when clicked on another comment 'Quick Edit'. Because you are editing the existing comment, which was not empty. But in the case of a reply, the field was empty, so on another comment, if the reply is clicked then it was showing 'Are you sure?' alert was a bug because the reply content was empty. Based on the issue reported, this was the specific fix provided.

Additionally, the id of the reply field and edit field are the same.

[cu121]

This fix is final, it involves solving the issue for quick edit and no reply content. (Quick Edit) If the original content of the quick edit message is not matched with the updated message, then it will show an alert.

[cu121]

@hasanuzzamanshamim thank you for pointing out the issue with your specific test case :).

Trac ticket: https://core.trac.wordpress.org/ticket/54990

I remove t.originalContent in line 970 because it's overwriting the value so I think we don't need this.

[hellofromTonya]

6.0 RC1 is tomorrow and today is the last beta before RC starts. The patch needs testing and review. As this bug was not introduced in the 6.0 cycle, moving the ticket to 6.1.

---Code Explanation---
Added fix of solving the issue for quick edit and no reply content. (Quick Edit) If the original content of the quick edit message is not matched with the updated message, then it will show an alert. Code Modified On-Line Number 1228

Trac ticket: https://core.trac.wordpress.org/ticket/54990

[faisal03]

I have tested the PR ​https://github.com/WordPress/wordpress-develop/pull/2762 and it is working as expected.

See the video: ​https://www.screencast.com/t/LwB4xthLrc

[JeffPaul]

@faisal03 thanks for the testing, @davidbaumwald I'll leave this to you if you want to try and get in before 6.1 Beta 1 (or leave for during the beta cycles).

Thanks for the PR! This was merged into core with https://core.trac.wordpress.org/changeset/54334.

Thanks for the PR! This was merged into core with https://core.trac.wordpress.org/changeset/54334.

[davidbaumwald]

Not sure why Trac didn't capture the commit, but this was resolved in r54334.

@dream-encode Thank you for accepting my PR.