Make WordPress Core

Opened 16 years ago

Closed 16 years ago

#7435 closed enhancement (fixed)

Reply to comments from admin

Reported by: ryan's profile ryan Owned by: azaozz's profile azaozz
Milestone: 2.7 Priority: normal
Severity: normal Version:
Component: Administration Keywords: blessed
Focuses: Cc:

Description

Add ability to reply to comments from the admin (edit-comments.php).

Suggested design:

  • Add "Reply" link to actions
  • Popup reply form when "Reply" clicked
  • Submit reply via AJAX
  • Set comment_parent to the ID of the comment being replied to

Prior art: http://planetozh.com/blog/my-projects/absolute-comments-manager-instant-reply/

Attachments (8)

cg-comment-reply.php (5.1 KB) - added by caesarsgrunt 16 years ago.
OK, so here's my plugin. Warning : I've never used jQuery before... Furthermore, I've only tested in in Firefox and Safari, so far. YMMV, etc.
wp_comment_reply filter call.diff (528 bytes) - added by caesarsgrunt 16 years ago.
Modifies the wp_comment_reply(...) function's wp_comment_reply filter to pass on the variables passed to it.
wp_comment_reply filter call.2.diff (528 bytes) - added by caesarsgrunt 16 years ago.
Modifies the wp_comment_reply(...) function's wp_comment_reply filter to pass on the variables passed to it.
comment-reply-no-js-validate.diff (2.0 KB) - added by nbachiyski 16 years ago.
Hides the Reply button if no js and fixes some HTML validation problems
comment-reply-tabindex.diff (1.1 KB) - added by nbachiyski 16 years ago.
Sane tabindices
comment-action-link-classes.diff (1.1 KB) - added by caesarsgrunt 16 years ago.
Allows action links to specify additional class names such as hide-if-no-js.
hide-if-no-js.diff (499 bytes) - added by caesarsgrunt 16 years ago.
Fix the hide-if-no-js bug.
hide-if-no-js.2.diff (455 bytes) - added by caesarsgrunt 16 years ago.
Better fix for the hide-if-no-js bug

Download all attachments as: .zip

Change History (35)

#1 @ryan
16 years ago

  • Owner changed from anonymous to azaozz

#2 @azaozz
16 years ago

(In [8720]) Reply to comments from admin, first run, see #7435

#3 follow-up: @caesarsgrunt
16 years ago

  • Cc caesarsgrunt added

Great stuff - this is one of my pet peeves.
However, I have a few comments to make about the current code in the trunk.

First, we need MORE HOOKS!!! What if I want to add to the form, rather than replace it entirely? (I do...)

