Repository: GoogleChrome/lighthouse
Pull request #9030 (2019-05-22 23:36:55)

renderer: fix word break issue for display text, move metric disclaimer
Before:

![image](https://user-images.githubusercontent.com/4071474/58215628-a45a1980-7caf-11e9-80a5-709fac78d98a.png)

![image](https://user-images.githubusercontent.com/4071474/58215591-6eb53080-7caf-11e9-9c68-723583154df7.png)

![image](https://user-images.githubusercontent.com/4071474/58215600-796fc580-7caf-11e9-8811-ce2e3ae75831.png)


After:

![image](https://user-images.githubusercontent.com/4071474/58215636-b20f9f00-7caf-11e9-9904-2831c2e483f7.png)

![image](https://user-images.githubusercontent.com/4071474/58215584-64933200-7caf-11e9-8fd0-b4e6492adc8a.png)

![image](https://user-images.githubusercontent.com/4071474/58215610-84c2f100-7caf-11e9-87e7-321cf91c3a3c.png)

See #9026


Patch:
diff --git a/docs/throttling.md b/docs/throttling.md
index c54b162cd..c29cf95a1 100644
--- a/docs/throttling.md
+++ b/docs/throttling.md
@@ -53,6 +53,6 @@ comcast --latency=150 --target-bw=1638
 # Run Lighthouse with its own throttling disabled
 lighthouse --throttling.requestLatencyMs=0 --throttling.downloadThroughputKbps=0 --throttling.uploadThroughputKbps=0 # ...
 
-# Disable the traffic throttling once you see "Retrieving trace"
+# Disable the traffic throttling once you see "Gathering trace"
 comcast --stop
 ```
diff --git a/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js b/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
index 324c38bde..4e96e0397 100644
--- a/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
+++ b/lighthouse-core/audits/byte-efficiency/uses-responsive-images.js
@@ -94,19 +94,22 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
     const warnings = [];
     /** @type {Map<string, LH.Audit.ByteEfficiencyItem>} */
     const resultsMap = new Map();
-    images.forEach(image => {
-      // TODO: give SVG a free pass until a detail per pixel metric is available
-      if (!image.resourceSize || image.mimeType === 'image/svg+xml') {
-        return;
+    for (const image of images) {
+      // Ignore images without resource size information.
+      // Give SVG a free pass because creating a "responsive" SVG is of questionable value.
+      // Ignore CSS images because it's difficult to determine what is a spritesheet,
+      // and the reward-to-effort ratio for responsive CSS images is quite low https://css-tricks.com/responsive-images-css/.
+      if (!image.resourceSize || image.mimeType === 'image/svg+xml' || image.isCss) {
+        continue;
       }
 
       const processed = UsesResponsiveImages.computeWaste(image, DPR);
-      if (!processed) return;
+      if (!processed) continue;
 
       if (processed instanceof Error) {
         warnings.push(processed.message);
         Sentry.captureException(processed, {tags: {audit: this.meta.id}, level: 'warning'});
-        return;
+        continue;
       }
 
       // Don't warn about an image that was later used appropriately
@@ -114,7 +117,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
       if (!existing || existing.wastedBytes > processed.wastedBytes) {
         resultsMap.set(processed.url, processed);
       }
-    });
+    }
 
     const items = Array.from(resultsMap.values())
         .filter(item => item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES);
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index 9c637cfcb..46388f345 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -228,14 +228,14 @@ class GatherRunner {
 
     let trace;
     if (passConfig.recordTrace) {
-      const status = {msg: 'Retrieving trace', id: `lh:gather:getTrace`};
+      const status = {msg: 'Gathering trace', id: `lh:gather:getTrace`};
       log.time(status);
       trace = await driver.endTrace();
       log.timeEnd(status);
     }
 
     const status = {
-      msg: 'Retrieving devtoolsLog & network records',
+      msg: 'Gathering devtoolsLog & network records',
       id: `lh:gather:getDevtoolsLog`,
     };
     log.time(status);
@@ -265,7 +265,7 @@ class GatherRunner {
       // Abuse the passContext to pass through gatherer options
       passContext.options = gathererDefn.options || {};
       const status = {
-        msg: `Retrieving setup: ${gatherer.name}`,
+        msg: `Gathering setup: ${gatherer.name}`,
         id: `lh:gather:beforePass:${gatherer.name}`,
       };
       log.time(status, 'verbose');
@@ -295,7 +295,7 @@ class GatherRunner {
       // Abuse the passContext to pass through gatherer options
       passContext.options = gathererDefn.options || {};
       const status = {
-        msg: `Retrieving in-page: ${gatherer.name}`,
+        msg: `Gathering in-page: ${gatherer.name}`,
         id: `lh:gather:pass:${gatherer.name}`,
       };
       log.time(status);
@@ -327,7 +327,7 @@ class GatherRunner {
     for (const gathererDefn of gatherers) {
       const gatherer = gathererDefn.instance;
       const status = {
-        msg: `Retrieving: ${gatherer.name}`,
+        msg: `Gathering: ${gatherer.name}`,
         id: `lh:gather:afterPass:${gatherer.name}`,
       };
       log.time(status);
diff --git a/lighthouse-core/gather/gatherers/image-elements.js b/lighthouse-core/gather/gatherers/image-elements.js
index 4722ebe34..77b7e566d 100644
--- a/lighthouse-core/gather/gatherers/image-elements.js
+++ b/lighthouse-core/gather/gatherers/image-elements.js
@@ -15,30 +15,31 @@ const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars
 
 /* global window, getElementsInDocument, Image */
 
-/** @return {Array<LH.Artifacts.ImageElement>} */
+
+/** @param {Element} element */
 /* istanbul ignore next */
-function collectImageElementInfo() {
-  /** @param {Element} element */
-  function getClientRect(element) {
-    const clientRect = element.getBoundingClientRect();
-    return {
-      // Just grab the DOMRect properties we want, excluding x/y/width/height
-      top: clientRect.top,
-      bottom: clientRect.bottom,
-      left: clientRect.left,
-      right: clientRect.right,
-    };
-  }
+function getClientRect(element) {
+  const clientRect = element.getBoundingClientRect();
+  return {
+    // Just grab the DOMRect properties we want, excluding x/y/width/height
+    top: clientRect.top,
+    bottom: clientRect.bottom,
+    left: clientRect.left,
+    right: clientRect.right,
+  };
+}
 
-  /** @type {Array<Element>} */
-  // @ts-ignore - added by getElementsInDocumentFnString
-  const allElements = getElementsInDocument();
+/**
+ * @param {Array<Element>} allElements
+ * @return {Array<LH.Artifacts.ImageElement>}
+ */
+/* istanbul ignore next */
+function getHTMLImages(allElements) {
   const allImageElements = /** @type {Array<HTMLImageElement>} */ (allElements.filter(element => {
     return element.localName === 'img';
   }));
 
-  /** @type {Array<LH.Artifacts.ImageElement>} */
-  const htmlImages = allImageElements.map(element => {
+  return allImageElements.map(element => {
     const computedStyle = window.getComputedStyle(element);
     return {
       // currentSrc used over src to get the url as determined by the browser
@@ -57,30 +58,30 @@ function collectImageElementInfo() {
       ),
     };
   });
+}
 
+/**
+ * @param {Array<Element>} allElements
+ * @return {Array<LH.Artifacts.ImageElement>}
+ */
+/* istanbul ignore next */
+function getCSSImages(allElements) {
   // Chrome normalizes background image style from getComputedStyle to be an absolute URL in quotes.
   // Only match basic background-image: url("http://host/image.jpeg") declarations
   const CSS_URL_REGEX = /^url\("([^"]+)"\)$/;
-  // Only find images that aren't specifically scaled
-  const CSS_SIZE_REGEX = /(auto|contain|cover)/;
 
-  const cssImages = allElements.reduce((images, element) => {
+  /** @type {Array<LH.Artifacts.ImageElement>} */
+  const images = [];
+
+  for (const element of allElements) {
     const style = window.getComputedStyle(element);
-    if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage) ||
-        !style.backgroundSize || !CSS_SIZE_REGEX.test(style.backgroundSize)) {
-      return images;
-    }
+    // If the element didn't have a CSS background image, we're not interested.
+    if (!style.backgroundImage || !CSS_URL_REGEX.test(style.backgroundImage)) continue;
 
     const imageMatch = style.backgroundImage.match(CSS_URL_REGEX);
     // @ts-ignore test() above ensures that there is a match.
     const url = imageMatch[1];
 
-    // Heuristic to filter out sprite sheets
-    const differentImages = images.filter(image => image.src !== url);
-    if (images.length - differentImages.length > 2) {
-      return differentImages;
-    }
-
     images.push({
       src: url,
       displayedWidth: element.clientWidth,
@@ -94,11 +95,18 @@ function collectImageElementInfo() {
       usesObjectFit: false,
       resourceSize: 0, // this will get overwritten below
     });
+  }
 
-    return images;
-  }, /** @type {Array<LH.Artifacts.ImageElement>} */ ([]));
+  return images;
+}
 
-  return htmlImages.concat(cssImages);
+/** @return {Array<LH.Artifacts.ImageElement>} */
+/* istanbul ignore next */
+function collectImageElementInfo() {
+  /** @type {Array<Element>} */
+  // @ts-ignore - added by getElementsInDocumentFnString
+  const allElements = getElementsInDocument();
+  return getHTMLImages(allElements).concat(getCSSImages(allElements));
 }
 
 /**
@@ -160,16 +168,23 @@ class ImageElements extends Gatherer {
 
     const expression = `(function() {
       ${pageFunctions.getElementsInDocumentString}; // define function on page
-      return (${collectImageElementInfo.toString()})();
+      ${getClientRect.toString()};
+      ${getHTMLImages.toString()};
+      ${getCSSImages.toString()};
+      ${collectImageElementInfo.toString()};
+
+      return collectImageElementInfo();
     })()`;
 
     /** @type {Array<LH.Artifacts.ImageElement>} */
     const elements = await driver.evaluateAsync(expression);
 
+    /** @type {Array<LH.Artifacts.ImageElement>} */
     const imageUsage = [];
     const top50Images = Object.values(indexedNetworkRecords)
       .sort((a, b) => b.resourceSize - a.resourceSize)
       .slice(0, 50);
+
     for (let element of elements) {
       // Pull some of our information directly off the network record.
       const networkRecord = indexedNetworkRecords[element.src] || {};
diff --git a/lighthouse-core/report/html/renderer/performance-category-renderer.js b/lighthouse-core/report/html/renderer/performance-category-renderer.js
index 8ca8c4539..58083b5ee 100644
--- a/lighthouse-core/report/html/renderer/performance-category-renderer.js
+++ b/lighthouse-core/report/html/renderer/performance-category-renderer.js
@@ -147,7 +147,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
 
     // 'Values are estimated and may vary' is used as the category description for PSI
     if (environment !== 'PSI') {
-      const estValuesEl = this.dom.createChildOf(metricsColumn1El, 'div', 'lh-metrics__disclaimer');
+      const estValuesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics__disclaimer');
       estValuesEl.textContent = Util.UIStrings.varianceDisclaimer;
     }
 
diff --git a/lighthouse-core/report/html/report-styles.css b/lighthouse-core/report/html/report-styles.css
index be3769bc1..d9ddda523 100644
--- a/lighthouse-core/report/html/report-styles.css
+++ b/lighthouse-core/report/html/report-styles.css
@@ -478,7 +478,7 @@
   margin-right: 0;
 }
 
-.lh-audit__display-text {
+.lh-audit__title-and-text {
   flex: 1;
 }
 
diff --git a/lighthouse-core/report/html/templates.html b/lighthouse-core/report/html/templates.html
index cc8d7c798..7032ea4b8 100644
--- a/lighthouse-core/report/html/templates.html
+++ b/lighthouse-core/report/html/templates.html
@@ -94,8 +94,10 @@ limitations under the License.
       <summary>
         <div class="lh-audit__header lh-expandable-details__summary">
           <span class="lh-audit__score-icon"></span>
-          <span class="lh-audit__title"></span>
-          <span class="lh-audit__display-text"></span>
+          <span class="lh-audit__title-and-text">
+            <span class="lh-audit__title"></span>
+            <span class="lh-audit__display-text"></span>
+          </span>
           <div class="lh-chevron-container"></div>
         </div>
       </summary>
diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js
index 5795cc8ae..17522fa1b 100644
--- a/lighthouse-core/runner.js
+++ b/lighthouse-core/runner.js
@@ -274,7 +274,7 @@ class Runner {
   static async _runAudit(auditDefn, artifacts, sharedAuditContext) {
     const audit = auditDefn.implementation;
     const status = {
-      msg: `Evaluating: ${i18n.getFormatted(audit.meta.title, 'en-US')}`,
+      msg: `Auditing: ${i18n.getFormatted(audit.meta.title, 'en-US')}`,
       id: `lh:audit:${audit.meta.id}`,
     };
     log.time(status);


Test patch:
diff --git a/lighthouse-cli/test/fixtures/byte-efficiency/tester.html b/lighthouse-cli/test/fixtures/byte-efficiency/tester.html
index 17e1372dc..15df1431b 100644
--- a/lighthouse-cli/test/fixtures/byte-efficiency/tester.html
+++ b/lighthouse-cli/test/fixtures/byte-efficiency/tester.html
@@ -102,7 +102,7 @@ setTimeout(() => {
 
   <!-- PASS(optimized): image is JPEG optimized -->
   <!-- PASS(webp): image is WebP optimized -->
-  <!-- PASSWARN(responsive): image is 25% used at DPR 2 (but small savings) -->
+  <!-- FAIL(responsive): image is 25% used at DPR 2 (but small savings) -->
   <!-- FAIL(offscreen): image is offscreen -->
   <img style="margin-top: 1000px; width: 120px; height: 80px;" src="lighthouse-480x320.webp">
 
@@ -130,15 +130,15 @@ setTimeout(() => {
   <!-- PASS(offscreen): image is onscreen -->
   <img class="onscreen" src="lighthouse-320x212-poor.jpg?duplicate">
 
-  <!-- PASS(optimized): image is WebP -->
-  <!-- PASS(webp): image is WebP optimized -->
-  <!-- FAIL(responsive): image is used at 1/16 size -->
+  <!-- PASS(optimized): image is JPEG optimized -->
+  <!-- FAIL(webp): image is not WebP optimized -->
+  <!-- PASS(responsive): image is in CSS -->
   <!-- PASS(offscreen): image is onscreen -->
-  <div class="onscreen" style="width: 120px; height: 80px; background: 50% 50% url(lighthouse-480x320.webp?css);"></div>
+  <div class="onscreen" style="width: 120px; height: 80px; background: 50% 50% url(lighthouse-480x320.jpg?css);"></div>
 
   <!-- PASS(optimized): image is JPEG optimized -->
   <!-- FAIL(webp): image is not WebP optimized -->
-  <!-- PASS(responsive): image is used numerous times in sprite-fashion -->
+  <!-- PASS(responsive): image is in CSS -->
   <!-- PASS(offscreen): image is onscreen -->
   <div class="onscreen" style="width: 30px; height: 30px; background: 0% 50% url(lighthouse-480x320.jpg?sprite);"></div>
   <div class="onscreen" style="width: 30px; height: 30px; background: 25% 50% url(lighthouse-480x320.jpg?sprite);"></div>
diff --git a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
index 3da2d9062..649b674e4 100644
--- a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
+++ b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
@@ -83,7 +83,7 @@ module.exports = [
           details: {
             overallSavingsBytes: '>60000',
             items: {
-              length: 4,
+              length: 5,
             },
           },
         },
@@ -106,14 +106,14 @@ module.exports = [
           },
         },
         'uses-responsive-images': {
-          displayValue: 'Potential savings of 75\xa0KB',
+          displayValue: 'Potential savings of 69\xa0KB',
           details: {
-            overallSavingsBytes: '>75000',
-            items: [
-              {wastedPercent: '<60'},
-              {wastedPercent: '<60'},
-              {wastedPercent: '<60'},
-            ],
+            overallSavingsBytes: '>65000',
+            items: {
+              0: {wastedPercent: '<60'},
+              1: {wastedPercent: '<60'},
+              length: 2,
+            },
           },
         },
       },
diff --git a/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js b/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js
index 68aec09f8..a10e7b697 100644
--- a/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js
+++ b/lighthouse-core/test/audits/byte-efficiency/uses-responsive-images-test.js
@@ -147,6 +147,37 @@ describe('Page uses responsive images', () => {
     assert.equal(auditResult.items.length, 0);
   });
 
+  it('ignores CSS', () => {
+    const urlA = 'https://google.com/logo.png';
+    const naturalSizeA = generateSize(450, 450, 'natural');
+    const recordA = generateRecord(100, 300);
+
+    const auditResult = UsesResponsiveImagesAudit.audit_({
+      ViewportDimensions: {devicePixelRatio: 1},
+      ImageElements: [
+        {...generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA), isCss: true},
+      ],
+    });
+
+    assert.equal(auditResult.items.length, 0);
+  });
+
+  it('handles failure', () => {
+    const urlA = 'https://google.com/logo.png';
+    const naturalSizeA = generateSize(NaN, 450, 'natural');
+    const recordA = generateRecord(100, 300);
+
+    const auditResult = UsesResponsiveImagesAudit.audit_({
+      ViewportDimensions: {devicePixelRatio: 1},
+      ImageElements: [
+        generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA),
+      ],
+    });
+
+    assert.equal(auditResult.items.length, 0);
+    assert.equal(auditResult.warnings.length, 1);
+  });
+
   it('de-dupes images', () => {
     const urlA = 'https://google.com/logo.png';
     const naturalSizeA = generateSize(450, 450, 'natural');