Site Health: is_writable() check when there are more than 1 theme directories

[pbiron]

While working on the patch to #48199, I discovered the existence of ​register_theme_directory(). Using register_theme_directory() it seems to be possible to have a setup such as:

• WP_CONTENT_DIR/parent-themes/some-parent-theme
• WP_CONTENT_DIR/child-themes/current-theme

where current-theme is a child of some-parent-theme. In such a setup, the current behavior (and that in the patch for #48199) actually checks whether WP_CONTENT_DIR/parent-themes is writable.

2 questions:

1. should the is_writable() actually be checking whether get_theme_root( get_stylesheet() ) is writable?
2. should there be additional checks whether all registered theme directories are writable?

[Clorith]

Hmm, yeah should probably be done. The intention of the check is to see if theme installations/updates can happen, for installations the default location would be used, but updates would use the theme root location, so makes sense to loop over them and check them all.

Great catch!

[Clorith]

Is this something you'd like to work on @pbiron?

I'm thinking we need to actually grab the $wp_theme_directories global variable for this one, and check if it's got more than one entry; loop over them and check if they are writable, but only if they are within ABSPATH to avoid hitting the same kind of scenario as the symlink issue you noted in the previous ticket. (This because it's possible to provide an absolute path, alongside the ability to register a relative path from wp-content)

[pbiron]

sure. But I don't think I'll have the time before 6.1 beta, but certainly for 6.2 (or potentially a 6.1.x).

I made sure all themes in the wp_theme_directories global are checked for wp_is_writable
Trac ticket: https://core.trac.wordpress.org/ticket/50018

Thank you for the patch @thijsoo !

In addition to the suggested variable naming from @costdev, there's a few things we need to keep track of, the current proposed solution would set the value of $is_writable_template_directory to the status of the very last directory being checked, so instead it may be an idea to instead show each available directory, and if they are writable or not, as the value (this can be passed as an array, some pseudo-code for you below to demonstrate)

{{{php
$is_writable_value = [];
$is_writable_debug = [];
foreach ( $theme_directories as $theme_directory ) {

$is_writable_value[] = sprintf(

'%s: %s',
$theme_directory,
( wp_is_writable( $theme_directory ) ? ( 'Writable' ) : ( 'Not writable' ) )

);

$is_writable_debug[] = sprintf(

'%s: %s',
$theme_directory,
( wp_is_writable( $theme_directory ) ? 'writable' : 'not writable' )

);

}
}}}

(Note that it needs two variables, as the debug value is never translated, for consistency between languages in debug data.)

The ​label for "The themes directory" should also be updated to use the ​`_n()` translation function, so that it can account for if there is only one, or multiple theme directories available.

[Clorith]

As beta-1 is planned for tomorrow, and the patch still needs some refinement, I'm moving it out of the milestone for now (but will keep watching it, so feel free to improve on the patch at your own time!)