Second, it would be nice if the add-comment function could accept POST values for all the comment metadata. (For example, I want to write a plugin which will allow the admin 'replying' to make the reply be 'from' another user or a visitor whose name, email, url and IP can be set. It would be best if I could still submit the form to the core function rather than replicating it.

Third, I think the form should be inserted in another tr below the comment being replied to. Of course, it's quite simple to write a plugin which does this (I have done so, in fact, and I don't mind uploading some of the code here if anyone's interested in changing the core behaviour).

@caesarsgrunt
16 years ago

OK, so here's my plugin. Warning : I've never used jQuery before... Furthermore, I've only tested in in Firefox and Safari, so far. YMMV, etc.

#4 @caesarsgrunt
16 years ago

The same form could be used to add a new comment to an article without replying to an existing comment. It would just need a link saying 'Add Comment' or similar, which didn't set the comment_parent.

Why is Type set to Defect? And why isn't Component set to Administration?
Is it right for me to change things like that, or leave it for an admin?
Sorry - I'm fairly new here. :)

#5 in reply to: ↑ 3 ; follow-up: @azaozz
16 years ago

  • Component changed from General to Administration
  • Type changed from defect to enhancement

Replying to caesarsgrunt:

First, we need MORE HOOKS!!! What if I want to add to the form, rather than replace it entirely? (I do...)

Could probably add another filter for the textarea tag, however replacing all of the html would give the plugin complete control of how the popup works/looks and would probably be more compatible down the road.

Second, it would be nice if the add-comment function could accept POST values for all the comment metadata. (For example, I want to write a plugin which will allow the admin 'replying' to make the reply be 'from' another user or a visitor whose name, email, url and IP can be set. It would be best if I could still submit the form to the core function rather than replicating it.

Don't think this is a good idea. For single user blogs it wouldn't matter much but for multi-user blogs, one logged-in user pretending to be another user.. Not good.

Third, I think the form should be inserted in another tr below the comment being replied to.

Yes, replying to comments would really benefit from comment threading. Can insert the reply under the comment with just a small change to the current code, however it would be confusing as it won't show there on the blog.

Looking at your plugin: there used to be a "Go back" button after error in an earlier design, but what would the user do when going back? The only possible actions would be either to resubmit the comment resulting in a duplicate comment/another error, or to cancel it.

Showing the reply form as a row in the comments table works good, was thinking to do it like that but went with a draggable popup for more flexibility. Can be changed back if more people prefer it.

#6 in reply to: ↑ 5 ; follow-up: @caesarsgrunt
16 years ago

Replying to azaozz:

Could probably add another filter for the textarea tag, however replacing all of the html would give the plugin complete control of how the popup works/looks and would probably be more compatible down the road.

I think the default form code should passed to the existing filter, allowing plugins to either replace it entirely or just change it slightly and return it. Just like most other WP filters.

Don't think this is a good idea. For single user blogs it wouldn't matter much but for multi-user blogs, one logged-in user pretending to be another user.. Not good.

I didn't mean the default form should alow this, I just meant the PHP script which handles the AJAX submission should support POSTed values for all comment metadata. Plugins can do the rest.
Regarding the specific security issue you mention, the plugin I am developing only allows admins to submit comments 'from' other users. I need it for my own blog because users often email me and then ask me to post the email as a comment from them...

Third, I think the form should be inserted in another tr below the comment being replied to.

Yes, replying to comments would really benefit from comment threading. Can insert the reply under the comment with just a small change to the current code, however it would be confusing as it won't show there on the blog.

I meant the reply form as a row in the table, not my submitted reply below the replyed-to comment. Like in my plugin. Interesting that you had it like that and changed it...

Looking at your plugin: there used to be a "Go back" button after error in an earlier design, but what would the user do when going back? The only possible actions would be either to resubmit the comment resulting in a duplicate comment/another error, or to cancel it.

Two reasons. First, if the user clicked submit by mistake when the comment was blank. But I suppose they could just close and open it again if so.
More importantly, what if the internet connection goes down or times out whilst submitting the comment? If you just close the form the comment will be lost, since when the form is reopened it is cleared. Clicking my 'Go Back' button, however, just un-hides the form field, with it's existing contents still in it. For me, on my slow internet connection (I live on a boat), this is a major consideration - and probably for plenty of other people too.

#7 in reply to: ↑ 6 ; follow-up: @azaozz
16 years ago

Replying to caesarsgrunt:

I think the default form code should passed to the existing filter, allowing plugins to either replace it entirely or just change it slightly and return it. Just like most other WP filters.

That would mean the plugin will have to parse the HTML to modify it. Not that easy/fast as supplying replacement form.

I didn't mean the default form should alow this, I just meant the PHP script which handles the AJAX submission should support POSTed values for all comment metadata. Plugins can do the rest.
Regarding the specific security issue you mention, the plugin I am developing only allows admins to submit comments 'from' other users. I need it for my own blog because users often email me and then ask me to post the email as a comment from them...

Still not convinced we should allow that. There's a good reason why this is not allowed in wp-comments-post.php too. Of course you can make a plugin that would insert the comment directly and doesn't have this restriction.

I meant the reply form as a row in the table, not my submitted reply below the replyed-to comment. Like in my plugin. Interesting that you had it like that and changed it...

This has some other implications: now comments are listed on the write page too, but there they are in the main form. Appending the comment reply form inside the table would break the HTML completely (nested forms).

More importantly, what if the internet connection goes down or times out whilst submitting the comment? If you just close the form the comment will be lost, since when the form is reopened it is cleared. Clicking my 'Go Back' button, however, just un-hides the form field, with it's existing contents still in it. For me, on my slow internet connection (I live on a boat), this is a major consideration - and probably for plenty of other people too.

Agreed. Will add the Back button in the next patch.

#8 in reply to: ↑ 7 @caesarsgrunt
16 years ago

Replying to azaozz:

That would mean the plugin will have to parse the HTML to modify it. Not that easy/fast as supplying replacement form.

True, but I still think it would be nice if the plugin had the option to do so, so as not to have to update the plugin every time slight changes were made to the core.
It would still be able to just replace the whole form if it wanted.

Regardless of whether the HTML for the default form is passed to the filter handler, it is essential that the variables passed to the wp_comment_reply function are to be passed to it's filter. Otherwise the plugin can't populate the hidden fields for checkbox, mode, and position, which are essential for it to work on different pages.

I'll upload a patch in a moment. You'll note that I've passed an array of these values as a third argument - replacing the second one with them breaks something when my plugin isn't enabled.


Still not convinced we should allow that. There's a good reason why this is not allowed in wp-comments-post.php too. Of course you can make a plugin that would insert the comment directly and doesn't have this restriction.

I don't understand where the security problem lies if the PHP script checks to make sure the form was submitted by an admin if any advanced metadata is set, but as you say the plugin can always handle the submission itself so it's not that important.


I meant the reply form as a row in the table, not my submitted reply below the replyed-to comment. Like in my plugin. Interesting that you had it like that and changed it...

This has some other implications: now comments are listed on the write page too, but there they are in the main form. Appending the comment reply form inside the table would break the HTML completely (nested forms).

Ah. I hadn't noticed the comments on the edit post page.
I've modified my plugin so that it will work on this page too, so long as the patch mentioned above is applied. Otherwise there's no sensible way for it to know which page it's on.

For what it's worth I don't like the comments on this page where they are; I think they should be below everything else rather than in one of the meta boxes. I also think it's confusing sorting the comments in the opposite order here from in the rest of the admin.

Anyhow, assuming it stays where it is, I have a few observations to make.

  1. Note that the nested forms thing actually this applies to the Manage Comments page too...
  2. Whilst this is undoubtedly incorrect HTML,
    1. it's only generated by javascript, and
    2. I don't think this makes any difference to the functionality of the form which is submitted by AJAX, does it? Anyhow it works for me in Firefox and Safari, not yet tested in other browsers...
  3. This could be avoided by positioning the reply form in a div outside the other form, adding a blank tr of the appropriate height, and positioning the div absolutely on top of it.
  4. It could also be avoided on the edit post page by moving the comments outside (below) the edit post form, which IMO would be nicer from a UX perspective anyway.


Agreed. Will add the Back button in the next patch.

Good. :-)

Also, a bug report :
After adding a comment or multiple comments using the new reply form - with or without my plugin enabled - the AJAX deletion of comments no longer works, instead following the link and reloading the page as if JavaScript was disabled.
Sorry for such a long response!

@caesarsgrunt
16 years ago

Modifies the wp_comment_reply(...) function's wp_comment_reply filter to pass on the variables passed to it.

@caesarsgrunt
16 years ago

Modifies the wp_comment_reply(...) function's wp_comment_reply filter to pass on the variables passed to it.

@nbachiyski
16 years ago

Hides the Reply button if no js and fixes some HTML validation problems

#9 follow-ups: @nbachiyski
16 years ago

Here is a little patch, which hides the Reply button if we don't have JS. Also, fixes some small HTML validations issues.

#10 @caesarsgrunt
16 years ago

  • Keywords has-patch dev-feedback added

Can anyone see any potential problems or disadvantages to my patch wp_comment_reply filter call.diff, which passes arguments on to the filter?
If not, it would be nice if it could be committed...
(Sorry it went up twice...)

#11 @azaozz
16 years ago

(In [8764]) Pass on variables to the wp_comment_reply filter. Props caesarsgrunt, see #7435

#12 @azaozz
16 years ago

(In [8765]) Hide the Reply button if no js and fix some HTML validation problems. Props nbachiyski, see #7435

#13 in reply to: ↑ 9 @azaozz
16 years ago

  • Keywords has-patch dev-feedback removed

Replying to nbachiyski:

Here is a little patch, which hides the Reply button if we don't have JS. Also, fixes some small HTML validations issues.

Was wondering why removing the logo from the error message? I was thinking to make similar prompt/error message that can be reused for all js prompts currently shown. It would be fully translatable including the buttons and would certainly look better than the standard browser prompt.

#14 @azaozz
16 years ago

(In [8767]) Reply to comments from admin, small improvements and fixes, see #7435

#15 @nbachiyski
16 years ago

Another small fix -- tabindex attribute of the textarea was set to 10 and there were no tabindex for the buttons. Added 1000 for the textarea and 1001 and 1002 for Submit and Cancel respectively.

@nbachiyski
16 years ago

Sane tabindices

#16 @azaozz
16 years ago

(In [8776]) Add tabindex to the comment reply form. Props nbachiyski, see #7435

#17 in reply to: ↑ 9 @caesarsgrunt
16 years ago

Replying to nbachiyski:

Here is a little patch, which hides the Reply button if we don't have JS. Also, fixes some small HTML validations issues.

I don't like the nested spans :

<span class="reply"><span class="hide-if-no-js">...</span></span>

We could get one span only by doing

$actions['reply hide-if-no-js'] = ...

but that wouldn't be good for plugins etc which want to modify the reply link.

Attached is a patch which allows any of the actions links to specify additional class names by optionally being arrays instead of strings.
This also enhances the extensibility of the system by allowing plugins to specify additional classes for action-links which they add.

What does anyone else think of this? Is there anything wrong with this system? Have I broken anything? :)

