core(icons): Add PNG check to icon validation

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

@@ -37,13 +37,13 @@ class ManifestValues extends ComputedArtifact {
       },
       {
         id: 'hasIconsAtLeast192px',
-        failureText: 'Manifest does not have icons at least 192px',
+        failureText: 'Manifest does not have a PNG icon of at least 192px',
         validate: manifestValue => icons.doExist(manifestValue) &&
             icons.sizeAtLeast(192, manifestValue).length > 0,
       },
       {
         id: 'hasIconsAtLeast512px',
-        failureText: 'Manifest does not have icons at least 512px',
+        failureText: 'Manifest does not have a PNG icon of at least 512px',
         validate: manifestValue => icons.doExist(manifestValue) &&
             icons.sizeAtLeast(512, manifestValue).length > 0,
       },

lighthouse-core/lib/icons.js

@@ -31,7 +31,10 @@ function sizeAtLeast(sizeRequirement, manifest) {
   /** @type {Array<string>} */
   const flattenedSizes = [];
   iconValues.forEach(icon => {
-    if (icon.value.sizes.value) {
+    // check that the icon has a size, src, and is a png
+    if (icon.value.sizes.value &&
+        icon.value.src.value &&
+        icon.value.src.value.endsWith('.png')) {
       flattenedSizes.push(...icon.value.sizes.value);
     }
   });

lighthouse-core/test/audits/splash-screen-test.js

@@ -10,13 +10,15 @@ const assert = require('assert');
 const manifestParser = require('../../lib/manifest-parser');
 
 const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
+const manifestDirtyJpgSrc = JSON.stringify(require('../fixtures/manifest-dirty-jpg.json'));
 const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
-const exampleManifest = noUrlManifestParser(manifestSrc);
 
 const Runner = require('../../runner.js');
 
-function generateMockArtifacts() {
+function generateMockArtifacts(src = manifestSrc) {
+  const exampleManifest = manifestParser(src, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+
   const computedArtifacts = Runner.instantiateComputedArtifacts();
   const mockArtifacts = Object.assign({}, computedArtifacts, {
     Manifest: exampleManifest,
@@ -123,7 +125,18 @@ describe('PWA: splash screen audit', () => {
 
       return SplashScreenAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
-        assert.ok(result.explanation.includes('icons'), result.explanation);
+        assert.ok(result.explanation.includes('PNG icon'), result.explanation);
+      });
+    });
+
+    it('fails if an icon was not PNG', () => {
+      const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
+
+      return SplashScreenAudit.audit(artifacts).then(result => {
+        assert.strictEqual(result.rawValue, false);
+        assert.ok(result.explanation.includes('PNG icon'), result.explanation);
+        const failures = result.details.items[0].failures;
+        assert.strictEqual(failures.length, 1, failures);
       });
     });
   });

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

@@ -10,13 +10,15 @@ const assert = require('assert');
 const manifestParser = require('../../lib/manifest-parser');
 
 const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
+const manifestDirtyJpgSrc = JSON.stringify(require('../fixtures/manifest-dirty-jpg.json'));
 const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
-const exampleManifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
 
 const Runner = require('../../runner.js');
 
-function generateMockArtifacts() {
+function generateMockArtifacts(src = manifestSrc) {
+  const exampleManifest = manifestParser(src, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+
   const computedArtifacts = Runner.instantiateComputedArtifacts();
   const clonedArtifacts = JSON.parse(JSON.stringify({
     Manifest: exampleManifest,
@@ -118,13 +120,24 @@ describe('PWA: webapp install banner audit', () => {
 
       return WebappInstallBannerAudit.audit(artifacts).then(result => {
         assert.strictEqual(result.rawValue, false);
-        assert.ok(result.explanation.includes('icons'), result.explanation);
+        assert.ok(result.explanation.includes('PNG icon'), result.explanation);
         const failures = result.details.items[0].failures;
         assert.strictEqual(failures.length, 1, failures);
       });
     });
   });
 
+  it('fails if an icon was not PNG', () => {
+    const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
+
+    return WebappInstallBannerAudit.audit(artifacts).then(result => {
+      assert.strictEqual(result.rawValue, false);
+      assert.ok(result.explanation.includes('PNG icon'), result.explanation);
+      const failures = result.details.items[0].failures;
+      assert.strictEqual(failures.length, 1, failures);
+    });
+  });
+
   it('fails if page had no SW', () => {
     const artifacts = generateMockArtifacts();
     artifacts.ServiceWorker.versions = [];

lighthouse-core/test/fixtures/manifest-dirty-jpg.json

@@ -0,0 +1,31 @@
+{
+  "short_name": "ExApp",
+  "name": "Example App",
+  "start_url": "./",
+  "icons": [
+    {
+      "src": "/images/chrome-touch-icon-96x96.png",
+      "sizes": "96x96",
+      "type": "image/png"
+    },
+    {
+      "src": "/images/chrome-touch-icon-192x192.ico",
+      "sizes": "192x192",
+      "type": "image/x-icon"
+    },
+     {
+      "src": "/images/chrome-touch-icon-512x512.jpg",
+      "sizes": "512x512",
+      "type": "image/jpg"
+    },
+    {
+      "src": "/images/chrome-touch-icon-128x128.png",
+      "sizes": "64x64 128x128",
+      "type": "image/png"
+    }
+  ],
+  "background_color": "#FAFAFA",
+  "theme_color": "#123123",
+  "display": "standalone",
+  "orientation": "portrait"
+}

lighthouse-core/test/lib/icons-test.js

@@ -143,12 +143,94 @@ describe('Icons helper', () => {
     it('fails when an icon uses an invalid string for its size', () => {
       const manifestSrc = JSON.stringify({
         icons: [{
-          src: 'icon-vector.svg',
+          src: 'icon-vector.png',
           sizes: 'any',
         }],
       });
       const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
       assert.equal(icons.sizeAtLeast(192, manifest.value).length, 0);
     });
+
+    it('fails when an icon is big enough but is not png', () => {
+      const manifestSrc = JSON.stringify({
+        icons: [{
+          src: 'icon-vector.svg',
+          sizes: '256x256',
+        }],
+      });
+      const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+      assert.equal(icons.sizeAtLeast(192, manifest.value).length, 0);
+    });
+
+    it('fails with mixed files with no good pngs', () => {
+      const manifestSrc = JSON.stringify({
+        icons: [{
+          src: 'icon-vector.svg',
+          sizes: '256x256',
+        },
+        {
+          src: 'icon.png',
+          sizes: '100x100',
+        },
+        {
+          src: 'path/icon.ico',
+          sizes: '256x256',
+        }],
+      });
+      const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+      assert.equal(icons.sizeAtLeast(192, manifest.value).length, 0);
+    });
+
+    it('succeeds with mixed files with good pngs', () => {
+      const manifestSrc = JSON.stringify({
+        icons: [{
+          src: 'icon-vector.svg',
+          sizes: '100x100',
+        },
+        {
+          src: 'icon.png',
+          sizes: '256x256',
+        },
+        {
+          src: 'path/icon.ico',
+          sizes: '100x100',
+        }],
+      });
+      const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+      assert.equal(icons.sizeAtLeast(192, manifest.value).length, 1);
+    });
+
+    it('succeeds with an icon that has no standalone filename', () => {
+      const manifestSrc = JSON.stringify({
+        icons: [{
+          src: '.png',
+          sizes: '200x200',
+        }],
+      });
+      const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+      assert.equal(icons.sizeAtLeast(192, manifest.value).length, 1);
+    });
+
+    it('succeeds with an icon that has a path but no filename', () => {
+      const manifestSrc = JSON.stringify({
+        icons: [{
+          src: 'path/to/.png',
+          sizes: '200x200',
+        }],
+      });
+      const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+      assert.equal(icons.sizeAtLeast(192, manifest.value).length, 1);
+    });
+
+    it('succeeds with an icon that has a path', () => {
+      const manifestSrc = JSON.stringify({
+        icons: [{
+          src: 'path/to/image.png',
+          sizes: '200x200',
+        }],
+      });
+      const manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
+      assert.equal(icons.sizeAtLeast(192, manifest.value).length, 1);
+    });
   });
 });