Skip to content

Fix incorrect usage of badge colors in all Site Health checks #472

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 14, 2022
Merged

Fix incorrect usage of badge colors in all Site Health checks #472

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84586cc
Fix incorrect usage of badge colors in all Site Health checks.
felixarntz Aug 12, 2022
22c8407
Replace unnecessary usage of esc_html__.
felixarntz Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix incorrect usage of badge colors in all Site Health checks.
  • Loading branch information
@felixarntz
felixarntz committed Aug 12, 2022
commit 84586cc58bd8a9a6fc8cbd9869da2f054c67edf9
7 changes: 3 additions & 4 deletions modules/object-cache/persistent-object-cache-health-check/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@ function perflab_oc_health_persistent_object_cache() {
*/
$notes = apply_filters( 'perflab_oc_site_status_persistent_object_cache_notes', $notes, $available_services );

$result['status'] = 'recommended';
$result['label'] = __( 'You should use a persistent object cache', 'performance-lab' );
$result['badge']['color'] = 'orange';
$result['description'] .= sprintf(
$result['status'] = 'recommended';
$result['label'] = __( 'You should use a persistent object cache', 'performance-lab' );
$result['description'] .= sprintf(
'<p>%s</p>',
wp_kses(
$notes,
Expand Down
9 changes: 4 additions & 5 deletions modules/site-health/audit-autoloaded-options/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function perflab_aao_autoloaded_options_test() {
'label' => esc_html__( 'Autoloaded options are acceptable', 'performance-lab' ),
Copy link
Member

@adamsilverstein adamsilverstein Aug 12, 2022

Choose a reason for hiding this comment

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

I noticed we are applying esc_html__ inconsistently to translations and that you removed a bunch in this PR.

For core, we don't escape translations, however for plugins I generally recommend escaping (as 10up recommends).

Maybe rather than changing here we can open a follow issue to use a uniform approach and change throughout the plugin?

Copy link
Member Author

@felixarntz felixarntz Aug 14, 2022

Choose a reason for hiding this comment

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

While escaping is recommended for plugins more than core, the rules also say that escaping should happen at the output level. In all the places here though, we're only writing data into an array, so per those guidelines escaping here would not follow the best practice.

All escaping for Site Health check content should happen in WordPress core per that definition, since that's where the data is being output.

'status' => 'good',
'badge' => array(
'label' => esc_html__( 'Performance', 'performance-lab' ),
'label' => __( 'Performance', 'performance-lab' ),
'color' => 'blue',
),
'description' => sprintf(
Expand All @@ -69,10 +69,9 @@ function perflab_aao_autoloaded_options_test() {
return $result;
}

$result['status'] = 'critical';
$result['badge']['color'] = 'red';
$result['label'] = esc_html__( 'Autoloaded options could affect performance', 'performance-lab' );
$result['description'] = sprintf(
$result['status'] = 'critical';
$result['label'] = esc_html__( 'Autoloaded options could affect performance', 'performance-lab' );
$result['description'] = sprintf(
/* translators: 1. Number of autoloaded options. 2. Autoloaded options size. */
'<p>' . esc_html( $base_description ) . ' ' . esc_html__( 'Your site has %1$s autoloaded options (size: %2$s) in the options table, which could cause your site to be slow. You can reduce the number of autoloaded options by cleaning up your site\'s options table.', 'performance-lab' ) . '</p>',
$autoloaded_options_count,
Expand Down
10 changes: 4 additions & 6 deletions modules/site-health/audit-enqueued-assets/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ function perflab_aea_enqueued_js_assets_test() {
'label' => esc_html__( 'Enqueued scripts', 'performance-lab' ),
'status' => 'good',
'badge' => array(
'label' => esc_html__( 'Performance', 'performance-lab' ),
'label' => __( 'Performance', 'performance-lab' ),
'color' => 'blue',
),
'description' => sprintf(
Expand Down Expand Up @@ -186,8 +186,7 @@ function perflab_aea_enqueued_js_assets_test() {
$scripts_size_treshold = apply_filters( 'perflab_aea_enqueued_scripts_byte_size_threshold', 300000 );

if ( $enqueued_scripts > $scripts_treshold || perflab_aea_get_total_size_bytes_enqueued_scripts() > $scripts_size_treshold ) {
$result['status'] = 'recommended';
$result['badge']['color'] = 'orange';
$result['status'] = 'recommended';

$result['description'] = sprintf(
'<p>%s</p>',
Expand Down Expand Up @@ -238,7 +237,7 @@ function perflab_aea_enqueued_css_assets_test() {
'label' => esc_html__( 'Enqueued styles', 'performance-lab' ),
'status' => 'good',
'badge' => array(
'label' => esc_html__( 'Performance', 'performance-lab' ),
'label' => __( 'Performance', 'performance-lab' ),
'color' => 'blue',
),
'description' => sprintf(
Expand Down Expand Up @@ -279,8 +278,7 @@ function perflab_aea_enqueued_css_assets_test() {
*/
$styles_size_threshold = apply_filters( 'perflab_aea_enqueued_styles_byte_size_threshold', 100000 );
if ( $enqueued_styles > $styles_threshold || perflab_aea_get_total_size_bytes_enqueued_styles() > $styles_size_threshold ) {
$result['status'] = 'recommended';
$result['badge']['color'] = 'orange';
$result['status'] = 'recommended';

$result['description'] = sprintf(
'<p>%s</p>',
Expand Down
16 changes: 6 additions & 10 deletions modules/site-health/audit-full-page-cache/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function perflab_afpc_page_cache_test() {
$result = array(
'badge' => array(
'label' => __( 'Performance', 'performance-lab' ),
'color' => 'green',
'color' => 'blue',
),
'description' => wp_kses_post( $description ),
'test' => 'perflab_page_cache',
Expand All @@ -65,10 +65,9 @@ function perflab_afpc_page_cache_test() {
$page_cache_detail = perflab_afpc_get_page_cache_detail();

if ( is_wp_error( $page_cache_detail ) ) {
$result['badge']['color'] = 'orange';
$result['label'] = __( 'Unable to detect the presence of page caching', 'performance-lab' );
$result['status'] = 'recommended';
$error_info = sprintf(
$result['label'] = __( 'Unable to detect the presence of page caching', 'performance-lab' );
$result['status'] = 'recommended';
$error_info = sprintf(
/* translators: 1 is error message, 2 is error code */
__( 'Unable to detect page caching due to possible loopback request problem. Please verify that the loopback request test is passing. Error: %1$s (Code: %2$s)', 'performance-lab' ),
$page_cache_detail->get_error_message(),
Expand All @@ -82,15 +81,12 @@ function perflab_afpc_page_cache_test() {

switch ( $page_cache_detail['status'] ) {
case 'recommended':
$result['badge']['color'] = 'orange';
$result['label'] = __( 'Page caching is not detected but the server response time is OK', 'performance-lab' );
$result['label'] = __( 'Page caching is not detected but the server response time is OK', 'performance-lab' );
break;
case 'good':
$result['badge']['color'] = 'green';
$result['label'] = __( 'Page caching is detected and the server response time is good', 'performance-lab' );
$result['label'] = __( 'Page caching is detected and the server response time is good', 'performance-lab' );
break;
default:
$result['badge']['color'] = 'red';
if ( empty( $page_cache_detail['headers'] ) && ! $page_cache_detail['advanced_cache_present'] ) {
$result['label'] = __( 'Page caching is not detected and the server response time is slow', 'performance-lab' );
} else {
Expand Down
8 changes: 1 addition & 7 deletions tests/modules/site-health/audit-full-page-cache/audit-full-page-cache-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,10 @@ public function test_perflab_afpc_add_full_page_cache_test() {
* @covers ::perflab_afpc_check_for_page_caching()
*/
public function test_perflab_afpc_page_cache_test( $responses, $expected_status, $expected_label, $good_basic_auth = null, $delay_the_response = false ) {
$badge_color = array(
'critical' => 'red',
'recommended' => 'orange',
'good' => 'green',
);

$expected_props = array(
'badge' => array(
'label' => __( 'Performance', 'performance-lab' ),
'color' => $badge_color[ $expected_status ],
'color' => 'blue',
),
'test' => 'perflab_page_cache',
'status' => $expected_status,
Expand Down
14 changes: 6 additions & 8 deletions ...s/testdata/modules/site-health/audit-enqueued-assets/class-site-health-mock-responses.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,9 @@ public static function return_aea_enqueued_js_assets_test_callback_less_than_thr
* @return array
*/
public static function return_aea_enqueued_js_assets_test_callback_more_than_threshold( $enqueued_scripts ) {
$result = self::return_aea_enqueued_js_assets_test_callback_less_than_threshold();
$result['status'] = 'recommended';
$result['badge']['color'] = 'orange';
$result['description'] = sprintf(
$result = self::return_aea_enqueued_js_assets_test_callback_less_than_threshold();
$result['status'] = 'recommended';
$result['description'] = sprintf(
'<p>%s</p>',
esc_html(
sprintf(
Expand Down Expand Up @@ -149,10 +148,9 @@ public static function return_aea_enqueued_css_assets_test_callback_less_than_th
* @return array
*/
public static function return_aea_enqueued_css_assets_test_callback_more_than_threshold( $enqueued_styles ) {
$result = self::return_aea_enqueued_css_assets_test_callback_less_than_threshold();
$result['status'] = 'recommended';
$result['badge']['color'] = 'orange';
$result['description'] = sprintf(
$result = self::return_aea_enqueued_css_assets_test_callback_less_than_threshold();
$result['status'] = 'recommended';
$result['description'] = sprintf(
'<p>%s</p>',
esc_html(
sprintf(
Expand Down