Skip to content

Conversation

@westonruter
Copy link
Contributor

@westonruter westonruter commented Jul 17, 2025

This is a follow-up to #37 and its PR #40.

I'm working on WordPress core ticket #63636 to:

Enable instant page navigations from browser history via bfcache when sending "nocache" headers

In order to take advantage of bfcache, the no-store directive needs to be omitted from the Cache-Control header. In order to prevent proxies from caching authenticated responses, the private directive should be used instead, which WordPress has done since 6.3 via r55968. In this ticket I'm working on, I'm proposing that no-store should be removed while leaving private so that bfcache won't be blocked in the browser. I've also developed a No-cache BFCache feature plugin as a way to test compatibility of this change with the WordPress ecosystem. This has resulted in changes to WooCommerce (woocommerce/woocommerce#58445) and Jetpack (Automattic/jetpack#44322), and now for Pantheon Page Cache.

The immediate workaround for this is to use plugin code like this:

add_filter(
	'pantheon_skip_cache_control',
	static function (): bool {
		return is_admin() || is_user_logged_in();
	}
);

But having to override this is not ideal, naturally.

Even so, I was looking at \Pantheon_Cache::filter_rest_post_dispatch_send_cache_control() and I found it was not also applying this filter, so this PR introduces a helper method to apply the filter so that REST API responses will respect pantheon_skip_cache_control.

In order to better facilitate filtering and compatibility with other plugins, this PR replaces the send_header action with filtering wp_headers. Nevertheless, unless rest_send_nocache_headers in core is filtered to be false, whatever Cache-Control header set by Pantheon ends up getting overridden by WP_REST_Server::serve_request() anyway, where it sends the headers returned by wp_get_nocache_headers(). So this PR does the underlying removal of max-age=0 by filtering nocache_headers where this directive is initially introduced. Similarly, instead of hard-coding the Cache-Control directives to use, this PR uses the value of wp_get_nocache_headers()['Cache-Control'] from which max-age=0 is stripped out.

@westonruter
Copy link
Contributor Author

Check failures do not appear to be valid.

@jazzsequence
Copy link
Contributor

Pretty sure the failing tests are because the forked PR lacks the right permissions in GitHub Actions.

Copy link
Contributor

@jazzsequence jazzsequence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally 👍 on this PR.

I'd like to see some unit tests added for the changes (but notably, none of our current tests are breaking which is a good sign).

My only hesitation is how much breaking PHP 5.x support matters. With the way WordPress and mu-plugin updates on Pantheon work, the workaround for a 5.x WP site would be to use an old version of WordPress, although notably, WordPress dropped support of < 7.0 in 6.5, so maybe this is a non-issue.

I'm good to merge beyond that.

@scottbuscemi
Copy link

Thanks for the PR Weston. This is awesome.

It's ok to target PHP 7.0+ only. Let's update the readme to notate the requirement, and we'll include this in the next release notes for the mu-plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants