Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#19905 closed defect (bug) (fixed)

Filtering the edit-comments.php screen results in incorrect Pending count

Reported by: nacin's profile nacin Owned by: duck_'s profile duck_
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch
Focuses: Cc:

Description (last modified by nacin)

  1. Go to edit-comments.php and view the comments for a single post (click the bubble, or do ?p=1234).
  1. Note the Pending comment number, say, 4, which would be lower than the total number of pending comments, say, 10.
  1. Trash an unapproved (pending) comment.
  1. (Expected) The pending comment number in the toolbar and admin menu should reduce to 9. The Pending number on edit-comments.php should reduce to three.

Actual: All three numbers are set to 9.

Tracked it down with ryan to delAfter getting the pending count from $('span.pending-count').eq(0). Three elements match span.pending-count -- the admin menu, Pending (0), and the toolbar, in that order. eq(0) means it pulls from the global pending #, from the admin menu.

If you change it to eq(1), then the toolbar and admin menu are both updated to 3. All three values are linked no matter what. Normally, this is proper. But in the case of a filtered edit-comments.php, they must not be.

Attachments (3)

19905.patch (24.8 KB) - added by ericlewis 12 years ago.
basic fix.
19905.diff (2.5 KB) - added by duck_ 12 years ago.
19905.2.diff (3.2 KB) - added by duck_ 12 years ago.

Download all attachments as: .zip

Change History (10)

#1 @nacin
12 years ago

  • Description modified (diff)

@ericlewis
12 years ago

basic fix.

#2 follow-up: @ericlewis
12 years ago

To deal with the issue, my patch moves the .each() out of the updatePending function, and now requires a 'span.pending-count' element as a parameter. This way, for each pending-count the new pending count can be calculated independently, and then updated only for that element.

@duck_
12 years ago

#3 in reply to: ↑ 2 @duck_
12 years ago

  • Keywords has-patch added

Replying to ericlewis:

To deal with the issue, my patch moves the .each() out of the updatePending function

I took a slightly different approach. I left the .each() loop in updatePending() and instead of passing a new total to the function you pass the difference (i.e. 1 or -1). This is good because the difference only needs to be calculated once instead of every loop iteration, only the current total for each element needs to be grabbed. It also allows a lot of code deduplication in dimAfter().

I also moved the call to dashboardTotals() out of the loop as it only needs to be called once. I'm not even sure if it needs to be called in updatePending() at all.

P.S. for future reference you don't need to include the minified version of CSS/JS in patches as the committer (well bumpbot now, see #19592) will do the minification.

@duck_
12 years ago

#4 @duck_
12 years ago

19905.2.diff moves a number of function declarations into the closure, see [17832] when they became global. It also fixes the incorrect pending numbers when filtered on a single post and a comment is approved via "Approve and reply".

#5 @duck_
12 years ago

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

In [19895]:

Fix incorrect pending comment count updates for filtered views. Fixes #19905.

Pass the difference (i.e. 1 or -1) to updatePending() so that the current
pending count is changed for each element independently. Also move several
functions back out of the global scope, see r17832 for their introduction.

#6 follow-up: @duck_
12 years ago

In [20535]:

Don't call updatePending() if the pending count hasn't changed. See #19905.

Stops a zero pending count bubble being shown when unspamming and approved comment
when there are no pending comments.

#7 in reply to: ↑ 6 @duck_
12 years ago

Replying to duck_:

Stops a zero pending count bubble being shown when unspamming and approved comment
when there are no pending comments.

*when unspamming an approved

Note: See TracTickets for help on using tickets.