Repository navigation
Customize: Keep stored custom CSS post ID when the lookup fails. #14225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2018,9 +2018,12 @@ function wp_get_custom_css_post( $stylesheet = '' ) { | |
| $post = $query->post; | ||
| /* | ||
| * Cache the lookup. See wp_update_custom_css_post(). | ||
| * A stored post ID is kept when the lookup fails, as the post is expected to exist. | ||
| * @todo This should get cleared if a custom_css post is added/removed. | ||
| */ | ||
| set_theme_mod( 'custom_css_post_id', $post ? $post->ID : -1 ); | ||
| if ( $post || empty( $post_id ) ) { | ||
| set_theme_mod( 'custom_css_post_id', $post ? $post->ID : -1 ); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Would checking if ( ! $post && $wpdb->last_error ) {
// The lookup failed; keep the stored ID rather than caching "no post".
} else {
set_theme_mod( 'custom_css_post_id', $post ? $post->ID : -1 );
}There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to add some real-world context to this trade-off. I've encountered orphaned custom CSS posts dozens of times over the years across both my own and clients' WordPress installations. I can't 100% say every instance was caused by this particular bug, but it's certainly a recurring problem. On the other hand, I've never encountered anyone manually deleting a custom CSS post directly from the database. Doing so bypasses WordPress's normal housekeeping anyway, so some resulting inconsistencies are to be expected. Personally, I'd favour keeping the fix simple rather than introducing additional complexity to accommodate an edge case that, in my experience at least, is considerably less likely than the problem we're trying to solve.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the manually deleted case, I agree with @webd-uk. Deleting the custom_css post directly in the database skips WordPress's own housekeeping, so it's already "doing it wrong". Once the CSS is saved again it fixes itself, until then the cost is a few extra queries per request. I think that's a better trade-off than a site losing its CSS completely. I originally thought about checking
|
||
| } | ||
| } | ||
| } else { | ||
| $query = new WP_Query( $custom_css_query_vars ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should fix: This changes behaviour but has no test yet, as @webd-uk also asked. A DB failure is easy to simulate with the
queryfilter. The test below fails on trunk (Failed asserting that -1 is identical to <ID>) and passes with this PR. I ran it locally, so it could go intotests/phpunit/tests/customize/custom-css-setting.phpor a newtests/phpunit/tests/theme/wpGetCustomCssPost.php:A second test for a site with no custom CSS post caching
-1would cover the other branch.