WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#29789 closed enhancement (fixed)

Bulk Actions Selectbox is displayed even when user can't edit or trash anything

Reported by: psoluch Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch ux-feedback
Focuses: ui, administration Cc:

Description

What steps should be taken to consistently reproduce the problem?

  1. Log in as author or contributor
  2. Go to the posts page in wp-admin

Problem? The user can't edit or trash anything, yet the bulk actions select box and check-all-checkboxes are displayed. This is confusing for users.

Solution The users might be able to edit or trash posts created by himself so checking for capabilities is not enough. When the bulk edit selectbox is displayed it's not yet known if the user can edit or trash something so it's easier to use javascript to hide the unwanted elements.

Patch will follow below (not sure if the function is in the right place)

Attachments (4)

bulk_edit.patch (1.0 KB) - added by psoluch 4 years ago.
patch to hide bulk edit selectbox when user can't edit or trash anything on post list
29789.diff (1.3 KB) - added by DrewAPicture 3 years ago.
29789.fix2.diff (1.2 KB) - added by kitchin 3 years ago.
revert php, add js
29789.fix3.diff (1.2 KB) - added by DeBAAT 3 years ago.
Alternative patch.

Download all attachments as: .zip

Change History (16)

@psoluch
4 years ago

patch to hide bulk edit selectbox when user can't edit or trash anything on post list

#1 @psoluch
4 years ago

  • Keywords has-patch added

#2 @johnbillion
4 years ago

  • Keywords ux-feedback added
  • Type changed from defect (bug) to enhancement
  • Version trunk deleted

@DrewAPicture
3 years ago

#3 @DrewAPicture
3 years ago

  • Focuses javascript removed
  • Milestone changed from Awaiting Review to 4.4
  • Summary changed from UX bug: Bulk Actions Selectbox is displayed even when user can't edit or trash anything to Bulk Actions Selectbox is displayed even when user can't edit or trash anything

Hi @psoluch,

I agree that it's a little strange to display the bulk actions options if you can't actually use any of them. I would think we'd probably want to handle this directly in the list table classes instead of trying to remove the element after the fact with JavaScript.

I've added 29789.diff which handles hiding bulk actions for the posts list table, and would be a good starting point for patching the other list tables, if you're interested.

#4 @kitchin
3 years ago

Do you need to modify class-wp-list-table.php ? Seems the patch to class-wp-posts-list-table.php suffices. Plugins may use the empty html placeholder for bulkactions, especially since there are few hooks in list table rendering.

#5 @wonderboymusic
3 years ago

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

In 33713:

Posts List Table:

Don't show bulk actions if the user can't edit posts.

Props DrewAPicture.
Fixes #29789.

#6 @DeBAAT
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Sorry, just saw the changeset passing on my twitter timeline and don't know how to comment on that changeset. So I try to get my point across over here.

Personally, I think it is wrong to check for 'edit_posts' when you are deleting a post.

I agree, the check for 'delete_posts' should be implemented correctly. There might be situations where a user is not allowed to edit_posts but may absolutely delete_posts.

I don't know exactly how to produce a diff, but my suggestion would be to have the check implemented as follows, starting from line 285:

	if ( current_user_can( $post_type_obj->cap->edit_posts ) ) {
		if ( $this->is_trash ) {
			$actions['untrash'] = __( 'Restore' );
		} else {
			$actions['edit'] = __( 'Edit' );
		}
	}

#7 @kitchin
3 years ago

Probably should revert and re-read the original bug report. Caps depend on author owning at least one post in the list.

The javascript patch by psoluch ("bulk_edit.patch​") may be the correct way to go.

@kitchin
3 years ago

revert php, add js

#8 @kitchin
3 years ago

I didn't understand the original "bulk_edit.patch" so I wrote one similar to "Disable upload buttons until files are selected" in common.js.

Caveats:

  • Affects all '.wp-list-table'. I tested for an author on Posts.
  • Plugin authors using the checkbox column in unknown ways may need to .show().

#9 @DeBAAT
3 years ago

I still have problems testing something different than intended.

As far as I know, the test should be on whether the user has some capabilities or not. If a certain post is restricted for that particular capability, it should be the responsibility of that post to prevent unauthorized actions.

In the meantime, I figured out how to make a diff, so I created my diff suggestion against the current development version (4.4-alpha-33717).

See file 29789.fix3.diff.

@DeBAAT
3 years ago

Alternative patch.

#10 @DeBAAT
3 years ago

Another difference with the other patches is that they also remove the ability to bulk delete posts. My patch will test only those parts related to the capability checked.

#11 @kitchin
3 years ago

@DeBAAT : The caps per post are checked for each row and that determines whether a checkbox is present on that row. Some sites may have delete/edit caps in odd configurations. The bug is to hide or remove the select dropdown when it cannot be used on the list displayed.

In wp-admin/post.php and wp-admin/edit.php you can see 'untrash' is checked against current_user_can( 'delete_post', $post_id). Rather than guessing with the overall caps 'delete_posts' and 'edit_posts' (note the "...s"), it makes more sense to have the <select> UI agree with the checkboxes.

To make it more accurate, we could add data to each checkbox for what actions are available on that row, and then reduce the <options> as needed. But aside from that, I think using the UI is an accurate solution.

For testing, start with the Hello World post, and then as a Contributor create a new draft. In the posts lists you will have one checkbox and <select>. Trash the draft and you will have no checkboxes and no <select>. Switch to "Trash" and you will have one checkbox and <select>.

#12 @wonderboymusic
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 33733:

Instead of [33713], allow WP_Posts_List_Table::get_bulk_actions() to check edit_posts AND delete_posts.

Props DeBAAT.
Fixes #29789.

Note: See TracTickets for help on using tickets.