From c264f72ff7feaf9fb1863755e931cc1eaef6e0a4 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Thu, 7 Apr 2016 13:31:00 +1000 Subject: [PATCH] added short-name-length check --- .../index.js | 9 +++ audits/manifest/short-name-length.js | 61 +++++++++++++++++++ extension/app/scripts.babel/pwa-check.js | 1 + index.js | 1 + test/audits/manifest/short-name-length.js | 59 ++++++++++++++++++ 5 files changed, 131 insertions(+) create mode 100644 audits/manifest/short-name-length.js create mode 100644 test/audits/manifest/short-name-length.js diff --git a/aggregators/will-get-add-to-homescreen-prompt/index.js b/aggregators/will-get-add-to-homescreen-prompt/index.js index a74ffcca9b8a..6931a8f4bcc7 100644 --- a/aggregators/will-get-add-to-homescreen-prompt/index.js +++ b/aggregators/will-get-add-to-homescreen-prompt/index.js @@ -37,6 +37,9 @@ const manifestIcons192 = require('../../audits/manifest/icons-192').name; /** @type {string} */ const manifestShortName = require('../../audits/manifest/short-name').name; +/** @type {string} */ +const manifestShortNameLength = require('../../audits/manifest/short-name-length').name; + class AddToHomescreen extends Aggregate { /** @@ -55,6 +58,7 @@ class AddToHomescreen extends Aggregate { * - valid start_url * - valid name * - valid short_name + * - short_name of reasonable length * - icon of size >= 144x144 and png (either type `image/png` or filename ending in `.png` * @see https://github.com/GoogleChrome/lighthouse/issues/23 * @@ -94,6 +98,11 @@ class AddToHomescreen extends Aggregate { weight: 0 }; + criteria[manifestShortNameLength] = { + value: true, + weight: 0 + }; + return criteria; } } diff --git a/audits/manifest/short-name-length.js b/audits/manifest/short-name-length.js new file mode 100644 index 000000000000..1f467df7dd8f --- /dev/null +++ b/audits/manifest/short-name-length.js @@ -0,0 +1,61 @@ +/** + * @license + * Copyright 2016 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +const Audit = require('../audit'); + +class ManifestShortNameLength extends Audit { + /** + * @override + */ + static get tags() { + return ['Manifest']; + } + + /** + * @override + */ + static get name() { + return 'manifest-short-name-length'; + } + + /** + * @override + */ + static get description() { + return 'Manifest short_name won\'t be truncated'; + } + + /** + * @param {!Artifacts} artifacts + * @return {!AuditResult} + */ + static audit(artifacts) { + let isShortNameShortEnough = false; + const manifest = artifacts.manifest.value; + + if (manifest && manifest.short_name) { + // Historically, Chrome recommended 12 chars as the maximum length to prevent truncation. + isShortNameShortEnough = (manifest.short_name.value.length <= 12); + } + + return ManifestShortNameLength.generateAuditResult(isShortNameShortEnough); + } +} + +module.exports = ManifestShortNameLength; diff --git a/extension/app/scripts.babel/pwa-check.js b/extension/app/scripts.babel/pwa-check.js index 745440bf8371..3adc45e09d15 100644 --- a/extension/app/scripts.babel/pwa-check.js +++ b/extension/app/scripts.babel/pwa-check.js @@ -43,6 +43,7 @@ const audits = [ require('../../../audits/manifest/icons-192'), require('../../../audits/manifest/name'), require('../../../audits/manifest/short-name'), + require('../../../audits/manifest/short-name-length'), require('../../../audits/manifest/start-url'), require('../../../audits/html/meta-theme-color') ]; diff --git a/index.js b/index.js index e59b1f8b9548..bad995296246 100644 --- a/index.js +++ b/index.js @@ -48,6 +48,7 @@ const audits = [ require('./audits/manifest/icons-192'), require('./audits/manifest/name'), require('./audits/manifest/short-name'), + require('./audits/manifest/short-name-length'), require('./audits/manifest/start-url'), require('./audits/html/meta-theme-color') ]; diff --git a/test/audits/manifest/short-name-length.js b/test/audits/manifest/short-name-length.js new file mode 100644 index 000000000000..2aef8131339b --- /dev/null +++ b/test/audits/manifest/short-name-length.js @@ -0,0 +1,59 @@ +/** + * Copyright 2016 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +const Audit = require('../../../audits/manifest/short-name-length.js'); +const assert = require('assert'); +const manifestSrc = JSON.stringify(require('./manifest.json')); +const manifestParser = require('../../../helpers/manifest-parser'); +const manifest = manifestParser(manifestSrc); + +/* global describe, it*/ + +describe('Manifest: short_name_length audit', () => { + it('fails when no manifest present', () => { + return assert.equal(Audit.audit({manifest: { + value: undefined + }}).value, false); + }); + + it('fails when an empty manifest is present', () => { + return assert.equal(Audit.audit({manifest: {}}).value, false); + }); + + // Need to disable camelcase check for dealing with short_name. + /* eslint-disable camelcase */ + it('fails when a manifest contains no short_name', () => { + const inputs = { + manifest: { + short_name: null + } + }; + return assert.equal(Audit.audit(inputs).value, false); + }); + + it('fails when a manifest contains a long short_name', () => { + const inputs = { + manifest: { + short_name: 'i\'m much longer than the recommended size' + } + }; + return assert.equal(Audit.audit(inputs).value, false); + }); + /* eslint-enable camelcase */ + + it('succeeds when a manifest contains a short_name', () => { + return assert.equal(Audit.audit({manifest: manifest}).value, true); + }); +});