Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#35904 closed defect (bug) (fixed)

Comments screen: extra AJAX requests when undoing spam or trash actions

Reported by: afercia Owned by: SergeyBiryukov
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.4
Component: Comments Keywords: has-screenshots has-patch
Focuses: ui, javascript Cc:


Introduced in [34977] and [33655].

To easily reproduce and clearly see what's happening, I'd recommend to be sure to have several comments in the comments list table and then set "Number of items per page" in the Screen Options to 2.

Inspect the Network panel in your browser console and Trash a comment. So far so good, one request to delete the comment row and one request to refill the list adding one comment row from the "extra" hidden list.

Now click on "Undo" and... four requests! And an extra row gets added at the bottom of the list. This shouldn't happen. See in the screenshot below:


In WordPress 4.3 the "undo" action triggers just one request, the only one that's needed to delete the "undo" row. So what's happening here?

First part: [34977]

Worth reminding when working with jQuery events, return false and preventDefault() are not exactly the same thing. return false does both preventDefault() and stopPropagation().

Since now there's only preventDefault() the click event bubbles up the DOM until it finds a click event delegated on the body targeting the same "undo" link
This click event is actually in wp-lists.js, see:

$el.delegate( '[data-wp-lists^="delete:' + list.id + ':"]', 'click', function(){

a bit too more generic selector but I guess that's intentional since wp-lists.js is intended for general purpose.

To fix this the event bubbling must be stopped, as return false was doing before the change.

Second part: [33655]

Repeat the test, when clicking "undo" there's still one extra request running.
Before [33655] there was a check for the untrash || unspam actions in order to return before calling wpList.add() and refillTheExtraList().

if ( ! theExtraList || theExtraList.size() === 0 || theExtraList.children().size() === 0 || untrash || unspam ) {

So there's still the need to return when "undoing", this can be done in several ways, I'd keep it simple and just use a flag.

Attachments (1)

35904.patch (1.2 KB) - added by afercia 6 years ago.

Download all attachments as: .zip

Change History (5)

6 years ago

#1 @afercia
6 years ago

  • Keywords has-patch added

First pass.

This ticket was mentioned in Slack in #core-editor by afercia. View the logs.

6 years ago

This ticket was mentioned in Slack in #core by afercia. View the logs.

6 years ago

#4 @SergeyBiryukov
6 years ago

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

In 36774:

Comments: Avoid extra AJAX requests when undoing Spam or Trash actions.

Props afercia.
Fixes #35904.

Note: See TracTickets for help on using tickets.