Using wp_set_option_autoload() for theme options #5692
Conversation
| @@ -866,6 +866,13 @@ function switch_theme( $stylesheet ) { | |||
|
|
|||
| update_option( 'theme_switched', $old_theme->get_stylesheet() ); | |||
|
|
|||
| // Set autoload=no for the previous theme | |||
| $old_theme_stylesheet = $old_theme->get_stylesheet(); | |||
| wp_set_option_autoload( "theme_mods_$old_theme_stylesheet", 'no' ); | |||
There was a problem hiding this comment.
This adjustment alters the autoload option for the previous theme. If the user had activated multiple themes previously, it becomes essential to verify and modify the autoload value for those themes as well. What do you think?
There was a problem hiding this comment.
@mukeshpanchal27 I think this is only an issue for existing sites that had multiple themes active before updating to WordPress 6.5. So I think this is better addressed separately via your new ticket https://core.trac.wordpress.org/ticket/59975 and #5711.
Any site that starts with WordPress 6.5 should not have this problem as every time the theme is switched the previous theme's theme mods are set to not autoload. So there should at no point be more than one theme with autoloaded theme mods.
There was a problem hiding this comment.
Thank you for the PR @Rajinsharwar, I left a few points of feedback.
Would you also be able to add a unit test for the new functionality in switch_theme()?
| @@ -866,6 +866,13 @@ function switch_theme( $stylesheet ) { | |||
|
|
|||
| update_option( 'theme_switched', $old_theme->get_stylesheet() ); | |||
|
|
|||
| // Set autoload=no for the previous theme | |||
| $old_theme_stylesheet = $old_theme->get_stylesheet(); | |||
| wp_set_option_autoload( "theme_mods_$old_theme_stylesheet", 'no' ); | |||
There was a problem hiding this comment.
@mukeshpanchal27 I think this is only an issue for existing sites that had multiple themes active before updating to WordPress 6.5. So I think this is better addressed separately via your new ticket https://core.trac.wordpress.org/ticket/59975 and #5711.
Any site that starts with WordPress 6.5 should not have this problem as every time the theme is switched the previous theme's theme mods are set to not autoload. So there should at no point be more than one theme with autoloaded theme mods.
| $old_theme_stylesheet = $old_theme->get_stylesheet(); | ||
| wp_set_option_autoload( "theme_mods_$old_theme_stylesheet", 'no' ); |
There was a problem hiding this comment.
We need to also cover a potential parent theme here (i.e. if the old template is different from the old stylesheet). We could then use wp_set_options_autoload() to reduce the database requests needed.
| $old_theme_stylesheet = $old_theme->get_stylesheet(); | |
| wp_set_option_autoload( "theme_mods_$old_theme_stylesheet", 'no' ); | |
| $old_theme_mods = array_unique( | |
| array( | |
| "theme_mods_{$old_theme->get_stylesheet()}", | |
| "theme_mods_{$old_theme->get_template()}" | |
| ) | |
| ); | |
| wp_set_options_autoload( $old_theme_mods, 'no' ); |
| @@ -866,6 +866,13 @@ function switch_theme( $stylesheet ) { | |||
|
|
|||
| update_option( 'theme_switched', $old_theme->get_stylesheet() ); | |||
|
|
|||
| // Set autoload=no for the previous theme | |||
There was a problem hiding this comment.
Missing period after inline comment.
| // Set autoload=no for the previous theme | |
| // Set autoload=no for the previous theme. |
| // Set autoload=yes for the switched theme if not already set | ||
| wp_set_option_autoload( "theme_mods_$stylesheet", 'yes' ); |
There was a problem hiding this comment.
Same here:
| // Set autoload=yes for the switched theme if not already set | |
| wp_set_option_autoload( "theme_mods_$stylesheet", 'yes' ); | |
| // Set autoload=yes for the switched to theme if not already set. | |
| $new_theme_mods = array_unique( | |
| array( | |
| "theme_mods_{$stylesheet}", | |
| "theme_mods_{$template}" | |
| ) | |
| ); | |
| wp_set_options_autoload( $new_theme_mods, 'yes' ); |
|
Closing in favor of #5706. Thanks for working on this! |
Using wp_set_option_autoload() to set no to the old theme's autoload option.
Trac ticket: https://core.trac.wordpress.org/ticket/39537
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.