correctly fall back on erroneous manifest display modes

also converts `display` to lowercase to correctly match spec

lighthouse-core/audits/manifest-display.js

@@ -46,7 +46,7 @@ class ManifestDisplay extends Audit {
    */
   static audit(artifacts) {
     const manifest = artifacts.Manifest.value;
-    const displayValue = (!manifest || !manifest.display) ? undefined : manifest.display.value;
+    const displayValue = manifest && manifest.display.value;
 
     const hasRecommendedValue = ManifestDisplay.hasRecommendedValue(displayValue);
 

lighthouse-core/closure/typedefs/Manifest.js

@@ -94,7 +94,7 @@ Manifest.prototype.short_name;
 /** @type {!ManifestNode<(string|undefined)>} */
 Manifest.prototype.start_url;
 
-/** @type {!ManifestNode<(string|undefined)>} */
+/** @type {!ManifestNode<string>} */
 Manifest.prototype.display;
 
 /** @type {!ManifestNode<(string|undefined)>} */

lighthouse-core/lib/manifest-parser.js

@@ -24,6 +24,11 @@ const ALLOWED_DISPLAY_VALUES = [
   'minimal-ui',
   'browser'
 ];
+/**
+ * All display-mode fallbacks, including when unset, lead to default display mode 'browser'.
+ * @see https://w3c.github.io/manifest/#dfn-default-display-mode
+ */
+const DEFAULT_DISPLAY_MODE = 'browser';
 
 const ALLOWED_ORIENTATION_VALUES = [
   'any',
@@ -98,9 +103,16 @@ function parseStartUrl(jsonInput) {
 function parseDisplay(jsonInput) {
   let display = parseString(jsonInput.display, true);
 
-  if (display.value && ALLOWED_DISPLAY_VALUES.indexOf(display.value.toLowerCase()) === -1) {
-    display.value = undefined;
-    display.debugString = 'ERROR: \'display\' has an invalid value, will be ignored.';
+  if (!display.value) {
+    display.value = DEFAULT_DISPLAY_MODE;
+    return display;
+  }
+
+  display.value = display.value.toLowerCase();
+  if (ALLOWED_DISPLAY_VALUES.indexOf(display.value) === -1) {
+    display.debugString = 'ERROR: \'display\' has invalid value ' + display.value +
+        ` will fall back to ${DEFAULT_DISPLAY_MODE}.`;
+    display.value = DEFAULT_DISPLAY_MODE;
   }
 
   return display;

lighthouse-core/test/audits/display.js

@@ -36,15 +36,15 @@ describe('Mobile-friendly: display audit', () => {
     }}).rawValue, false);
   });
 
-  it('handles the case where there is no manifest display property', () => {
+  it('falls back to the successful default when there is no manifest display property', () => {
     const artifacts = {
       Manifest: manifestParser('{}')
     };
     const output = Audit.audit(artifacts);
 
-    assert.equal(output.score, false);
-    assert.equal(output.displayValue, '');
-    assert.equal(output.rawValue, false);
+    assert.equal(output.score, true);
+    assert.equal(output.displayValue, 'browser');
+    assert.equal(output.rawValue, true);
   });
 
   it('succeeds when a manifest has a display property', () => {

lighthouse-core/test/manifest-parser.js → lighthouse-core/test/lib/manifest-parser.js

@@ -15,11 +15,11 @@
  */
 'use strict';
 
-/* global describe, it */
+/* eslint-env mocha */
 
-var manifestParser = require('../lib/manifest-parser');
+var manifestParser = require('../../lib/manifest-parser');
 var assert = require('assert');
-const manifestStub = require('./fixtures/manifest.json');
+const manifestStub = require('../fixtures/manifest.json');
 
 describe('Manifest Parser', function() {
   it('should not parse empty string input', function() {
@@ -33,7 +33,7 @@ describe('Manifest Parser', function() {
     assert.equal(parsedManifest.value.name.value, undefined);
     assert.equal(parsedManifest.value.short_name.value, undefined);
     assert.equal(parsedManifest.value.start_url.value, undefined);
-    assert.equal(parsedManifest.value.display.value, undefined);
+    assert.equal(parsedManifest.value.display.value, 'browser');
     assert.equal(parsedManifest.value.orientation.value, undefined);
     assert.equal(parsedManifest.value.theme_color.value, undefined);
     assert.equal(parsedManifest.value.background_color.value, undefined);
@@ -43,19 +43,6 @@ describe('Manifest Parser', function() {
     // prefer_related_applications
   });
 
-  it('accepts unknown values', function() {
-    // TODO(bckenny): this is the same exact test as above
-    let parsedManifest = manifestParser('{}');
-    assert(!parsedManifest.debugString);
-    assert.equal(parsedManifest.value.name.value, undefined);
-    assert.equal(parsedManifest.value.short_name.value, undefined);
-    assert.equal(parsedManifest.value.start_url.value, undefined);
-    assert.equal(parsedManifest.value.display.value, undefined);
-    assert.equal(parsedManifest.value.orientation.value, undefined);
-    assert.equal(parsedManifest.value.theme_color.value, undefined);
-    assert.equal(parsedManifest.value.background_color.value, undefined);
-  });
-
   describe('icon parsing', function() {
     it('parses basic string', function() {
       let parsedManifest = manifestParser('{"icons": [{"src": "192.png", "sizes": "192x192"}]}');
@@ -131,4 +118,82 @@ describe('Manifest Parser', function() {
       assert.equal(parsedManifest.value.short_name.value, undefined);
     });
   });
+
+  /**
+   * @see https://w3c.github.io/manifest/#display-member
+   */
+  describe('display parsing', () => {
+    it('falls back to \'browser\' and issues a warning for an invalid value', () => {
+      const parsedManifest = manifestParser('{"display": {} }');
+      const display = parsedManifest.value.display;
+      assert.ok(display.debugString);
+      assert.equal(display.value, 'browser');
+    });
+
+    it('falls back to \'browser\' and issues a warning for an invalid value', () => {
+      const parsedManifest = manifestParser('{"display": 5 }');
+      const display = parsedManifest.value.display;
+      assert.ok(display.debugString);
+      assert.equal(display.value, 'browser');
+    });
+
+    it('falls back to \'browser\' and issues no warning when undefined', () => {
+      const parsedManifest = manifestParser('{}');
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'browser');
+      assert.equal(display.rawValue, undefined);
+    });
+
+    it('trims whitespace', () => {
+      const displayValue = ' fullscreen     ';
+      const parsedManifest = manifestParser(`{"display": "${displayValue}" }`);
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'fullscreen');
+    });
+
+    it('converts to lowercase', () => {
+      const displayValue = 'fUlLScrEEn';
+      const parsedManifest = manifestParser(`{"display": "${displayValue}" }`);
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'fullscreen');
+    });
+
+    it('falls back to \'browser\' and issues a warning when a non-existent mode', () => {
+      const parsedManifest = manifestParser('{"display": "fullestscreen" }');
+      const display = parsedManifest.value.display;
+      assert.ok(display.debugString);
+      assert.equal(display.value, 'browser');
+    });
+
+    it('correctly parses `fullscreen` display mode', () => {
+      const parsedManifest = manifestParser('{"display": "fullscreen" }');
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'fullscreen');
+    });
+
+    it('correctly parses `standalone` display mode', () => {
+      const parsedManifest = manifestParser('{"display": "standalone" }');
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'standalone');
+    });
+
+    it('correctly parses `minimal-ui` display mode', () => {
+      const parsedManifest = manifestParser('{"display": "minimal-ui" }');
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'minimal-ui');
+    });
+
+    it('correctly parses `browser` display mode', () => {
+      const parsedManifest = manifestParser('{"display": "browser" }');
+      const display = parsedManifest.value.display;
+      assert.ok(!display.debugString);
+      assert.equal(display.value, 'browser');
+    });
+  });
 });