Merge pull request #306 from GoogleChrome/manifest-icon-fixes

Fixes a manifest validation and lets add PWAs without icon

lib/pwa.js

@@ -338,7 +338,10 @@ exports._save = function(pwa) {
           // Validates the manifest or, reject and return errors in an array
           const res = libManifest.validateManifest(
             manifest.raw, pwa.manifestUrl, pwa.absoluteStartUrl);
-          pwa.manifest = res.length === 0 ? manifest : Promise.reject(res);
+          if (res.length !== 0) {
+            return Promise.reject('Error while validating the manifest: ' + res);
+          }
+          pwa.manifest = manifest;
 
           return this.savePwa(pwa)
             .then(savedPwa => {
@@ -426,7 +429,7 @@ exports.updatePwaIcon = function(pwa) {
   const bestIconUrl = pwa.manifest.getBestIconUrl();
   if (!bestIconUrl) {
     console.log('bestIconUrl is null');
-    Promise.resolve(null);
+    return Promise.resolve(pwa);
   }
   const extension = path.extname(url.parse(bestIconUrl).pathname);
   const bucketFileName = pwa.id + extension;

test/lib/manifest.js

@@ -33,7 +33,7 @@ describe('lib.manifest', () => {
     it('returns empty array if manifest ok', () => {
       const manifest = fs.readFileSync('./test/manifests/icon-url-with-parameter.json');
       const manifestUrl = 'https://example.com/';
-      const documentUrl = 'https://www.terra.com.br/';
+      const documentUrl = 'https://www.example.com/';
       const actual = libManifest.validateManifest(manifest, manifestUrl, documentUrl);
       assert.deepEqual(actual, []);
     });

test/lib/pwa.js

@@ -13,50 +13,58 @@
  * limitations under the License.
  */
 
-/* global describe it before beforeEach afterEach*/
+/* global describe it beforeEach afterEach*/
 'use strict';
 
-let dataFetcher = require('../../lib/data-fetcher');
-let libPwa = require('../../lib/pwa');
-let libImages = require('../../lib/images');
-let libManifest = require('../../lib/manifest');
-let libLighthouse = require('../../lib/lighthouse');
-let db = require('../../lib/model-datastore');
-let cache = require('../../lib/data-cache');
+const fs = require('fs');
+const dataFetcher = require('../../lib/data-fetcher');
+const libPwa = require('../../lib/pwa');
+const libImages = require('../../lib/images');
+const libManifest = require('../../lib/manifest');
+const libLighthouse = require('../../lib/lighthouse');
+const db = require('../../lib/model-datastore');
+const cache = require('../../lib/data-cache');
 
-let Lighthouse = require('../../models/lighthouse');
-let Manifest = require('../../models/manifest');
-let Pwa = require('../../models/pwa');
+const Lighthouse = require('../../models/lighthouse');
+const Pwa = require('../../models/pwa');
 
-let simpleMock = require('simple-mock');
-let chai = require('chai');
-let chaiAsPromised = require('chai-as-promised');
+const testPwa = require('../models/pwa');
+const simpleMock = require('simple-mock');
+const chai = require('chai');
+const chaiAsPromised = require('chai-as-promised');
 chai.use(chaiAsPromised);
 chai.should();
-let assert = require('chai').assert;
+const assert = require('chai').assert;
 
-const MANIFEST_URL = 'https://www.terra.com.br/manifest-br.json';
-const START_URL = 'https://www.terra.com.br/?utm_source=homescreen';
-const MANIFEST_DATA = './test/manifests/icon-url-with-parameter.json';
+const MANIFEST_URL = 'https://www.domain.com/manifest-br.json';
+const START_URL = 'https://www.domain.com/?utm_source=homescreen';
 const LIGHTHOUSE_JSON_EXAMPLE = './test/lib/lighthouse-example.json';
 
+/* eslint-disable camelcase */
+const MANIFEST_DATA = {
+  name: 'Test',
+  icons: [
+    {
+      src: 'img/launcher-icon.png?v2',
+      sizes: '192x192',
+      type: 'image/png'
+    }
+  ],
+  start_url: 'https://www.example.com/?utm_source=homescreen'
+};
+const MANIFEST_NO_ICON = {name: 'Test', description: 'Manifest without icons', start_url: '/'};
+const MANIFEST_INVALID_THEME_COLOR = {
+  description: 'Manifest with an invalid theme_color', theme_color: ''};
+/* eslint-enable camelcase */
+
 describe('lib.pwa', () => {
-  let manifest;
-  let pwa;
-  let lighthouse;
-  before(done => {
-    dataFetcher.readFile(MANIFEST_DATA)
-      .then(jsonString => {
-        manifest = new Manifest(MANIFEST_URL, JSON.parse(jsonString));
-        pwa = new Pwa(MANIFEST_URL, manifest);
-        pwa.id = '123456789';
-        dataFetcher.readFile(LIGHTHOUSE_JSON_EXAMPLE)
-          .then(data => {
-            lighthouse = new Lighthouse('123456789', 'www.domain.com', JSON.parse(data));
-            done();
-          });
-      });
-  });
+  const pwa = testPwa.createPwa(MANIFEST_URL, MANIFEST_DATA);
+  pwa.id = '123456789';
+  const manifest = pwa.manifest;
+  const pwaNoIcon = testPwa.createPwa(MANIFEST_URL, MANIFEST_NO_ICON);
+  const pwaInvalidThemeColor = testPwa.createPwa(MANIFEST_URL, MANIFEST_INVALID_THEME_COLOR);
+  const lighthouse = new Lighthouse(
+    '123456789', 'www.domain.com', JSON.parse(fs.readFileSync(LIGHTHOUSE_JSON_EXAMPLE)));
 
   describe('#updatePwaMetadataDescription', () => {
     afterEach(() => {
@@ -95,13 +103,18 @@ describe('lib.pwa', () => {
       return libPwa.updatePwaIcon(pwa).should.be.fulfilled.then(updatedPwa => {
         assert.equal(libImages.fetchAndSave.callCount, 1);
         assert.equal(libImages.fetchAndSave.lastCall.args[0],
-          'https://s1.trrsf.com/fe/zaz-morph/_img/launcher-icon.png?v2');
+          'https://www.domain.com/img/launcher-icon.png?v2');
         assert.equal(libImages.fetchAndSave.lastCall.args[1], '123456789.png');
         assert.equal(updatedPwa.iconUrl, 'original');
         assert.equal(updatedPwa.iconUrl128, '128');
         assert.equal(updatedPwa.iconUrl64, '64');
       });
     });
+    it('allows PWAs without icon', () => {
+      return libPwa.updatePwaIcon(pwaNoIcon).should.be.fulfilled.then(updatedPwa => {
+        assert.equal(updatedPwa.iconUrl, null);
+      });
+    });
   });
 
   describe('#updatePwaLighthouseInfo', () => {
@@ -230,10 +243,17 @@ describe('lib.pwa', () => {
       });
     });
     it('handles E_MANIFEST_ERROR error', () => {
-      simpleMock.mock(libManifest, 'fetchManifest').resolveWith(manifest);
+      simpleMock.mock(libPwa, 'fetchManifest').resolveWith(manifest);
       simpleMock.mock(libPwa, 'findByManifestUrl').rejectWith(new Error('Testing error'));
       return libPwa._save(pwa).should.be.rejectedWith(libPwa.E_MANIFEST_ERROR);
     });
+    it('rejects invalid Manifest', () => {
+      simpleMock.mock(libPwa, 'fetchManifest').resolveWith(pwaInvalidThemeColor.manifest);
+      simpleMock.mock(libPwa, 'findByManifestUrl').resolveWith(pwaInvalidThemeColor);
+      return libPwa._save(pwaInvalidThemeColor).should.be.rejected.then(error => {
+        assert.equal(error, 'Error while validating the manifest: ERROR: color parsing failed.');
+      });
+    });
   });
 
   describe('#save (validation logic)', () => {

test/manifests/icon-url-with-parameter.json

@@ -1,17 +1,11 @@
 {
-  "lang": "pt-BR",
-  "short_name": "Terra Brasil",
-  "name": "Terra Brasil",
+  "name": "Test",
   "icons": [
     {
-      "src": "https://s1.trrsf.com/fe/zaz-morph/_img/launcher-icon.png?v2",
+      "src": "img/launcher-icon.png?v2",
       "sizes": "192x192",
       "type": "image/png"
     }
   ],
-  "start_url": "https://www.terra.com.br/?utm_source=homescreen",
-  "display": "standalone",
-  "orientation": "portrait",
-  "background_color": "#ff7212",
-  "theme_color": "#ff7212"
-}
+  "start_url": "https://www.example.com/?utm_source=homescreen"
+}

test/manifests/inline-image-large-content.json

@@ -1 +1 @@
-{"short_name":"Twitter","name":"Twitter","icons":[{"src":"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMAAAADACAMAAABlApw1AAAAk1BMVEX///8dofJ9yfggo/IeofLF5/wnpvM6rfT7/f8sqPM0q/MkpPLs9/7l9P7Q7P2k2fr3/P+T0vnw+f7g8/2FzfhevPZWufVKtPXz+v604Pt2x/dAsPT9/v+Lz/lwxPdrwvdRt/Wv3vqr3PpjvvbL6vy64vua1vlau/bZ7/2+5PtnwPbd8f1EsvROtvXn9v6e1/lHsvRrYjr1AAAESUlEQVR42u3c2XbaMBQF0HM84xkMmHkmDCFN///r2iRNmzRMVmRj1rr7iRdkX10hCVkyhBBCCCGEEEIIIYQQQgghhBBCCCGEOC9aN419Z7OftOexhzszsgcuP3A6SQ/3o9d0+UUwiU8Eu2ihVtJmg8ctt/hqnc9QKzuXJ5njFJ+M+jm5gEb2Gt/iGRbPOWT4p/XwEqzvQaPcifENrRkvaOzwJl1PA75ItOaf9CMoi4a8yJwDiBZT5z0nHjTqkByGUPTD5RXM9uRDnJYNjX6Y/C3vQUnvwOIM6JTw1ZNaBEsW9+RBp857sS0U98zi/B/QaeTwj58hisoChfvP8EcEHbb8a5gpZK8oN8Or1vPPFXTo859GwSJtFjb7AQCtxTLgGFo88IOgjyI2LMgyPPTscdck2R1BC+PzFdoFio1ZUGA0p7nJV34I/QG86IYFvqrO2UKTAf/j27iO16Aycw1djCMNNcU1VlRmzaFNm18NY1yhSVVmAn2ej/cWLeU+6Kq5qUY7HuVfvohPNcEaOkU8IbdxVs+ikoYNBUoV2YlxRkYlw0doNuFpm9W5YUzFvgfd1jxn1vd09qId6Jc6PMsfR/oyMEUJBrzA6iYtfPFIBXuUYGvxInOTZPgsrEsTAva8ysHoh/jAqU0Ajyav5e6buwhv8rr8BoA2CwkOnUH7YTGrTwDpkBUZoBxbh9VooyR9i5V4QFkeWIkFtBvs/kRQSQ5saJczfxtn5wHL9wjtuiTNzsMqReyybFYK7SZ8Yw43HYslc6HfmIrqMhXq87zaDwMRL6h5Lwq4PKvunRBgsDLOCCVYsTIblOInq9JEKRJWZYVSeC6r4Xgox5zVWKIsXVYiQVkyhxWwQnx2b/OJLkrU5jH30oJeTFg2s4UyeUciuJc+qKJWtEPZ5g5L5I5QuqzLz+5gHvSfucuSBCEq4S0OvOzm2+TOitslpMHMULqw9f7BNgJqNkH5+mwc8m4+dKif+YhvUN89U/vHAlUtcQURqhA6LMkYx91LCvweqtHzWYo5qrLjB3VfDapslS7IUB1vxnd38FzvmHDIV3fyT7jAgRJVjQgVC3Pq1Efl0ul9zqI/WDjUZObhJqKpRR38ELcS799DqMUOdRXR2OWLWuxQV/SYTPIGVVkJaiEdW/cwAp/U2td6HeiinX8XM6BTItXqt55RA17ToZqgj9vzEpesyQkBBWnik6zLCYHCtkaDypY93FaYzKjObOKW0rg5M/kNboxbCeO58RTwW6xBD1Wyl8vJwGgPBtPNweH3uTYq1jJMamO2U1Rv26Umywy3Yc+oQdfG1WoYQtfGbdlLi8rM6Qq3l7UbVOKPI9SDt54GLMiZ2CPUSK8/LZCHxmCdonY8ezwLeJGzaca1qvtP0lUyeHJOVnzXWGzre/N/jTJ70TSWT0O/8ZvvunlnYjT7cc1eTiaEEEIIIYQQQgghhBBCCCGEEEIITX4BpH1Py9uEEC8AAAAASUVORK5CYII=","sizes":"192x192","type":"image/png"}],"gcm_sender_id":"49625052041","gcm_user_visible_only":true,"start_url":"/","display":"standalone","orientation":"portrait","background_color":"white","theme_color":"white"}
+{"short_name":"Twitter","name":"Twitter","icons":[{"src":"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMAAAADACAMAAABlApw1AAAAk1BMVEX///8dofJ9yfggo/IeofLF5/wnpvM6rfT7/f8sqPM0q/MkpPLs9/7l9P7Q7P2k2fr3/P+T0vnw+f7g8/2FzfhevPZWufVKtPXz+v604Pt2x/dAsPT9/v+Lz/lwxPdrwvdRt/Wv3vqr3PpjvvbL6vy64vua1vlau/bZ7/2+5PtnwPbd8f1EsvROtvXn9v6e1/lHsvRrYjr1AAAESUlEQVR42u3c2XbaMBQF0HM84xkMmHkmDCFN///r2iRNmzRMVmRj1rr7iRdkX10hCVkyhBBCCCGEEEIIIYQQQgghhBBCCCGEOC9aN419Z7OftOexhzszsgcuP3A6SQ/3o9d0+UUwiU8Eu2ihVtJmg8ctt/hqnc9QKzuXJ5njFJ+M+jm5gEb2Gt/iGRbPOWT4p/XwEqzvQaPcifENrRkvaOzwJl1PA75ItOaf9CMoi4a8yJwDiBZT5z0nHjTqkByGUPTD5RXM9uRDnJYNjX6Y/C3vQUnvwOIM6JTw1ZNaBEsW9+RBp857sS0U98zi/B/QaeTwj58hisoChfvP8EcEHbb8a5gpZK8oN8Or1vPPFXTo859GwSJtFjb7AQCtxTLgGFo88IOgjyI2LMgyPPTscdck2R1BC+PzFdoFio1ZUGA0p7nJV34I/QG86IYFvqrO2UKTAf/j27iO16Aycw1djCMNNcU1VlRmzaFNm18NY1yhSVVmAn2ej/cWLeU+6Kq5qUY7HuVfvohPNcEaOkU8IbdxVs+ikoYNBUoV2YlxRkYlw0doNuFpm9W5YUzFvgfd1jxn1vd09qId6Jc6PMsfR/oyMEUJBrzA6iYtfPFIBXuUYGvxInOTZPgsrEsTAva8ysHoh/jAqU0Ajyav5e6buwhv8rr8BoA2CwkOnUH7YTGrTwDpkBUZoBxbh9VooyR9i5V4QFkeWIkFtBvs/kRQSQ5saJczfxtn5wHL9wjtuiTNzsMqReyybFYK7SZ8Yw43HYslc6HfmIrqMhXq87zaDwMRL6h5Lwq4PKvunRBgsDLOCCVYsTIblOInq9JEKRJWZYVSeC6r4Xgox5zVWKIsXVYiQVkyhxWwQnx2b/OJLkrU5jH30oJeTFg2s4UyeUciuJc+qKJWtEPZ5g5L5I5QuqzLz+5gHvSfucuSBCEq4S0OvOzm2+TOitslpMHMULqw9f7BNgJqNkH5+mwc8m4+dKif+YhvUN89U/vHAlUtcQURqhA6LMkYx91LCvweqtHzWYo5qrLjB3VfDapslS7IUB1vxnd38FzvmHDIV3fyT7jAgRJVjQgVC3Pq1Efl0ul9zqI/WDjUZObhJqKpRR38ELcS799DqMUOdRXR2OWLWuxQV/SYTPIGVVkJaiEdW/cwAp/U2td6HeiinX8XM6BTItXqt55RA17ToZqgj9vzEpesyQkBBWnik6zLCYHCtkaDypY93FaYzKjObOKW0rg5M/kNboxbCeO58RTwW6xBD1Wyl8vJwGgPBtPNweH3uTYq1jJMamO2U1Rv26Umywy3Yc+oQdfG1WoYQtfGbdlLi8rM6Qq3l7UbVOKPI9SDt54GLMiZ2CPUSK8/LZCHxmCdonY8ezwLeJGzaca1qvtP0lUyeHJOVnzXWGzre/N/jTJ70TSWT0O/8ZvvunlnYjT7cc1eTiaEEEIIIYQQQgghhBBCCCGEEEIITX4BpH1Py9uEEC8AAAAASUVORK5CYII=","sizes":"192x192","type":"image/png"}],"start_url":"/","display":"standalone","orientation":"portrait","background_color":"white","theme_color":"white"}

test/manifests/invalid-theme-color.json

@@ -1,17 +1,5 @@
 {
-  "lang": "pt-BR",
-  "short_name": "Terra Brasil",
-  "name": "Terra Brasil",
-  "icons": [
-    {
-      "src": "https://s1.trrsf.com/fe/zaz-morph/_img/launcher-icon.png?v2",
-      "sizes": "192x192",
-      "type": "image/png"
-    }
-  ],
   "start_url": "https://www.terra.com.br/?utm_source=homescreen",
-  "display": "standalone",
-  "orientation": "portrait",
-  "background_color": "#ff7212",
+  "description": "Manifest with an invalid theme_color",
   "theme_color": "not_a_real_color"
-}
+}

test/manifests/no-icon-array.json

@@ -1,23 +1,6 @@
 {
-  "name": "Telegram",
-  "description": "Telegram Web App.\nMore info & source code here: https://github.com/zhukov/webogram",
-  "version": "0.5.5",
-  "short_name": "Telegram",
-  "manifest_version": 2,
-  "app": {
-    "background": {
-      "scripts": ["js/background.js"]
-    }
-  },
-  "permissions": [
-    "notifications",
-    "webview",
-    {"fileSystem": ["write"]},
-    "storage",
-    "unlimitedStorage",
-    "fullscreen"
-  ],
-  "gcm_sender_id": "122867383838",
+  "name": "Test",
+  "description": "Manifest with icons that are not in array",
   "icons": {
     "16": "img/icons/icon16.png",
     "32": "img/icons/icon32.png",

test/models/pwa.js

@@ -97,37 +97,44 @@ describe('models/pwa.js', () => {
 
   describe('displayName', () => {
     it('is name', () => {
-      const pwa = createPwa('www.manifesturl.com', {
+      const pwa = this.createPwa('www.manifesturl.com', {
         name: 'Example PWA',
         short_name: 'PWA'
       });
       assert.equal(pwa.displayName, 'Example PWA');
     });
     it('is short name', () => {
-      const pwa = createPwa('www.manifesturl.com', {
+      const pwa = this.createPwa('www.manifesturl.com', {
         short_name: 'PWA'
       });
       assert.equal(pwa.displayName, 'PWA');
     });
     it('is url', () => {
-      const pwa = createPwa('www.manifesturl.com', {
+      const pwa = this.createPwa('www.manifesturl.com', {
       });
       assert.equal(pwa.displayName, 'www.manifesturl.com');
     });
     it('is url without file name', () => {
-      const pwa = createPwa('www.manifesturl.com/manifest.json', {
+      const pwa = this.createPwa('www.manifesturl.com/manifest.json', {
       });
       assert.equal(pwa.displayName, 'www.manifesturl.com');
     });
     it('is url without scheme', () => {
-      const pwa = createPwa('https://www.manifesturl.com/manifest.json', {
+      const pwa = this.createPwa('https://www.manifesturl.com/manifest.json', {
       });
       assert.equal(pwa.displayName, 'www.manifesturl.com');
     });
   });
-
-  function createPwa(manifestUrl, manifestData) {
-    const manifest = new Manifest(manifestUrl, manifestData);
-    return new Pwa(manifestUrl, manifest);
-  }
 });
+
+/**
+ * Creates a PWA object from a Manifest URL and Json Data for testing.
+ *
+ * @param {string} manifestUrl the URL for the Manifest
+ * @param {Json} manifestData the Json object with the Manifest data
+ * @return {Pwa}
+ */
+exports.createPwa = function(manifestUrl, manifestData) {
+  const manifest = new Manifest(manifestUrl, manifestData);
+  return new Pwa(manifestUrl, manifest);
+};