Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#17483 closed defect (bug) (fixed)

Comments displayed multiple times in edit moderated comments screen

Reported by: mintindeed Owned by: azaozz
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.1.2
Component: Comments Keywords:
Focuses: Cc:


When moderating comments on WP Admin > Comments > Pending, there is a race condition in the ajax code that can cause the same comment to be displayed multiple times.

If you change the status (approve, spam, trash) of a comment, then change the status of another comment before the ajax response from refillTheExtraList() (located in trunk/wp-admin/js/edit-comments.dev.js) is returned, then the response from refillTheExtraList() triggered by the second comment will populate the extra list with a duplicate comment. This isn't immediately obvious unless you are moderating enough comments to cause the duplicate comments to float to the top and appear in the comments list. At that point, you may have multiple instances of the same comment (with the same id) visible on the page, which will start causing weird things to happen when you try and modify them (since there are now multiple comments on the page with the same ID).

Here are steps to get it to reproduce, it can be reproduces in WP 3.1.2 as well as trunk as of svn r17949:

  • Go to WP Admin > Comments > Pending and view the contents of the hidden the-extra-comment-list table using firebug etc. Note the comments IDs that are present. (It's easier if you change the number of comments shown to something small, like 5.)
  • Using a proxy like Charles or Fiddler, throttle your connections to the server so that admin-ajax.php will take a while to respond (long enough for you to change the status of multiple comments).
  • Approve a bunch of comments and watch the contents of the the-extra-comment-list table. You will start seeing duplicate comments appear.
  • Once you’ve approved enough comments, the duplicate comments start getting appended to the "real" comments list.
  • Once the "real" comments list is appended with duplicate comments and you start trying to interact with the duplicates, the page starts getting jacked up.

We have coded up a fairly simple patch to wpList (/wp-includes/js/wp-lists.dev.js) that checks whether an element with the same ID exists before appending it to the extra list. We put it here because this same condition may present itself in other places where the wpList.add method is used.

This is only half the solution, however, because if you consistently moderate enough comments that this issue presents itself, and you encounter this race condition more than 8 times, the extra list will eventually shrink to 1 row, and will eventually shrink the number of comments presented for moderation down to 1 because you're moderating faster than the server can fill the list.

I'm not sure if the answer here is:

  1. to call refillTheExtraList() using an exponential backoff if an element of the extra list already exists with the same ID (which presents some additional challenges since refillTheExtraList() is implemented as a private method), or
  2. to implement a queue so that each subsequent time refillTheExtraList() is called its offset is increased by the number of admin-ajax.php called still haven't responded (which would need some significant refactoring of the code), or
  3. just let the list dry up and let the user page forward or refresh if they run out of comments, since it's an edge case already (the easiest solution, but maybe not the correct one)

Attachments (1)

wp-lists.dev.js.patch (370 bytes) - added by mintindeed 10 years ago.
wpList.add only appends if element with same ID doesn't already exist

Download all attachments as: .zip

Change History (8)

10 years ago

wpList.add only appends if element with same ID doesn't already exist

#1 @yoavf
10 years ago

  • Cc yoavf added

#2 @mintindeed
10 years ago

Another possibility we discussed here at the office: wpList.add should stay simple and just do what it's told, add a row if that row doesn't already exist. The calling function (refillTheExtraList) might be the one who should figure out "hey, the extra list has fewer rows than it's supposed to" and then request the number of rows it needs.

#3 @azaozz
10 years ago

Yes, ideally the-extra-comment-list should be refilled with several comments at a time (10?) and the XHR request should happen when it starts to get used, not on the comment moderation clicks.

However that would mean refactoring this functionality which can perhaps happen for 3.3. For now, this patch seems to fix the problem.

#4 @azaozz
10 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [17965]:

Fix race condition in the comments river XHR, props mintindeed, fixes #17483

#5 @greuben
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[17965] broke custom-fields list table.

Steps to reproduce:

  1. Edit a post
  2. Enter a custom field key and value then click 'Add custom field'
  3. The custom fields #list-table is not updated.

#6 @ocean90
10 years ago

  • Milestone changed from Awaiting Review to 3.2

#7 @azaozz
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [17981]:

Fix updating of custom fields, fixes #17483

Note: See TracTickets for help on using tickets.