@caesarsgrunt
16 years ago

Allows action links to specify additional class names such as hide-if-no-js.

#18 follow-up: @caesarsgrunt
16 years ago

  • Keywords has-patch added

The class="hide-if-no-js" on the reply link actually stops it from being shown for comments added via the ajax reply box, since they are added after the script which removes the hide-if-no-js class.

Here's a patch.

@caesarsgrunt
16 years ago

Fix the hide-if-no-js bug.

@caesarsgrunt
16 years ago

Better fix for the hide-if-no-js bug

#19 @caesarsgrunt
16 years ago

I think the newly-added comment should flash green instead of blue. #ceb, perhaps.

#20 @caesarsgrunt
16 years ago

  • Keywords commit added

Let's commit the hide-if-no-js fix, shall we?

#21 in reply to: ↑ 18 ; follow-up: @azaozz
16 years ago

  • Keywords has-patch commit removed

Replying to caesarsgrunt:

The class="hide-if-no-js" on the reply link actually stops it from being shown for comments added via the ajax reply box, since they are added after the script which removes the hide-if-no-js class.

I kind of liked that side effect of not being able to reply to your own reply, but plugins may need that.

#22 @azaozz
16 years ago

(In [8821]) Don't hide reply action on comments added with AJAX, props caesarsgrunt, see #7435

