WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 4 weeks ago

#42486 assigned defect (bug)

The Tools screen is blank for users who cannot manage categories or tags

Reported by: johnbillion Owned by: desrosj
Milestone: 4.9.8 Priority: normal
Severity: normal Version: 4.9
Component: Administration Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Since Press This was removed in #41689, the Tools screen is only composed of the Categories and Tags Converter.

For users who can't manage categories or tags (Authors and Contributors), the Tools screen is now completely empty. Subscribers currently don't see the Tools admin menu item.

The Tools admin menu item should be removed if there's nothing to display on it.

Attachments (9)

42486-menu.php.patch (1.1 KB) - added by janak007 7 months ago.
42486-tools.php.patch (856 bytes) - added by janak007 7 months ago.
42486.patch (2.8 KB) - added by janak007 7 months ago.
Merged file patch
42486.diff (1.2 KB) - added by dd32 7 months ago.
42486v2.diff (910 bytes) - added by seanchayes 5 months ago.
Refreshed patch and updated display logic based on the comments in this ticket
42486.3.diff (1.3 KB) - added by audrasjb 4 months ago.
Displays a message when tools.php screen is blank (also merging previous patch)
42486.4.diff (1.7 KB) - added by audrasjb 4 months ago.
Fixes toolbox action handling in tools.php
42486.5.diff (1.8 KB) - added by audrasjb 4 months ago.
Fixes inline comment message
42486.2.diff (2.6 KB) - added by desrosj 2 months ago.

Download all attachments as: .zip

Change History (39)

#1 @janak007
7 months ago

  • Keywords has-patch added; needs-patch removed

This would need workaround in two files. 1) menu.php for hiding the tools from admin menu. 2) tools.php for preventing users to access the page from by entering direct URL.

I have added patch for two files. After applying these patches only user with import capability can access to tools page.

#2 @ocean90
7 months ago

  • Version changed from trunk to 4.9

@janak007
7 months ago

Merged file patch

#3 @johnbillion
7 months ago

  • Milestone changed from 4.9.1 to 4.9.2

@dd32
7 months ago

#4 @dd32
7 months ago

A use-case to consider here is when a plugin has added something to the tools page, such as the press this plugin.

I don't think we need to worry about blocking direct access to the page.

The attachments so far on this ticket don't really seem like good solutions, consider these use-cases:

  • User can't import, but can export
  • User can't export, but can import
  • Multisite network has Import/Export disabled, but can access Delete Site
  • Plugin has added submenus to the Tools menu
  • A plugin is adding extra things on the tools page.

To fix this, it's going to need to be some code in wp-admin/includes/menu.php around line 180 which checks that

  • Tools.php doesn't have any sub child nodes
  • !has_action( 'tool_box' ) (to detect custom output on that page) and only then remove the menu from the menu.

42486.diff does just this, and I think this covers enough of the edge cases here. As mentioned in the patch, it's still possible to hit a blank page when the user can import, but can't modify tags/cats, the menu will have subpages of importers/etc in that case. Maybe we could increase the amount of text on the page, showing a description of the page & a "Sorry, nothing you can access right now" message when no tools on offer?

#5 @dd32
5 months ago

  • Milestone changed from 4.9.2 to 4.9.3

Bumping to 4.9.3 due to 4.9.2s release

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


5 months ago

#7 @desrosj
5 months ago

  • Keywords needs-testing needs-refresh added

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


5 months ago

#9 @desrosj
5 months ago

  • Milestone changed from 4.9.3 to 4.9.4

Punting to 4.9.4.

#10 @seanchayes
5 months ago

I refreshed the patch and added the same logic from wp-admin/tools.php to help determine if the Tools menu needs to be displayed.

So, if something adds an action to tool_box, there's some entries for the Tools menu and the current user can manage default taxonomies the Tools menu will be shown.

For testing, I set up some local users - a subscriber, an author and an editor and added a function to a child theme (which could just as well have been a plugin) that added something to the tool_box action.

