Skip to content
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

Boost: Make sure source provider URLs are unique #40701

Open
wants to merge 8 commits into
base: trunk
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,15 @@ export function getPrimaryErrorSet( cssState: CriticalCssState ): ErrorSet | und
return undefined;
}

const primaryProviders = [ 'core_front_page', 'core_posts_page' ];
const isImportantProvider = ( provider: Provider ) => {
return provider.key.includes( 'cornerstone' ) || provider.key === 'core_front_page';
};

for ( const key of primaryProviders ) {
const provider = providersWithErrors.find( p => p.key === key );
if ( provider && provider.errors ) {
return getPrimaryGroupedError( provider.errors );
for ( const provider of providersWithErrors ) {
if ( isImportantProvider( provider ) ) {
if ( provider.errors ) {
return getPrimaryGroupedError( provider.errors );
}
}
}

Expand Down
33 changes: 25 additions & 8 deletions projects/plugins/boost/app/lib/Cornerstone_Pages.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Cornerstone_Pages implements Has_Setup {
public function setup() {
$this->register_ds_stores();

add_filter( 'jetpack_boost_critical_css_providers', array( $this, 'remove_ccss_front_page_provider' ), 10, 2 );
add_filter( 'jetpack_boost_critical_css_providers', array( $this, 'remove_core_posts_page' ), 10, 2 );
add_filter( 'display_post_states', array( $this, 'add_display_post_states' ), 10, 2 );
add_action( 'init', array( $this, 'set_default_pages' ), 0 );
}
Expand All @@ -32,16 +32,32 @@ private function register_ds_stores() {
jetpack_boost_register_readonly_option( 'cornerstone_pages_properties', array( $this, 'get_properties' ) );
}

public function remove_ccss_front_page_provider( $providers ) {
/**
* Remove the core posts page provider from the list of
* Cornerstone Pages only if there's no static front page,
* but there's a posts page set.
*
* @param array $providers The list of providers.
*
* @return array
*/
public function remove_core_posts_page( $providers ) {
$filtered_providers = array();

foreach ( $providers as $provider ) {
if ( $provider['key'] !== 'core_front_page' ) {
$filtered_providers[] = $provider;
$front_page = (int) get_option( 'page_on_front' );
$posts_page = (int) get_option( 'page_for_posts' );

// Only filter out core_posts_page if there's no static front page but there is a posts page
if ( ! $front_page && $posts_page ) {
foreach ( $providers as $provider ) {
if ( $provider['key'] !== 'core_posts_page' ) {
$filtered_providers[] = $provider;
}
}

return $filtered_providers;
}

return $filtered_providers;
return $providers;
}

private function default_pages() {
Expand All @@ -55,8 +71,9 @@ private function default_pages() {

$homepage = array( '' );

$urls = array_unique( array_merge( $homepage, $woocommerce_pages, $yoast_cornerstone_pages ) );
$urls = array_merge( $homepage, $woocommerce_pages, $yoast_cornerstone_pages );
$urls = array_map( 'untrailingslashit', $urls );
$urls = array_unique( $urls );

return array_slice( $urls, 0, $max_pages );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,24 +114,51 @@ public function get_current_critical_css_key() {
return $this->current_critical_css_key;
}

/**
* Get a flat array of URLs for a provider.
*
* @param string $provider The provider class name.
* @param array $context_posts The context posts.
*
* @return array
*/
public function get_provider_urls_flat( $provider, $context_posts = array() ) {
$urls = $provider::get_critical_source_urls( $context_posts );
$flat_urls = array();
foreach ( $urls as $url ) {
$flat_urls = array_merge( $flat_urls, $url );
}
return $flat_urls;
}

/**
* Get the URLs of the important providers.
* Important providers are the ones Boost checks against
* if critical css generation fails
*
* @param array $context_posts The context posts.
*
* @return array
*/
public function get_important_provider_urls( $context_posts = array() ) {
$important_provider_urls = array();

$wp_core_provider_urls = $this->get_provider_urls_flat( WP_Core_Provider::class, $context_posts );
$cornerstone_provider_urls = $this->get_provider_urls_flat( Cornerstone_Provider::class, $context_posts );
$important_provider_urls = array_merge( $wp_core_provider_urls, $cornerstone_provider_urls );

return array_values( array_unique( $important_provider_urls ) );
}

/**
* Get providers sources.
*
* @return array
*/
public function get_provider_sources( $context_posts = array() ) {
$sources = array();
$flat_core_and_cornerstone_urls = array();

$wp_core_provider_urls = WP_Core_Provider::get_critical_source_urls( $context_posts );
foreach ( $wp_core_provider_urls as $urls ) {
$flat_core_and_cornerstone_urls = array_merge( $flat_core_and_cornerstone_urls, $urls );
}
$cornerstone_provider_urls = Cornerstone_Provider::get_critical_source_urls( $context_posts );
foreach ( $cornerstone_provider_urls as $urls ) {
$flat_core_and_cornerstone_urls = array_merge( $flat_core_and_cornerstone_urls, $urls );
}
$flat_core_and_cornerstone_urls = array_values( array_unique( $flat_core_and_cornerstone_urls ) );
$sources = array();
$important_provider_urls = $this->get_important_provider_urls( $context_posts );
$cornerstone_provider_urls = $this->get_provider_urls_flat( Cornerstone_Provider::class, $context_posts );

foreach ( $this->get_providers() as $provider ) {
$provider_name = $provider::get_provider_name();
Expand All @@ -146,7 +173,12 @@ public function get_provider_sources( $context_posts = array() ) {
// This removes core and cornerstone URLs from the list of URLs,
// so they don't belong to two separate groups.
if ( ! in_array( $provider, array( WP_Core_Provider::class, Cornerstone_Provider::class ), true ) ) {
$urls = array_values( array_diff( $urls, $flat_core_and_cornerstone_urls ) );
$urls = array_values( array_diff( $urls, $important_provider_urls ) );
}

// Remove WP Core URLs if they are in the Cornerstone URLs list.
if ( WP_Core_Provider::class === $provider ) {
$urls = array_values( array_diff( $urls, $cornerstone_provider_urls ) );
}

if ( empty( $urls ) ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ public static function get_critical_source_urls( $context_posts = array() ) {
$front_page = (int) get_option( 'page_on_front' );
$posts_page = (int) get_option( 'page_for_posts' );

if ( ! empty( $front_page ) && empty( $context_posts ) ) {
$permalink = get_permalink( $front_page );
if ( ! empty( $permalink ) ) {
$urls['front_page'] = array( $permalink );
}
}

$context_post_types = wp_list_pluck( $context_posts, 'post_type' );
$context_post_ids = wp_list_pluck( $context_posts, 'ID' );

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Critical CSS: Make sure URLs used for generation are unique.
Loading