From 606fc6c9fd83d5b50cef71e25fd7fd754b66cc48 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 16 Aug 2019 14:41:46 -0700 Subject: [PATCH 1/5] Automatically copy name to short_name in web app manifest if <= 12 chars --- tests/test-class-wp-web-app-manifest.php | 10 +++++++++- wp-includes/class-wp-web-app-manifest.php | 13 +++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test-class-wp-web-app-manifest.php b/tests/test-class-wp-web-app-manifest.php index 5d1c7528b..208c0989f 100644 --- a/tests/test-class-wp-web-app-manifest.php +++ b/tests/test-class-wp-web-app-manifest.php @@ -130,7 +130,7 @@ public function test_get_theme_color() { */ public function test_get_manifest() { $this->mock_site_icon(); - $blogname = "PWA's Domain Is Here"; + $blogname = 'PWA Test'; update_option( 'blogname', $blogname ); $actual_manifest = $this->instance->get_manifest(); @@ -139,6 +139,7 @@ public function test_get_manifest() { 'description' => get_bloginfo( 'description' ), 'display' => 'minimal-ui', 'name' => $blogname, + 'short_name' => $blogname, 'lang' => get_bloginfo( 'language' ), 'dir' => is_rtl() ? 'rtl' : 'ltr', 'start_url' => home_url( '/' ), @@ -147,6 +148,13 @@ public function test_get_manifest() { ); $this->assertEquals( $expected_manifest, $actual_manifest ); + // Check that long names do not automatically copy to short name. + $blogname = str_repeat( 'x', 13 ); + update_option( 'blogname', $blogname ); + $actual_manifest = $this->instance->get_manifest(); + $this->assertEquals( $blogname, $actual_manifest['name'] ); + $this->assertArrayNotHasKey( 'short_name', $actual_manifest ); + // Test that the filter at the end of the method overrides the value. add_filter( 'web_app_manifest', array( $this, 'mock_manifest' ) ); $actual_manifest = $this->instance->get_manifest(); diff --git a/wp-includes/class-wp-web-app-manifest.php b/wp-includes/class-wp-web-app-manifest.php index 5c1d71ab8..ffe56d26f 100644 --- a/wp-includes/class-wp-web-app-manifest.php +++ b/wp-includes/class-wp-web-app-manifest.php @@ -123,6 +123,19 @@ public function get_manifest() { 'display' => 'minimal-ui', 'dir' => is_rtl() ? 'rtl' : 'ltr', ); + + /* + * If the name is 12 characters or less, use it as the short_name. Lighthouse complains when the short_name + * is absent, even when the name is 12 characters or less. Chrome's max recommended short_name length is 12 + * characters. + * + * https://developers.google.com/web/tools/lighthouse/audits/manifest-contains-short_name + * https://developer.chrome.com/apps/manifest/name#short_name + */ + if ( strlen( $manifest['name'] ) <= 12 ) { + $manifest['short_name'] = $manifest['name']; + } + $language = get_bloginfo( 'language' ); if ( $language ) { $manifest['lang'] = $language; From ab287006d9b53cb12c158838f27a014efe111f58 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 16 Aug 2019 15:21:02 -0700 Subject: [PATCH 2/5] Add site health check for short_name in web app manifest --- pwa.php | 2 +- wp-includes/class-wp-web-app-manifest.php | 93 ++++++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/pwa.php b/pwa.php index 430f47a56..db976278d 100644 --- a/pwa.php +++ b/pwa.php @@ -118,7 +118,7 @@ function _pwa_add_disabled_navigation_preload_site_status_test( $tests ) { add_filter( 'site_status_tests', '_pwa_add_disabled_navigation_preload_site_status_test' ); /** - * Print admin notice when a build has not been been performed. + * Flag navigation preload incorrectly being disabled. * * This is temporary measure to correct a mistake in the example for how navigation request caching strategies. * diff --git a/wp-includes/class-wp-web-app-manifest.php b/wp-includes/class-wp-web-app-manifest.php index ffe56d26f..fb5efb780 100644 --- a/wp-includes/class-wp-web-app-manifest.php +++ b/wp-includes/class-wp-web-app-manifest.php @@ -33,6 +33,15 @@ class WP_Web_App_Manifest { */ const REST_ROUTE = '/web-app-manifest'; + /** + * Maximum length for short_name. + * + * @link https://developers.google.com/web/tools/lighthouse/audits/manifest-contains-short_name + * @link https://developer.chrome.com/apps/manifest/name#short_name + * @var int + */ + const SHORT_NAME_MAX_LENGTH = 12; + /** * The default manifest icon sizes. * @@ -51,6 +60,7 @@ class WP_Web_App_Manifest { public function init() { add_action( 'wp_head', array( $this, 'manifest_link_and_meta' ) ); add_action( 'rest_api_init', array( $this, 'register_manifest_rest_route' ) ); + add_filter( 'site_status_tests', array( $this, 'add_short_name_site_status_test' ) ); } /** @@ -132,7 +142,7 @@ public function get_manifest() { * https://developers.google.com/web/tools/lighthouse/audits/manifest-contains-short_name * https://developer.chrome.com/apps/manifest/name#short_name */ - if ( strlen( $manifest['name'] ) <= 12 ) { + if ( strlen( $manifest['name'] ) <= self::SHORT_NAME_MAX_LENGTH ) { $manifest['short_name'] = $manifest['name']; } @@ -168,6 +178,87 @@ public function get_manifest() { return apply_filters( 'web_app_manifest', $manifest ); } + /** + * Register test for lacking short_name in web app manifest. + * + * @since 0.3.1 + * + * @param array $tests Tests. + * @return array Tests. + */ + public function add_short_name_site_status_test( $tests ) { + $tests['direct']['web_app_manifest_short_name'] = array( + 'label' => __( 'Short Name in Web App Manifest', 'pwa' ), + 'test' => array( $this, 'test_short_name_present_in_manifest' ), + ); + return $tests; + } + + /** + * Test that web app manifest contains a short_name. + * + * @since 0.3.1 + * @todo Add test for PNG site icon. + * + * @return array Test results. + */ + public function test_short_name_present_in_manifest() { + $manifest = $this->get_manifest(); + + /* translators: %d is the max length as a number */ + $description = sprintf( __( 'The short_name is a short version of your website’s name which is displayed when there is not enough space for the full name, for example with the site icon on a phone’s homescreen. It should be a maximum of %d characters long.', 'pwa' ), self::SHORT_NAME_MAX_LENGTH ); + + $actions = __( 'You currently may use web_app_manifest filter to set the short name, for example in your theme’s functions.php.', 'pwa' ); + + if ( empty( $manifest['short_name'] ) ) { + $result = array( + 'label' => __( 'Web App Manifest lacks a short_name entry', 'pwa' ), + 'status' => 'recommended', + 'badge' => array( + 'label' => __( 'Progressive Web App', 'pwa' ), + 'color' => 'orange', + ), + 'description' => wp_kses_post( sprintf( '

%s

', $description ) ), + 'actions' => wp_kses_post( $actions ), + ); + } elseif ( strlen( $manifest['short_name'] ) > self::SHORT_NAME_MAX_LENGTH ) { + $result = array( + 'label' => + sprintf( + /* translators: %1$s is the short name */ + __( 'Web App Manifest has a short_name (%s) that is too long', 'pwa' ), + esc_html( $manifest['short_name'] ) + ), + 'status' => 'recommended', + 'badge' => array( + 'label' => __( 'Progressive Web App', 'pwa' ), + 'color' => 'orange', + ), + 'description' => wp_kses_post( sprintf( '

%s

', $description ) ), + 'actions' => wp_kses_post( $actions ), + ); + } else { + $result = array( + 'label' => + sprintf( + /* translators: %1$s is the short name */ + __( 'Web App Manifest has a short_name (%s)', 'pwa' ), + esc_html( $manifest['short_name'] ) + ), + 'status' => 'good', + 'badge' => array( + 'label' => __( 'Progressive Web App', 'pwa' ), + 'color' => 'green', + ), + 'description' => wp_kses_post( sprintf( '

%s

', $description ) ), + ); + } + + $result['test'] = 'web_app_manifest_short_name'; + + return $result; + } + /** * Registers the rest route to get the manifest. */ From 5d78752c1c8d9bfec24123b58b1db37645546021 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 16 Aug 2019 15:24:38 -0700 Subject: [PATCH 3/5] Update since tags --- wp-includes/class-wp-web-app-manifest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/wp-includes/class-wp-web-app-manifest.php b/wp-includes/class-wp-web-app-manifest.php index fb5efb780..6426c678d 100644 --- a/wp-includes/class-wp-web-app-manifest.php +++ b/wp-includes/class-wp-web-app-manifest.php @@ -36,6 +36,7 @@ class WP_Web_App_Manifest { /** * Maximum length for short_name. * + * @since 0.4 * @link https://developers.google.com/web/tools/lighthouse/audits/manifest-contains-short_name * @link https://developer.chrome.com/apps/manifest/name#short_name * @var int @@ -181,7 +182,7 @@ public function get_manifest() { /** * Register test for lacking short_name in web app manifest. * - * @since 0.3.1 + * @since 0.4 * * @param array $tests Tests. * @return array Tests. @@ -197,7 +198,7 @@ public function add_short_name_site_status_test( $tests ) { /** * Test that web app manifest contains a short_name. * - * @since 0.3.1 + * @since 0.4 * @todo Add test for PNG site icon. * * @return array Test results. From 0d41abd959e31a6d5cf3938e56db9931d5f05753 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 19 Aug 2019 11:07:13 -0700 Subject: [PATCH 4/5] Set dist to trusty --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 64888e0bd..405f15c00 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,7 @@ language: php +dist: trusty + addons: apt: packages: From 7b40cb5a6f34354cfed4b69dddc6ff37eba27be9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 19 Aug 2019 20:39:57 -0700 Subject: [PATCH 5/5] Use placeholders in translation strings --- wp-includes/class-wp-web-app-manifest.php | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/wp-includes/class-wp-web-app-manifest.php b/wp-includes/class-wp-web-app-manifest.php index 6426c678d..bb3ac62c9 100644 --- a/wp-includes/class-wp-web-app-manifest.php +++ b/wp-includes/class-wp-web-app-manifest.php @@ -206,14 +206,23 @@ public function add_short_name_site_status_test( $tests ) { public function test_short_name_present_in_manifest() { $manifest = $this->get_manifest(); - /* translators: %d is the max length as a number */ - $description = sprintf( __( 'The short_name is a short version of your website’s name which is displayed when there is not enough space for the full name, for example with the site icon on a phone’s homescreen. It should be a maximum of %d characters long.', 'pwa' ), self::SHORT_NAME_MAX_LENGTH ); + $description = sprintf( + /* translators: %1$s is `short_name`, %2$d is the max length as a number */ + __( 'The %1$s is a short version of your website’s name. It is displayed when there is not enough space for the full name, for example with the site icon on a phone’s homescreen. It should be a maximum of %2$d characters long.', 'pwa' ), + 'short_name', + self::SHORT_NAME_MAX_LENGTH + ); - $actions = __( 'You currently may use web_app_manifest filter to set the short name, for example in your theme’s functions.php.', 'pwa' ); + $actions = sprintf( + /* translators: %1$s is `web_app_manifest`, %2$s is `functions.php` */ + __( 'You currently may use %1$s filter to set the short name, for example in your theme’s %2$s.', 'pwa' ), + 'web_app_manifest', + 'functions.php' + ); if ( empty( $manifest['short_name'] ) ) { $result = array( - 'label' => __( 'Web App Manifest lacks a short_name entry', 'pwa' ), + 'label' => __( 'Web App Manifest lacks a short name entry', 'pwa' ), 'status' => 'recommended', 'badge' => array( 'label' => __( 'Progressive Web App', 'pwa' ), @@ -227,7 +236,7 @@ public function test_short_name_present_in_manifest() { 'label' => sprintf( /* translators: %1$s is the short name */ - __( 'Web App Manifest has a short_name (%s) that is too long', 'pwa' ), + __( 'Web App Manifest has a short name (%s) that is too long', 'pwa' ), esc_html( $manifest['short_name'] ) ), 'status' => 'recommended', @@ -243,7 +252,7 @@ public function test_short_name_present_in_manifest() { 'label' => sprintf( /* translators: %1$s is the short name */ - __( 'Web App Manifest has a short_name (%s)', 'pwa' ), + __( 'Web App Manifest has a short name (%s)', 'pwa' ), esc_html( $manifest['short_name'] ) ), 'status' => 'good',