core(font-display): limit false positives

lighthouse-core/audits/font-display.js

@@ -44,10 +44,11 @@ class FontDisplay extends Audit {
 
   /**
    * @param {LH.Artifacts} artifacts
+   * @return {Set<string>}
    */
   static findPassingFontDisplayDeclarations(artifacts) {
     /** @type {Set<string>} */
-    const passingURLs = new Set();
+    const failingURLs = new Set();
 
     // Go through all the stylesheets to find all @font-face declarations
     for (const stylesheet of artifacts.CSSUsage.stylesheets) {
@@ -59,14 +60,16 @@ class FontDisplay extends Audit {
       for (const declaration of fontFaceDeclarations) {
         // 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[1]);
-        if (!hasPassingFontDisplay) continue;
-
-        // If it's passing, we'll try to find the URL it's referencing.
+        const fontDisplayMatch = declaration.match(/font-display\s*:\s*(\w+)\s*(;|\})/);
+        const rawFontDisplay = (fontDisplayMatch && fontDisplayMatch[1]) || '';
+        const hasPassingFontDisplay = PASSING_FONT_DISPLAY_REGEX.test(rawFontDisplay);
+        // If they have one of the passing font-display values, it's fine; bail
+        if (rawFontDisplay && hasPassingFontDisplay) continue;
+
+        // If they didn't have a font-display value or it wasn't set it a passing value, we need
+        // to flag it as a failing font...
+
+        // We'll try to find the URL it's referencing.
         const rawFontURLs = declaration.match(CSS_URL_GLOBAL_REGEX);
         // If no URLs, we can't really do anything; bail
         if (!rawFontURLs) continue;
@@ -82,21 +85,22 @@ class FontDisplay extends Audit {
 
             return s;
           });
-        // Convert the relative CSS URL to an absolute URL and add it to the passing set
+
+        // Convert the relative CSS URL to an absolute URL and add it to the failing set.
         for (const relativeURL of relativeURLs) {
           try {
             const relativeRoot = URL.isValid(stylesheet.header.sourceURL) ?
               stylesheet.header.sourceURL : artifacts.URL.finalUrl;
             const absoluteURL = new URL(relativeURL, relativeRoot);
-            passingURLs.add(absoluteURL.href);
+            failingURLs.add(absoluteURL.href);
           } catch (err) {
             Sentry.captureException(err, {tags: {audit: this.meta.id}});
           }
         }
       }
     }
 
-    return passingURLs;
+    return failingURLs;
   }
 
   /**
@@ -107,13 +111,13 @@ class FontDisplay extends Audit {
   static async audit(artifacts, context) {
     const devtoolsLogs = artifacts.devtoolsLogs[this.DEFAULT_PASS];
     const networkRecords = await NetworkRecords.request(devtoolsLogs, context);
-    const passingFontURLs = FontDisplay.findPassingFontDisplayDeclarations(artifacts);
+    const failingFontURLs = FontDisplay.findPassingFontDisplayDeclarations(artifacts);
 
     const results = networkRecords
       // Find all fonts...
       .filter(record => record.resourceType === 'Font')
-      // ...that don't have a passing font-display value
-      .filter(record => !passingFontURLs.has(record.url))
+      // ...that have a failing font-display value
+      .filter(record => failingFontURLs.has(record.url))
       // ...and that aren't data URLs, the blocking concern doesn't really apply
       .filter(record => !/^data:/.test(record.url))
       .filter(record => !/^blob:/.test(record.url))

lighthouse-core/test/audits/font-display-test.js

@@ -282,4 +282,19 @@ describe('Performance: Font Display audit', () => {
     expect(result.details.items).toEqual([]);
     assert.strictEqual(result.score, 1);
   });
+
+  it('shuold not flag a URL for which there is not @font-face at all', async () => {
+    // Sometimes the content does not come through, see https://github.com/GoogleChrome/lighthouse/issues/8493
+    stylesheet.content = ``;
+
+    networkRecords = [{
+      url: `https://example.com/foo/bar/font-0.woff`,
+      endTime: 2, startTime: 1,
+      resourceType: 'Font',
+    }];
+
+    const result = await FontDisplayAudit.audit(getArtifacts(), context);
+    expect(result.details.items).toEqual([]);
+    assert.strictEqual(result.score, 1);
+  });
 });