Opened 13 years ago
Closed 13 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: |
Description
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:
- 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
- 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
- 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)
Change History (8)
#2
@
13 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
@
13 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
@
13 years ago
- Owner set to azaozz
- Resolution set to fixed
- Status changed from new to closed
In [17965]:
wpList.add only appends if element with same ID doesn't already exist