Repository: GoogleChrome/lighthouse
Pull request #7223 (2019-02-12 15:24:22)

core(mobile-friendly): Remove manual mobile friendly audit
**Summary**

Now that tap targets is merged we don't need need the manual mobile friendly check audit any more.


Patch:
diff --git a/.travis.yml b/.travis.yml
index bfd347b23..b42b9cebf 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -8,6 +8,8 @@ matrix:
     - node_js: "8"
     - node_js: "10"
       if: head_branch IS blank AND branch = master
+    - node_js: "11"
+      if: head_branch IS blank AND branch = master
 dist: trusty
 cache:
   yarn: true
diff --git a/lighthouse-core/audits/font-display.js b/lighthouse-core/audits/font-display.js
index 2a1ce2609..53953b73c 100644
--- a/lighthouse-core/audits/font-display.js
+++ b/lighthouse-core/audits/font-display.js
@@ -7,7 +7,7 @@
 
 const Audit = require('./audit');
 const URL = require('../lib/url-shim').URL;
-const PASSING_FONT_DISPLAY_REGEX = /block|fallback|optional|swap/;
+const PASSING_FONT_DISPLAY_REGEX = /^(block|fallback|optional|swap)$/;
 const CSS_URL_REGEX = /url\((.*?)\)/;
 const CSS_URL_GLOBAL_REGEX = new RegExp(CSS_URL_REGEX, 'g');
 const i18n = require('../lib/i18n/i18n.js');
@@ -57,11 +57,13 @@ class FontDisplay extends Audit {
       const fontFaceDeclarations = newlinesStripped.match(/@font-face\s*{(.*?)}/g) || [];
       // Go through all the @font-face declarations to find a declared `font-display: ` property
       for (const declaration of fontFaceDeclarations) {
-        const rawFontDisplay = declaration.match(/font-display:(.*?);/);
+        // Find the font-display value by matching a single token, optionally surrounded by whitespace,
+        // followed either by a semicolon or the end of a block.
+        const rawFontDisplay = declaration.match(/font-display\s*:\s*(\w+)\s*(;|\})/);
         // If they didn't have a font-display property, it's the default, and it's failing; bail
         if (!rawFontDisplay) continue;
         // If they don't have one of the passing font-display values, it's failing; bail
-        const hasPassingFontDisplay = PASSING_FONT_DISPLAY_REGEX.test(rawFontDisplay[0]);
+        const hasPassingFontDisplay = PASSING_FONT_DISPLAY_REGEX.test(rawFontDisplay[1]);
         if (!hasPassingFontDisplay) continue;
 
         // If it's passing, we'll try to find the URL it's referencing.
diff --git a/lighthouse-core/audits/seo/manual/mobile-friendly.js b/lighthouse-core/audits/seo/manual/mobile-friendly.js
deleted file mode 100644
index 402416a47..000000000
--- a/lighthouse-core/audits/seo/manual/mobile-friendly.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/**
- * @license Copyright 2017 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 ManualAudit = require('../../manual/manual-audit');
-
-/**
- * @fileoverview Manual SEO audit to check if page is mobile friendly.
- */
-
-class MobileFriendly extends ManualAudit {
-  /**
-   * @return {LH.Audit.Meta}
-   */
-  static get meta() {
-    return Object.assign({
-      id: 'mobile-friendly',
-      description: 'Take the [Mobile-Friendly Test](https://search.google.com/test/mobile-friendly) to check for audits not covered by Lighthouse, like sizing tap targets appropriately. [Learn more](https://developers.google.com/search/mobile-sites/).',
-      title: 'Page is mobile friendly',
-    }, super.partialMeta);
-  }
-}
-
-module.exports = MobileFriendly;
diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js
index 5a8370e57..78f30a9b2 100644
--- a/lighthouse-core/config/default-config.js
+++ b/lighthouse-core/config/default-config.js
@@ -256,7 +256,6 @@ const defaultConfig = {
     'seo/hreflang',
     'seo/plugins',
     'seo/canonical',
-    'seo/manual/mobile-friendly',
     'seo/manual/structured-data',
   ],
 
@@ -466,7 +465,6 @@ const defaultConfig = {
         {id: 'plugins', weight: 1, group: 'seo-content'},
         {id: 'tap-targets', weight: 1, group: 'seo-mobile'},
         // Manual audits
-        {id: 'mobile-friendly', weight: 0},
         {id: 'structured-data', weight: 0},
       ],
     },
diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json
index 97a1c102e..865e6e180 100644
--- a/proto/sample_v2_round_trip.json
+++ b/proto/sample_v2_round_trip.json
@@ -1606,13 +1606,6 @@
             "scoreDisplayMode": "informative", 
             "title": "Metrics"
         }, 
-        "mobile-friendly": {
-            "description": "Take the [Mobile-Friendly Test](https://search.google.com/test/mobile-friendly) to check for audits not covered by Lighthouse, like sizing tap targets appropriately. [Learn more](https://developers.google.com/search/mobile-sites/).", 
-            "id": "mobile-friendly", 
-            "score": null, 
-            "scoreDisplayMode": "manual", 
-            "title": "Page is mobile friendly"
-        }, 
         "network-requests": {
             "description": "Lists the network requests that were made during page load.", 
             "details": {
@@ -3622,10 +3615,6 @@
                     "id": "tap-targets", 
                     "weight": 1.0
                 }, 
-                {
-                    "id": "mobile-friendly", 
-                    "weight": 0.0
-                }, 
                 {
                     "id": "structured-data", 
                     "weight": 0.0
@@ -4654,12 +4643,6 @@
                 "name": "lh:audit:canonical", 
                 "startTime": 0.0
             }, 
-            {
-                "duration": 100.0, 
-                "entryType": "measure", 
-                "name": "lh:audit:mobile-friendly", 
-                "startTime": 0.0
-            }, 
             {
                 "duration": 100.0, 
                 "entryType": "measure", 
diff --git a/third-party/snyk/snapshot.json b/third-party/snyk/snapshot.json
index 299aae90e..0c7924b62 100644
--- a/third-party/snyk/snapshot.json
+++ b/third-party/snyk/snapshot.json
@@ -52,7 +52,7 @@
       {"id":"npm:handlebars:20110425","severity":"medium","semver":{"vulnerable":["<=1.0.0-beta.3"]}}
     ],
     "highcharts":[
-      {"id":"npm:highcharts:20180225","severity":"low","semver":{"vulnerable":["<6.1.0"]}}
+      {"id":"npm:highcharts:20180225","severity":"high","semver":{"vulnerable":["<6.1.0"]}}
     ],
     "jquery":[
       {"id":"npm:jquery:20160529","severity":"low","semver":{"vulnerable":["=3.0.0-rc.1"]}},
@@ -81,7 +81,7 @@
     "moment":[
       {"id":"npm:moment:20170905","severity":"low","semver":{"vulnerable":["<2.19.3"]}},
       {"id":"npm:moment:20161019","severity":"medium","semver":{"vulnerable":["<2.15.2"]}},
-      {"id":"npm:moment:20160126","severity":"low","semver":{"vulnerable":["<=2.11.1"]}}
+      {"id":"npm:moment:20160126","severity":"medium","semver":{"vulnerable":["<=2.11.1"]}}
     ],
     "mustache":[
       {"id":"npm:mustache:20151207","severity":"medium","semver":{"vulnerable":["<2.2.1"]}},


Test patch:
diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
index 40666ee60..1f34fbe16 100644
--- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
+++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
@@ -369,9 +369,6 @@ Object {
     Object {
       "path": "seo/canonical",
     },
-    Object {
-      "path": "seo/manual/mobile-friendly",
-    },
     Object {
       "path": "seo/manual/structured-data",
     },
@@ -989,10 +986,6 @@ Object {
           "id": "tap-targets",
           "weight": 1,
         },
-        Object {
-          "id": "mobile-friendly",
-          "weight": 0,
-        },
         Object {
           "id": "structured-data",
           "weight": 0,
diff --git a/lighthouse-core/test/audits/font-display-test.js b/lighthouse-core/test/audits/font-display-test.js
index 6a9dadd0d..0d20f4951 100644
--- a/lighthouse-core/test/audits/font-display-test.js
+++ b/lighthouse-core/test/audits/font-display-test.js
@@ -37,11 +37,13 @@ describe('Performance: Font Display audit', () => {
       }
 
       @font-face {
+        font-display: 'optional'; // invalid quotes, should still fail for being invalid rule
         /* try up a directory with ' */
         src: url('../font-b.woff');
       }
 
       @font-face {
+        font-display: optional // missing a semi-colon, should still fail for being invalid block
         /* try no path with no quotes ' */
         src: url(font.woff);
       }
@@ -72,32 +74,32 @@ describe('Performance: Font Display audit', () => {
       {url: networkRecords[2].url, wastedMs: 1000},
     ];
     assert.strictEqual(result.rawValue, false);
-    assert.deepEqual(result.details.items, items);
+    expect(result.details.items).toEqual(items);
   });
 
   it('resolves URLs relative to stylesheet URL when available', async () => {
     stylesheet.header.sourceURL = 'https://cdn.example.com/foo/bar/file.css';
     stylesheet.content = `
       @font-face {
-        font-display: 'block';
+        font-display: block;
         /* try with " and same directory */
         src: url("./font-a.woff");
       }
 
       @font-face {
-        font-display: 'block';
+        font-display: block;
         /* try with " and site origin */
         src: url("https://example.com/foo/bar/document-font.woff");
       }
 
       @font-face {
-        font-display: 'fallback';
+        font-display: fallback;
         /* try up a directory with ' */
         src: url('../font-b.woff');
       }
 
       @font-face {
-        font-display: 'optional';
+        font-display: optional;
         /* try no path with no quotes ' */
         src: url(font.woff);
       }
@@ -128,25 +130,25 @@ describe('Performance: Font Display audit', () => {
 
     const result = await FontDisplayAudit.audit(getArtifacts(), context);
     assert.strictEqual(result.rawValue, true);
-    assert.deepEqual(result.details.items, []);
+    expect(result.details.items).toEqual([]);
   });
 
   it('passes when all fonts have a correct font-display rule', async () => {
     stylesheet.content = `
       @font-face {
-        font-display: 'block';
+        font-display: block;
         /* try with " */
         src: url("./font-a.woff");
       }
 
       @font-face {
-        font-display: 'fallback';
+        font-display: fallback;
         /* try up a directory with ' */
         src: url('../font-b.woff');
       }
 
       @font-face {
-        font-display: 'optional';
+        font-display: optional;
         /* try no path with no quotes ' */
         src: url(font.woff);
       }
@@ -172,7 +174,7 @@ describe('Performance: Font Display audit', () => {
 
     const result = await FontDisplayAudit.audit(getArtifacts(), context);
     assert.strictEqual(result.rawValue, true);
-    assert.deepEqual(result.details.items, []);
+    expect(result.details.items).toEqual([]);
   });
 
   it('should handle real-world font-face declarations', async () => {
@@ -212,7 +214,7 @@ describe('Performance: Font Display audit', () => {
 
     const result = await FontDisplayAudit.audit(getArtifacts(), context);
     assert.strictEqual(result.rawValue, false);
-    assert.deepEqual(result.details.items.map(item => item.url), [
+    expect(result.details.items.map(item => item.url)).toEqual([
       'https://edition.i.cdn.cnn.com/.a/fonts/cnn/3.7.2/cnnclock-black.woff2',
       'https://registry.api.cnn.io/assets/fave/fonts/2.0.15/cnnsans-bold.woff',
       // FontAwesome should pass
@@ -220,4 +222,36 @@ describe('Performance: Font Display audit', () => {
       'https://fonts.gstatic.com/s/lato/v14/S6u9w4BMUTPHh50XSwiPGQ3q5d0.woff2',
     ]);
   });
+
+  it('handles varied font-display declarations', async () => {
+    stylesheet.content = `
+      @font-face {
+        src: url(font-0.woff);
+        font-display: swap
+      }
+
+      @font-face {src: url(font-1.woff);font-display   : fallback
+      }
+
+      @font-face {
+        src: url(font-2.woff);
+        font-display:optional}
+
+      @font-face {
+        src: url(font-3.woff);
+        font-display    :  swap  ;  }
+
+      @font-face {src: url(font-4.woff);font-display:swap;}
+    `;
+
+    networkRecords = Array.from({length: 5}).map((_, idx) => ({
+      url: `https://example.com/foo/bar/font-${idx}.woff`,
+      endTime: 2, startTime: 1,
+      resourceType: 'Font',
+    }));
+
+    const result = await FontDisplayAudit.audit(getArtifacts(), context);
+    expect(result.details.items).toEqual([]);
+    assert.strictEqual(result.rawValue, true);
+  });
 });
diff --git a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
index 58b09939f..6569b4f98 100644
--- a/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
+++ b/lighthouse-core/test/gather/gatherers/seo/font-size-test.js
@@ -172,69 +172,63 @@ describe('Font size gatherer', () => {
 
     it('should identify inline styles', () => {
       const result = FontSizeGather.getEffectiveFontRule({inlineStyle});
-      expect(result).toMatchInlineSnapshot(`
-Object {
-  "cssProperties": Array [
-    Object {
-      "name": "font-size",
-      "value": "1em",
-    },
-  ],
-  "styleSheetId": 1,
-  "type": "Inline",
-}
-`);
+      expect(result).toEqual({
+        cssProperties: [
+          {
+            name: 'font-size',
+            value: '1em',
+          },
+        ],
+        styleSheetId: 1,
+        type: 'Inline',
+      });
     });
 
     it('should identify direct CSS rules', () => {
       const result = FontSizeGather.getEffectiveFontRule({matchedCSSRules});
-      expect(result).toMatchInlineSnapshot(`
-Object {
-  "cssProperties": Array [
-    Object {
-      "name": "font-size",
-      "value": "1em",
-    },
-  ],
-  "parentRule": Object {
-    "origin": "regular",
-    "selectors": Array [
-      Object {
-        "text": "html body *",
-      },
-      Object {
-        "text": "#main",
-      },
-    ],
-  },
-  "styleSheetId": 123,
-  "type": "Regular",
-}
-`);
+      expect(result).toEqual({
+        cssProperties: [
+          {
+            name: 'font-size',
+            value: '1em',
+          },
+        ],
+        parentRule: {
+          origin: 'regular',
+          selectors: [
+            {
+              text: 'html body *',
+            },
+            {
+              text: '#main',
+            },
+          ],
+        },
+        styleSheetId: 123,
+        type: 'Regular',
+      });
     });
 
     it('should identify inherited CSS rules', () => {
       const result = FontSizeGather.getEffectiveFontRule({inherited});
-      expect(result).toMatchInlineSnapshot(`
-Object {
-  "cssProperties": Array [
-    Object {
-      "name": "font-size",
-      "value": 12,
-    },
-  ],
-  "parentRule": Object {
-    "origin": "user-agent",
-    "selectors": Array [
-      Object {
-        "text": "body",
-      },
-    ],
-  },
-  "styleSheetId": undefined,
-  "type": "Regular",
-}
-`);
+      expect(result).toEqual({
+        cssProperties: [
+          {
+            name: 'font-size',
+            value: 12,
+          },
+        ],
+        parentRule: {
+          origin: 'user-agent',
+          selectors: [
+            {
+              text: 'body',
+            },
+          ],
+        },
+        styleSheetId: undefined,
+        type: 'Regular',
+      });
     });
 
     it('should respect precendence', () => {
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index fd7b349a2..61ad1078c 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -3140,14 +3140,6 @@
       "scoreDisplayMode": "notApplicable",
       "rawValue": true
     },
-    "mobile-friendly": {
-      "id": "mobile-friendly",
-      "title": "Page is mobile friendly",
-      "description": "Take the [Mobile-Friendly Test](https://search.google.com/test/mobile-friendly) to check for audits not covered by Lighthouse, like sizing tap targets appropriately. [Learn more](https://developers.google.com/search/mobile-sites/).",
-      "score": null,
-      "scoreDisplayMode": "manual",
-      "rawValue": false
-    },
     "structured-data": {
       "id": "structured-data",
       "title": "Structured data is valid",
@@ -3726,10 +3718,6 @@
           "weight": 1,
           "group": "seo-mobile"
         },
-        {
-          "id": "mobile-friendly",
-          "weight": 0
-        },
         {
           "id": "structured-data",
           "weight": 0
@@ -4800,12 +4788,6 @@
         "duration": 100,
         "entryType": "measure"
       },
-      {
-        "startTime": 0,
-        "name": "lh:audit:mobile-friendly",
-        "duration": 100,
-        "entryType": "measure"
-      },
       {
         "startTime": 0,
         "name": "lh:audit:structured-data",