core(pwa): revamp and move short_name_length audit (#4860)

lighthouse-core/audits/manifest-short-name-length.js

@@ -14,9 +14,8 @@ class ManifestShortNameLength extends Audit {
   static get meta() {
     return {
       id: 'manifest-short-name-length',
-      title: 'Manifest\'s `short_name` won\'t be truncated when displayed on homescreen',
-      failureTitle: 'Manifest\'s `short_name` will be truncated when displayed ' +
-          'on homescreen',
+      title: 'The `short_name` won\'t be truncated on the homescreen',
+      failureTitle: 'The `short_name` will be truncated on the homescreen',
       description: 'Make your app\'s `short_name` fewer than 12 characters to ' +
           'ensure that it\'s not truncated on homescreens. [Learn ' +
           'more](https://developers.google.com/web/tools/lighthouse/audits/manifest-short_name-is-not-truncated).',
@@ -28,27 +27,37 @@ class ManifestShortNameLength extends Audit {
    * @param {LH.Artifacts} artifacts
    * @return {Promise<LH.Audit.Product>}
    */
-  static audit(artifacts) {
-    return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {
-      if (manifestValues.isParseFailure) {
-        return {
-          rawValue: false,
-        };
-      }
+  static async audit(artifacts) {
+    const manifestValues = await artifacts.requestManifestValues(artifacts.Manifest);
+    // If there's no valid manifest, this audit is not applicable
+    if (manifestValues.isParseFailure) {
+      return {
+        rawValue: true,
+        notApplicable: true,
+      };
+    }
 
-      const hasShortName = manifestValues.allChecks.find(i => i.id === 'hasShortName');
-      if (!hasShortName || !hasShortName.passing) {
-        return {
-          rawValue: false,
-          explanation: 'No short_name found in manifest.',
-        };
-      }
+    const shortNameCheck = manifestValues.allChecks.find(i => i.id === 'hasShortName');
+    const shortNameLengthCheck = manifestValues.allChecks.find(i => i.id === 'shortNameLength');
 
-      const isShortEnough = manifestValues.allChecks.find(i => i.id === 'shortNameLength');
+    // If there's no short_name present, this audit is not applicable
+    if (shortNameCheck && !shortNameCheck.passing) {
+      return {
+        rawValue: true,
+        notApplicable: true,
+      };
+    }
+    // Shortname is present, but it's too long
+    if (shortNameLengthCheck && !shortNameLengthCheck.passing) {
       return {
-        rawValue: !!isShortEnough && isShortEnough.passing,
+        rawValue: false,
+        explanation: `Failure: ${shortNameLengthCheck.failureText}.`,
       };
-    });
+    }
+    // Has a shortname that's under the threshold
+    return {
+      rawValue: true,
+    };
   }
 }
 

lighthouse-core/audits/webapp-install-banner.js

@@ -57,6 +57,12 @@ class WebappInstallBanner extends MultiCheckAudit {
     const failures = [];
     const bannerCheckIds = [
       'hasName',
+      // Technically shortname isn't required (if name is defined):
+      //   https://cs.chromium.org/chromium/src/chrome/browser/installable/installable_manager.cc?type=cs&q=IsManifestValidForWebApp+f:cc+-f:test&sq=package:chromium&l=473
+      // Despite this, we think it's better to require it anyway.
+      // short_name is preferred for the homescreen icon, but a longer name can be used in
+      // the splash screen and app title. Given the different usecases, we'd like to make it clearer
+      // that the developer has two possible strings to work with.
       'hasShortName',
       'hasStartUrl',
       'hasPWADisplayValue',

lighthouse-core/config/default-config.js

@@ -302,6 +302,7 @@ module.exports = {
         {id: 'splash-screen', weight: 1},
         {id: 'themed-omnibox', weight: 1},
         {id: 'content-width', weight: 1},
+        {id: 'manifest-short-name-length', weight: 0},
         // Manual audits
         {id: 'pwa-cross-browser', weight: 0},
         {id: 'pwa-page-transitions', weight: 0},
@@ -375,7 +376,6 @@ module.exports = {
         {id: 'no-vulnerable-libraries', weight: 1},
         {id: 'notification-on-start', weight: 1},
         {id: 'deprecations', weight: 1},
-        {id: 'manifest-short-name-length', weight: 1},
         {id: 'password-inputs-can-be-pasted-into', weight: 1},
         {id: 'errors-in-console', weight: 1},
         {id: 'image-aspect-ratio', weight: 1},

lighthouse-core/gather/computed/manifest-values.js

@@ -11,7 +11,7 @@ const icons = require('../../lib/icons');
 const PWA_DISPLAY_VALUES = ['minimal-ui', 'fullscreen', 'standalone'];
 
 // Historically, Chrome recommended 12 chars as the maximum short_name length to prevent truncation.
-// See #69 for more discussion & https://developer.chrome.com/apps/manifest/name#short_name
+// For more discussion, see https://github.com/GoogleChrome/lighthouse/issues/69 and https://developer.chrome.com/apps/manifest/name#short_name
 const SUGGESTED_SHORTNAME_LENGTH = 12;
 
 class ManifestValues extends ComputedArtifact {
@@ -69,7 +69,9 @@ class ManifestValues extends ComputedArtifact {
       },
       {
         id: 'shortNameLength',
-        failureText: 'Manifest `short_name` will be truncated when displayed on the homescreen',
+        failureText: `Manifest's \`short_name\` is too long (>${SUGGESTED_SHORTNAME_LENGTH} ` +
+          `characters) to be displayed on a homescreen without truncation`,
+        // Pass if there's no short_name. Don't want to report a non-existent string is too long
         validate: manifestValue => !!manifestValue.short_name.value &&
             manifestValue.short_name.value.length <= SUGGESTED_SHORTNAME_LENGTH,
       },

lighthouse-core/test/audits/manifest-short-name-length-test.js

@@ -25,34 +25,35 @@ function generateMockArtifacts() {
 /* eslint-env mocha */
 
 describe('Manifest: short_name_length audit', () => {
-  it('fails with no explanation if page had no manifest', () => {
+  it('marked as notApplicable if page had no manifest', () => {
     const artifacts = generateMockArtifacts();
     artifacts.Manifest = null;
 
     return ManifestShortNameLengthAudit.audit(artifacts).then(result => {
-      assert.strictEqual(result.rawValue, false);
-      assert.strictEqual(result.explanation, undefined);
+      assert.strictEqual(result.rawValue, true);
+      assert.strictEqual(result.notApplicable, true);
     });
   });
 
-  it('fails when an empty manifest is present', () => {
+  it('marked as notApplicable if manifest is present but empty', () => {
     const artifacts = generateMockArtifacts();
     artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
     return ManifestShortNameLengthAudit.audit(artifacts).then(result => {
-      assert.equal(result.rawValue, false);
-      assert.equal(result.explanation, 'No short_name found in manifest.');
+      assert.strictEqual(result.rawValue, true);
+      assert.strictEqual(result.notApplicable, true);
     });
   });
 
-  it('fails when a manifest contains no short_name and too long name', () => {
+  it('marked as notApplicable when a manifest contains no short_name', () => {
     const artifacts = generateMockArtifacts();
     const manifestSrc = JSON.stringify({
       name: 'i\'m much longer than the recommended size',
     });
     artifacts.Manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
     return ManifestShortNameLengthAudit.audit(artifacts).then(result => {
-      assert.equal(result.rawValue, false);
-      assert.notEqual(result.explanation, undefined);
+      assert.strictEqual(result.rawValue, true);
+      assert.strictEqual(result.notApplicable, true);
+      assert.equal(result.explanation, undefined);
     });
   });
 
@@ -66,17 +67,21 @@ describe('Manifest: short_name_length audit', () => {
     artifacts.Manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
     return ManifestShortNameLengthAudit.audit(artifacts).then(result => {
       assert.equal(result.rawValue, false);
+      assert.ok(result.explanation.includes('without truncation'), result.explanation);
+      assert.equal(result.notApplicable, undefined);
     });
   });
 
-  it('succeeds when a manifest contains a short_name', () => {
+  it('passes when a manifest contains a short_name', () => {
     const artifacts = generateMockArtifacts();
     const manifestSrc = JSON.stringify({
       short_name: 'Lighthouse',
     });
     artifacts.Manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
     return ManifestShortNameLengthAudit.audit(artifacts).then(result => {
       assert.equal(result.rawValue, true);
+      assert.equal(result.explanation, undefined);
+      assert.equal(result.notApplicable, undefined);
     });
   });
   /* eslint-enable camelcase */

lighthouse-core/test/results/sample_v2.json

@@ -543,11 +543,11 @@
     },
     "manifest-short-name-length": {
       "id": "manifest-short-name-length",
-      "title": "Manifest's `short_name` will be truncated when displayed on homescreen",
+      "title": "The `short_name` won't be truncated on the homescreen",
       "description": "Make your app's `short_name` fewer than 12 characters to ensure that it's not truncated on homescreens. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/manifest-short_name-is-not-truncated).",
-      "score": 0,
-      "scoreDisplayMode": "binary",
-      "rawValue": false
+      "score": null,
+      "scoreDisplayMode": "not-applicable",
+      "rawValue": true
     },
     "content-width": {
       "id": "content-width",
@@ -2982,6 +2982,10 @@
           "id": "content-width",
           "weight": 1
         },
+        {
+          "id": "manifest-short-name-length",
+          "weight": 0
+        },
         {
           "id": "pwa-cross-browser",
           "weight": 0
@@ -3269,10 +3273,6 @@
           "id": "deprecations",
           "weight": 1
         },
-        {
-          "id": "manifest-short-name-length",
-          "weight": 1
-        },
         {
           "id": "password-inputs-can-be-pasted-into",
           "weight": 1