Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 6 months ago

#49268 closed defect (bug) (fixed)

Broken themes should always be listed

Reported by: johnbillion's profile johnbillion Owned by: johnbillion's profile 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 4 years ago.
Remove capability check on displaying broken themes
49268.patch (913 bytes) - added by rebasaurus 4 years 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
4 years ago

Remove capability check on displaying broken themes

#1 @rebasaurus
4 years ago

  • Keywords has-patch added; needs-patch removed

#2 @johnbillion
4 years 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
4 years ago

Just for the record I will try to fix this.

#4 @sorenbronsted
4 years 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
4 years 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
4 years ago

  • Keywords has-patch added; needs-patch removed

#6 @sorenbronsted
4 years 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
4 years 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
4 years 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.


4 years ago
#9

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


4 years ago
#10

#11 @johnbillion
4 years 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.