#23 in reply to: ↑ 21 @caesarsgrunt
16 years ago

Replying to azaozz:

I kind of liked that side effect of not being able to reply to your own reply, but plugins may need that.

Hmm, I see what you mean, but it did look untidy and I don't think it was really the expected behaviour...

I see you changed the colour like I suggested too. Great! :)

#24 @caesarsgrunt
16 years ago

There's a bug which causes the delete function to be called repeatedly for any already existing comment after one has been added using this ajax form and the page hasn't been reloaded.

I haven't managed to find the cause of this bug yet, but I'd guess it must be in the code to do with the reply feature rather than the delete code.

I described this bug on #4529, but I have found the following extra details since :
It's not just the second comment. After a comment or comments have been added using the ajax 'reply' form, any time an already existing comment is deleted the delete function is called the number of comments added + 1 times. So if three comments are added, any already existing comments which are deleted will cause the delete comment handler to be called four times with the same comment ID.

Whilst this bug doesn't currently matter except for causing the deleted comment to flash repeatedly if enough comments have been added, if my patch on #4529 is committed the bug will cause it to malfunction. Whilst I can work around it, it makes more sense to fix it.

#25 @azaozz
16 years ago

(In [9098]) Convert the comment reply popup to a temp table row for consistency, add Quick Edit for comments, see #7435

#26 @azaozz
16 years ago

The above patch also fixes the bug with multiple AJAX delete calls. Now all events are reset properly.

#27 @ryan
16 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.