Bulk actions not correctly applied when selecting bulk actions in both the top and bottom bulk actions dropdowns

[engelen]

In WP_List_Table objects, the bulk actions dropdown and "Apply" button is displayed twice: once below the table and once above the table. The name attribute of the bulk actions dropdown element is always set "action". This wouldn't be a problem if the top and bottom actions were two in different forms.

However, both dropdown elements are in the posts-filter-form. You can start to see the problem here. Two form elements both have the same name, which yields unexpected behaviour when trying to apply bulk actions.

Take the following use case, for example:

• Select some posts from the posts list in /wp-admin/edit.php.
• Select "Edit" from the bulk actions dropdown above the list table.
• Select "Move to Trash" from the dropdown below the list table.
• Click the apply button next to the dropdown on the bottom of the list table.
• The submit request is sent. You would expect the posts to be moved to the trash, but instead, nothing happens.

Screencast of bug: ​http://recordit.co/EjHAbw2KNr

The solution I see would rename the dropdowns for the top and bottom dropdowns (they already have different names) and name the "Apply" buttons. Then, when one of the buttons is pressed, execute the corresponding bulk action.

In any case, we shouldn't have any two form elements with the same name in a single form.

[chris_dev]

Add name attribute to Apply button and check where the action is coming from

[engelen]

Fix bulk actions problems for all object types (including terms) and fix bulk user promotion

[engelen]

@chris_dev Thanks, the patch solves the problem for most objects.

Close inspection reveals that WP_Terms_List_Table overrides get_current_action() from WP_List_Table, and handles bulk deletion of terms wrongly. It doesn't check which "Apply" button is clicked when deleting terms, and deletes the selected term if either of them is clicked and either of the dropdowns was set to "Delete". This wasn't as big a problem as it is since 4.7, when custom bulk actions were introduced. As such, it can lead to unintended permantent deletion of terms. Somewhat major, if I were to judge it.

Furthermore, the same problem exists with user promotion on the users overview. Here, the role selection from the bottom dropdown overrides the role selection from the top dropdown, even if the top apply button is clicked.

Patch "39186.diff" addresses builds on the patch of @chris_dev and addresses the other issues. I would suggest to add (more?) unit tests to the bulk actions functionality, as these problems are bound to pop up more from 4.7 onward.

[bhargavbhandari90]

Rather than we can do like that, if any of bulk action dropdown is changed then also change other at the same time using jQuery.

[azaozz]

39186.diff looks good. Wondering if it makes sense to also add 39186.2.patch to show the action that will be performed at both places in the UI. Maybe best to combine them?

[chris_dev]

Thank you all @engelen @bhargavbhandari90

[engelen]

@azaozz Even though it's nice to have the actions to be performed reflected properly in the UI, I do not think that there is any ambiguity at this point. From a user perspective, the two dropdowns and buttons (at the top and at the bottom of the list table) are separate forms, and they have little to do with each other. Furthermore, with 4.7, many use cases can be considered where the patch would not be enough.

What would happen, for example, if a user would add custom bulk actions with corresponding additional settings added through jQuery? I have already encountered this use case in my own plugin (screencast of a case that would be difficult and strange to copy to the bottom bulk actions: ​http://recordit.co/JYcCMR71JN).

Concluding: I wouldn't say copying the current action to the bottom/top bulk actions is desirable behaviour.

And lastly: wouldn't this be more suited for a minor release? Especially the bug where you can permanently delete categories without intending to do so seems to warrant that.

[chris_dev]

Replying to engelen:

@azaozz Even though it's nice to have the actions to be performed reflected properly in the UI, I do not think that there is any ambiguity at this point. From a user perspective, the two dropdowns and buttons (at the top and at the bottom of the list table) are separate forms, and they have little to do with each other. Furthermore, with 4.7, many use cases can be considered where the patch would not be enough.

What would happen, for example, if a user would add custom bulk actions with corresponding additional settings added through jQuery? I have already encountered this use case in my own plugin (screencast of a case that would be difficult and strange to copy to the bottom bulk actions: ​http://recordit.co/JYcCMR71JN).

Concluding: I wouldn't say copying the current action to the bottom/top bulk actions is desirable behaviour.

And lastly: wouldn't this be more suited for a minor release? Especially the bug where you can permanently delete categories without intending to do so seems to warrant that.

@engelen I reckon it is more suitable for a minor release

[chris_dev]

Hi, I found an issue on the plugin page so I added it up.

[desrosj]

Related #40113

[desrosj]

I found this ticket when working on #40113 which is related to changing roles for users in the edit site section of the network admin. Like @engelen detailed above with term bulk edit and user roles, this area could have potentially large negative consequences.

On the edit site users dialogue in the network admin, the logic in the patch on #40113 (which was copied over from the single site user edit screen) will always respect the second dropdown field.

Say a person is at the bottom of the page and selects a role. Then they scroll up selecting users, and then select a different role in the dropdown at the top. The bottom dropdown value would be applied to all selected users. This could be very bad, and a huge pain if a large number of users had been selected. Especially if the user selects Administrator in the bottom dropdown, and then changes their mind at the top to a lesser role. All users would be promoted to Administrators.

To reproduce, select a role in the dropdown at the top of the table, and then select a different role in the bottom dropdown. Click change in either row, and the second dropdown is the value respected.

I think for certain core fields (like role change, bulk edit), it would make sense to make these values always match.

[jbpaul17]

Punting to 4.8.1 per discussion in today's 4.8 rc1 bug scrub in #core.

[jbpaul17]

Punting to 4.9 per today's bug scrub.

[engelen]

Replying to melchoyce:
...really?

[melchoyce]

Replying to engelen:

Replying to melchoyce:
...really?

Happy to un-punt if someone wants to take ownership and see it to commit.

[engelen]

Replying to melchoyce:

Replying to engelen:

Replying to melchoyce:
...really?

Happy to un-punt if someone wants to take ownership and see it to commit.

Sure. If you change the milestone to 4.9.1 and make me the owner I'll see to it that this gets the attention it needs.

[melchoyce]

@engelen All set :)

[peterwilsoncc]

Switching milestone due to the focus on the new editor (Gutenberg) for WordPress 5.0.

[oglekler]

I managed to reproduce this issue only by changing value of button select by request, otherwise it is handling correctly because changes of both selects now synchronized. But JS script is still picking up a value from the first selector, and it isn't correct behavior for the script. And this patch is only applied to backend. I assume that JS script which is making this action needs to be tested and possibly JS behavior to be improved via additional ticket.