WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 6 months ago

#49268 closed defect (bug) (fixed)

Broken themes should always be listed

Reported by: johnbillion Owned by: johnbillion
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Themes Keywords: good-first-bug has-patch needs-testing
Focuses: administration Cc:

Description

The list of broken themes on the Appearance screen only appears if the user can edit themes (current_user_can( 'edit_themes' )). This means if DISALLOW_FILE_MODS is set to true then the list of broken themes doesn't appear, which is confusing if the user knows for a fact there is a broken theme present.

The list should always been shown. The Delete and Install parent theme buttons should be hidden or shown depending on the edit_themes capability.

Attachments (2)

49268.diff (459 bytes) - added by rebasaurus 7 months ago.
Remove capability check on displaying broken themes
49268.patch (913 bytes) - added by rebasaurus 7 months ago.
Since edit_themes cap is changed by the constant DISALLOW_FILE_MODS, we'll use edit_theme_options to remove the dependency as mentioned by @sorenbronsted

Download all attachments as: .zip

Change History (13)

@rebasaurus
7 months ago

Remove capability check on displaying broken themes

#1 @rebasaurus
7 months ago

  • Keywords has-patch added; needs-patch removed

#2 @johnbillion
7 months ago

  • Keywords needs-patch added; has-patch removed

Thanks for the patch @rebasaurus! This needs some more work to account for the Delete and Install parent theme buttons as mentioned in the summary.

#3 @sorenbronsted
7 months ago

Just for the record I will try to fix this.

#4 @sorenbronsted
7 months ago

When I apply the patch from @rebasaurus and set DISALLOW_FILE_MODS to true it will show themes that are broken, but you can not take action on it. The reason for this behavior is that DISALLOW_FILE_MODS also modifies the edit_themes capability among others. So if I understand you @johnbillion correct, you want to break this dependency?

To me as an inexperienced wp contributer it make sense that there is a connection between setting the environment variable and the change in the users capabilities.

@rebasaurus
7 months ago

Since edit_themes cap is changed by the constant DISALLOW_FILE_MODS, we'll use edit_theme_options to remove the dependency as mentioned by @sorenbronsted

#5 @rebasaurus
7 months ago

  • Keywords has-patch added; needs-patch removed

#6 @sorenbronsted
7 months ago

Thanks @rebasaurus and it works, but for me there is a catch-22 to this bug report.

I presume that I can use DISALLOW_FILE_MODS constant to prevent users makes changes to a site.

If the constant DISALLOW_FILE_MODS is true then the first patch by @rebasaurus actually solves this, by showing to the user that there are themes which are broken, but the user can't do anything about it, because the user is not allowed to modify files. For me this is by design.

By allowing to modify files/themes even if DISALLOW_FILE_MODS is true defeats the purpose of this constant.

#7 @johnbillion
7 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.5

Thanks for the patches!

It looks like I was mistaken in my original report, and the visibility of the buttons is already handled. I think the first patch49268.diff is the only change that's needed.

#8 @sorenbronsted
6 months ago

@johnbillion Thanks for clarifying. I will apply the first patch and write a unittest for it.

This ticket was mentioned in PR #193 on WordPress/wordpress-develop by sorenbronsted.


6 months ago

This ticket was mentioned in PR #194 on WordPress/wordpress-develop by sorenbronsted.


6 months ago

#11 @johnbillion
6 months ago

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

In 47510:

Themes: Always list broken themes even if the user cannot take a corresponding action within the admin area.

This prevents broken themes being hidden when the DISALLOW_FILE_MODS constant is in use.

Props rebasaurus

Fixes #49268

Note: See TracTickets for help on using tickets.