Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#19905 closed defect (bug) (fixed)

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

Reported by: nacin Owned by: 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 4 years ago.
basic fix.
19905.diff (2.5 KB) - added by duck_ 4 years ago.
19905.2.diff (3.2 KB) - added by duck_ 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 @nacin4 years ago

  • Description modified (diff)

@ericlewis4 years ago

basic fix.

comment:2 follow-up: @ericlewis4 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_4 years ago

comment:3 in reply to: ↑ 2 @duck_4 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_4 years ago

comment:4 @duck_4 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".

comment:5 @duck_4 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.

comment:6 follow-up: @duck_3 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.

comment:7 in reply to: ↑ 6 @duck_3 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.