For each user, and while the added function was in place, the Tools menu displayed. As soon as I removed the function then regardless of non-admin user the Tools menu was hidden.

I couldn't think of any worthwhile text to display on at the wp-admin/tools.php page when it is empty and no submenu items are displayed and nothing is hooked into tool_box so it just shows the Tools heading if you go there directly and are not an admin or with the capabilities tested for.

42486v2.diff is the patch with these changes.

@seanchayes
5 months ago

Refreshed patch and updated display logic based on the comments in this ticket

#11 @chetan200891
5 months ago

I have tested 42486v2.diff

I have created Test Author, it hides Tools menu from Dashboard. But allows access tools.php by entering URL.

#12 @dd32
5 months ago

  • Milestone changed from 4.9.4 to 4.9.5

Bumping, 4.9.4 has been released.

#13 @ketanumretiya030
4 months ago

  • Keywords needs-patch added; has-patch removed

Hi,

I have test with All wordpress user role..

Editor,Contributor,Author Tools menu visible and open tools.php file with blank admin Page.

Administrator : Display below message in admin tools.php when click on Tools menu Categories and Tags Converter

If you want to convert your categories to tags (or vice versa), use the Categories and Tags Converter available from the Import screen.

Subscriber : Hide tool menu

Last edited 4 months ago by ketanumretiya030 (previous) (diff)

#14 @audrasjb
4 months ago

  • Keywords has-patch added; needs-testing needs-refresh needs-patch removed

Hello,

Here is a new patch with a message displayed for users coming from tools.php URL. This is displayed only to avoid blank screen in any case. It applies to editor, author and contributor roles or any custom role without import capabilities.

Cheers, Jb

@audrasjb
4 months ago

Displays a message when tools.php screen is blank (also merging previous patch)

#15 @audrasjb
4 months ago

Ow sorry, I forgot to manage toolbox actions (like in previous patch). Here is a new attempt.

@audrasjb
4 months ago

Fixes toolbox action handling in tools.php

@audrasjb
4 months ago

Fixes inline comment message

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


4 months ago

#17 @audrasjb
4 months ago

  • Keywords 2nd-opinion added

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


3 months ago

#19 @audrasjb
3 months ago

@desrosj @johnbillion @dd32 Are your OK to ship the last patch in 4.9.5?

Many thanks for your feedback! Cheers, Jb

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


3 months ago

#21 @audrasjb
3 months ago

  • Milestone changed from 4.9.5 to 4.9.6

Bumping to 4.9.6 due to 4.9.5 beta release.

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


2 months ago

#23 @desrosj
2 months ago

  • Owner set to desrosj
  • Status changed from new to assigned

#24 @desrosj
2 months ago

I am not sure that I like the idea of copying an existing code block (that checks if the current user has the capability to manage categories or tags) into two different locations.

I think moving the Categories and Tags converter to the action hook would be a better approach. Using this approach would only require the original code proposed in 42486.diff. It would also allow that card to be unhooked from the tools page, if desired.

@desrosj
2 months ago

#25 @desrosj
2 months ago

42486.2.diff is a patch with the approach I talked about above.

  • The Categories and Tags Converter card is moved into a function.
  • That function is attached to the tool_box hook at priority 1.

The new function definitely does not belong in the wp-admin/includes/menu.php file, but I am not sure where the best place to put it is. Also, there could probably be a better name for the function too.

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


2 months ago

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


8 weeks ago

#28 @desrosj
8 weeks ago

  • Milestone changed from 4.9.6 to 4.9.7

Punting due to lack of 2nd opinion.

#29 @desrosj
5 weeks ago

  • Milestone changed from 4.9.7 to 4.9.8

Moving all tickets in 4.9.7 to 4.9.8.

This ticket was mentioned in Slack in #gdpr-compliance by desrosj. View the logs.


4 weeks ago

Note: See TracTickets for help on using tickets.