core(pwa): revamp and move short_name_length audit

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

@@ -5,18 +5,17 @@
  */
 'use strict';
 
-const Audit = require('./audit');
+const MultiCheckAudit = require('./multi-check-audit');
 
-class ManifestShortNameLength extends Audit {
+class ManifestShortNameLength extends MultiCheckAudit {
   /**
    * @return {LH.Audit.Meta}
    */
   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).',
@@ -26,28 +25,37 @@ class ManifestShortNameLength extends Audit {
 
   /**
    * @param {LH.Artifacts} artifacts
-   * @return {Promise<LH.Audit.Product>}
+   * @return {Promise<{failures: Array<string>, manifestValues: LH.Artifacts.ManifestValues, notApplicable?: boolean}>}
    */
-  static audit(artifacts) {
+  static audit_(artifacts) {
+    /** @type {Array<string>} */
+    const failures = [];
+    /** @type {Array<string>} */
+    const warnings = [];
+
     return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {
+      const result = {warnings, failures, manifestValues};
+
+      // If there's no valid manifest, this audit is not applicable
       if (manifestValues.isParseFailure) {
-        return {
-          rawValue: false,
-        };
+        result.notApplicable = true;
+        return result;
+      }
+
+      const shortNameCheck = manifestValues.allChecks.find(i => i.id === 'hasShortName');
+      const shortNameLengthCheck = manifestValues.allChecks.find(i => i.id === 'shortNameLength');
+
+      // If there's no short_name present, this audit is not applicable
+      if (shortNameCheck && !shortNameCheck.passing) {
+        result.notApplicable = true;
+        return result;
       }
 
-      const hasShortName = manifestValues.allChecks.find(i => i.id === 'hasShortName');
-      if (!hasShortName || !hasShortName.passing) {
-        return {
-          rawValue: false,
-          explanation: 'No short_name found in manifest.',
-        };
+      if (shortNameLengthCheck && !shortNameLengthCheck.passing) {
+        failures.push(shortNameLengthCheck.failureText);
       }
 
-      const isShortEnough = manifestValues.allChecks.find(i => i.id === 'shortNameLength');
-      return {
-        rawValue: !!isShortEnough && isShortEnough.passing,
-      };
+      return result;
     });
   }
 }

lighthouse-core/audits/multi-check-audit.js

@@ -21,7 +21,7 @@ class MultiCheckAudit extends Audit {
   }
 
   /**
-   * @param {{failures: Array<string>, warnings?: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}} result
+   * @param {{failures: Array<string>, warnings?: Array<string>, manifestValues?: LH.Artifacts.ManifestValues, notApplicable?: boolean}} result
    * @return {LH.Audit.Product}
    */
   static createAuditProduct(result) {
@@ -42,6 +42,13 @@ class MultiCheckAudit extends Audit {
 
     const details = {items: [detailsItem]};
 
+    if (result.notApplicable) {
+      return {
+        rawValue: true,
+        notApplicable: true,
+      };
+    }
+
     // If we fail, share the failures
     if (result.failures.length > 0) {
       return {

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

@@ -30,29 +30,30 @@ describe('Manifest: short_name_length audit', () => {
     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', () => {
     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('fails 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,6 +67,8 @@ 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.ok(!result.explanation.includes('Manifest does not have `short_name`'));
     });
   });
 
@@ -77,6 +80,7 @@ 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, true);
+      assert.equal(result.explanation, 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",
@@ -3016,6 +3016,10 @@
           "id": "content-width",
           "weight": 1
         },
+        {
+          "id": "manifest-short-name-length",
+          "weight": 0
+        },
         {
           "id": "pwa-cross-browser",
           "weight": 0
@@ -3303,10 +3307,6 @@
           "id": "deprecations",
           "weight": 1
         },
-        {
-          "id": "manifest-short-name-length",
-          "weight": 1
-        },
         {
           "id": "password-inputs-can-be-pasted-into",
           "weight": 1