Repository: GoogleChrome/lighthouse
Pull request #6688 (2018-11-29 23:23:44)

report: larger vertical margins for audit-group summaries
part of #6395

Widens the vertical margins around group header summaries to match the specced design (see discussion starting with https://github.com/GoogleChrome/lighthouse/issues/6395#issuecomment-439259634).

Before:

<img width="827" alt="screen shot 2018-11-29 at 15 19 05" src="https://user-images.githubusercontent.com/316891/49258415-3c0a3980-f3ea-11e8-8d46-08c1361ce5cf.png">

After:

<img width="832" alt="screen shot 2018-11-29 at 15 21 25" src="https://user-images.githubusercontent.com/316891/49258489-77a50380-f3ea-11e8-99b5-60c700414f67.png">

Things are a little spread out for e.g. the a11y groups:

<img width="826" alt="screen shot 2018-11-29 at 15 22 46" src="https://user-images.githubusercontent.com/316891/49258545-a9b66580-f3ea-11e8-926e-71527af66946.png">

but it's not egregious

patrickhulce: Does this go for DevTools too? I guess we're already different enough from the rest of the aesthetic what's a little more spacious layout 😆 
brendankenny: > Does this go for DevTools too?

ha, ninjaed :)

It's possible it's still too spread out for devtools, but maybe we can tweak? It was `margin-bottom` 4px before, now it's 12px.



Patch:
diff --git a/lighthouse-cli/cli-flags.js b/lighthouse-cli/cli-flags.js
index ebfc1f00b..c6ca5dd57 100644
--- a/lighthouse-cli/cli-flags.js
+++ b/lighthouse-cli/cli-flags.js
@@ -90,7 +90,7 @@ function getFlags(manualArgv) {
         'config-path': 'The path to the config JSON.',
         'preset': 'Use a built-in configuration.',
         'chrome-flags':
-            `Custom flags to pass to Chrome (space-delimited). For a full list of flags, see http://bit.ly/chrome-flags
+            `Custom flags to pass to Chrome (space-delimited). For a full list of flags, see https://bit.ly/chrome-flags
             Additionally, use the CHROME_PATH environment variable to use a specific Chrome binary. Requires Chromium version 66.0 or later. If omitted, any detected Chrome Canary or Chrome stable will be used.`,
         'hostname': 'The hostname to use for the debugging protocol.',
         'port': 'The port to use for the debugging protocol. Use 0 for a random port',
diff --git a/lighthouse-core/gather/gatherers/accessibility.js b/lighthouse-core/gather/gatherers/accessibility.js
index a0226a7da..0af785460 100644
--- a/lighthouse-core/gather/gatherers/accessibility.js
+++ b/lighthouse-core/gather/gatherers/accessibility.js
@@ -5,7 +5,7 @@
  */
 'use strict';
 
-/* global window, document, Node, getOuterHTMLSnippet */
+/* global window, document, getOuterHTMLSnippet, getNodePath */
 
 const Gatherer = require('./gatherer');
 const fs = require('fs');
@@ -45,6 +45,7 @@ function runA11yChecks() {
     // Augment the node objects with outerHTML snippet & custom path string
     // @ts-ignore
     axeResult.violations.forEach(v => v.nodes.forEach(node => {
+      // @ts-ignore - getNodePath put into scope via stringification
       node.path = getNodePath(node.element);
       // @ts-ignore - getOuterHTMLSnippet put into scope via stringification
       node.snippet = getOuterHTMLSnippet(node.element);
@@ -56,37 +57,6 @@ function runA11yChecks() {
     axeResult = {violations: axeResult.violations, notApplicable: axeResult.inapplicable};
     return axeResult;
   });
-
-  /**
-   * Adapted from DevTools' SDK.DOMNode.prototype.path
-   *   https://github.com/ChromeDevTools/devtools-frontend/blob/7a2e162ddefd/front_end/sdk/DOMModel.js#L530-L552
-   * TODO: Doesn't handle frames or shadow roots...
-   * @param {Node} node
-   */
-  function getNodePath(node) {
-    /** @param {Node} node */
-    function getNodeIndex(node) {
-      let index = 0;
-      let prevNode;
-      while (prevNode = node.previousSibling) {
-        node = prevNode;
-        // skip empty text nodes
-        if (node.nodeType === Node.TEXT_NODE && node.textContent &&
-          node.textContent.trim().length === 0) continue;
-        index++;
-      }
-      return index;
-    }
-
-    const path = [];
-    while (node && node.parentNode) {
-      const index = getNodeIndex(node);
-      path.push([index, node.nodeName]);
-      node = node.parentNode;
-    }
-    path.reverse();
-    return path.join(',');
-  }
 }
 
 class Accessibility extends Gatherer {
@@ -98,6 +68,7 @@ class Accessibility extends Gatherer {
     const driver = passContext.driver;
     const expression = `(function () {
       ${pageFunctions.getOuterHTMLSnippetString};
+      ${pageFunctions.getNodePathString};
       ${axeLibSource};
       return (${runA11yChecks.toString()}());
     })()`;
diff --git a/lighthouse-core/lib/minification-estimator.js b/lighthouse-core/lib/minification-estimator.js
index 88686e1ca..256ed1565 100644
--- a/lighthouse-core/lib/minification-estimator.js
+++ b/lighthouse-core/lib/minification-estimator.js
@@ -47,6 +47,7 @@ function computeTokenLength(content, features) {
   let isInLicenseComment = false;
   let isInString = false;
   let isInRegex = false;
+  let isInRegexCharacterClass = false;
   let stringOpenChar = null;
 
   for (let i = 0; i < content.length; i++) {
@@ -94,7 +95,13 @@ function computeTokenLength(content, features) {
         // Skip over any escaped characters
         totalTokenLength++;
         i++;
-      } else if (char === '/') {
+      } else if (char === '[') {
+        // Register that we're entering a character class so we don't leave the regex prematurely
+        isInRegexCharacterClass = true;
+      } else if (char === ']' && isInRegexCharacterClass) {
+        // Register that we're exiting the character class
+        isInRegexCharacterClass = false;
+      } else if (char === '/' && !isInRegexCharacterClass) {
         // End the string when we hit the regex close character
         isInRegex = false;
         // console.log(i, 'leaving regex', char)
diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js
index 960476f15..9e9b79d69 100644
--- a/lighthouse-core/lib/page-functions.js
+++ b/lighthouse-core/lib/page-functions.js
@@ -6,7 +6,7 @@
 // @ts-nocheck
 'use strict';
 
-/* global window document */
+/* global window document Node */
 
 /**
  * Helper functions that are passed by `toString()` by Driver to be evaluated in target page.
@@ -119,7 +119,7 @@ function getOuterHTMLSnippet(element, ignoreAttrs = []) {
     clone.removeAttribute(attribute);
   });
 
-  const reOpeningTag = /^.*?>/;
+  const reOpeningTag = /^[\s\S]*?>/;
   const match = clone.outerHTML.match(reOpeningTag);
 
   return (match && match[0]) || '';
@@ -153,6 +153,71 @@ function ultradumbBenchmark() {
   return Math.round(iterations / durationInSeconds);
 }
 
+/**
+ * Adapted from DevTools' SDK.DOMNode.prototype.path
+ *   https://github.com/ChromeDevTools/devtools-frontend/blob/7a2e162ddefd/front_end/sdk/DOMModel.js#L530-L552
+ * TODO: Doesn't handle frames or shadow roots...
+ * @param {Node} node
+ */
+/* istanbul ignore next */
+function getNodePath(node) {
+  /** @param {Node} node */
+  function getNodeIndex(node) {
+    let index = 0;
+    let prevNode;
+    while (prevNode = node.previousSibling) {
+      node = prevNode;
+      // skip empty text nodes
+      if (node.nodeType === Node.TEXT_NODE && node.textContent &&
+        node.textContent.trim().length === 0) continue;
+      index++;
+    }
+    return index;
+  }
+
+  const path = [];
+  while (node && node.parentNode) {
+    const index = getNodeIndex(node);
+    path.push([index, node.nodeName]);
+    node = node.parentNode;
+  }
+  path.reverse();
+  return path.join(',');
+}
+
+/**
+ * @param {Element} node
+ * @returns {string}
+ */
+/* istanbul ignore next */
+function getNodeSelector(node) {
+  /**
+   * @param {Element} node
+   */
+  function getSelectorPart(node) {
+    let part = node.tagName.toLowerCase();
+    if (node.id) {
+      part += '#' + node.id;
+    } else if (node.classList.length > 0) {
+      part += '.' + node.classList[0];
+    }
+    return part;
+  }
+
+  const parts = [];
+  while (parts.length < 4) {
+    parts.unshift(getSelectorPart(node));
+    if (!node.parentElement) {
+      break;
+    }
+    node = node.parentElement;
+    if (node.tagName === 'HTML') {
+      break;
+    }
+  }
+  return parts.join(' > ');
+}
+
 module.exports = {
   wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(),
   registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(),
@@ -162,4 +227,7 @@ module.exports = {
   getOuterHTMLSnippet: getOuterHTMLSnippet,
   ultradumbBenchmark: ultradumbBenchmark,
   ultradumbBenchmarkString: ultradumbBenchmark.toString(),
+  getNodePathString: getNodePath.toString(),
+  getNodeSelectorString: getNodeSelector.toString(),
+  getNodeSelector: getNodeSelector,
 };
diff --git a/lighthouse-core/lib/rect-helpers.js b/lighthouse-core/lib/rect-helpers.js
new file mode 100644
index 000000000..9deb75bbf
--- /dev/null
+++ b/lighthouse-core/lib/rect-helpers.js
@@ -0,0 +1,258 @@
+/**
+ * @license Copyright 2018 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';
+
+/**
+ * @param {LH.Artifacts.Rect} rect
+ * @param {{x:number, y:number}} point
+ */
+// We sometimes run this as a part of a gatherer script injected into the page, so prevent
+// renaming the function for code coverage.
+/* istanbul ignore next */
+function rectContainsPoint(rect, {x, y}) {
+  return rect.left <= x && rect.right >= x && rect.top <= y && rect.bottom >= y;
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect1
+ * @param {LH.Artifacts.Rect} rect2
+ */
+// We sometimes run this as a part of a gatherer script injected into the page, so prevent
+// renaming the function for code coverage.
+/* istanbul ignore next */
+function rectContains(rect1, rect2) {
+  return (
+    // top left corner
+    rectContainsPoint(rect1, {
+      x: rect2.left,
+      y: rect2.top,
+    }) &&
+    // top right corner
+    rectContainsPoint(rect1, {
+      x: rect2.right,
+      y: rect2.top,
+    }) &&
+    // bottom left corner
+    rectContainsPoint(rect1, {
+      x: rect2.left,
+      y: rect2.bottom,
+    }) &&
+    // bottom right corner
+    rectContainsPoint(rect1, {
+      x: rect2.right,
+      y: rect2.bottom,
+    })
+  );
+}
+
+
+const rectContainsString = `
+  ${rectContainsPoint.toString()}
+  ${rectContains.toString()};
+`;
+
+/**
+ * @param {LH.Artifacts.Rect[]} rects
+ * @returns {LH.Artifacts.Rect[]}
+ */
+function filterOutTinyRects(rects) {
+  return rects.filter(
+    rect => rect.width > 1 && rect.height > 1
+  );
+}
+
+/**
+ * @param {LH.Artifacts.Rect[]} rects
+ * @returns {LH.Artifacts.Rect[]}
+ */
+function filterOutRectsContainedByOthers(rects) {
+  const rectsToKeep = new Set(rects);
+
+  for (const rect of rects) {
+    for (const possiblyContainingRect of rects) {
+      if (rect === possiblyContainingRect) continue;
+      if (!rectsToKeep.has(possiblyContainingRect)) continue;
+      if (rectContains(possiblyContainingRect, rect)) {
+        rectsToKeep.delete(rect);
+        break;
+      }
+    }
+  }
+
+  return Array.from(rectsToKeep);
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect
+ */
+function getRectCenterPoint(rect) {
+  return {
+    x: rect.left + rect.width / 2,
+    y: rect.top + rect.height / 2,
+  };
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rectA
+ * @param {LH.Artifacts.Rect} rectB
+ * @returns {boolean}
+ */
+function rectsTouchOrOverlap(rectA, rectB) {
+  // https://stackoverflow.com/questions/2752349/fast-rectangle-to-rectangle-intersection
+  return (
+    rectA.left <= rectB.right &&
+    rectB.left <= rectA.right &&
+    rectA.top <= rectB.bottom &&
+    rectB.top <= rectA.bottom
+  );
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rectA
+ * @param {LH.Artifacts.Rect} rectB
+ */
+function getBoundingRect(rectA, rectB) {
+  const left = Math.min(rectA.left, rectB.left);
+  const right = Math.max(rectA.right, rectB.right);
+  const top = Math.min(rectA.top, rectB.top);
+  const bottom = Math.max(rectA.bottom, rectB.bottom);
+  return addRectWidthAndHeight({
+    left,
+    right,
+    top,
+    bottom,
+  });
+}
+
+/**
+ * @param {{left:number, top:number, right:number, bottom: number}} rect
+ * @return {LH.Artifacts.Rect}
+ */
+function addRectWidthAndHeight({left, top, right, bottom}) {
+  return {
+    left,
+    top,
+    right,
+    bottom,
+    width: right - left,
+    height: bottom - top,
+  };
+}
+
+/**
+ * @param {{x:number, y:number, width:number, height: number}} rect
+ * @return {LH.Artifacts.Rect}
+ */
+function addRectTopAndBottom({x, y, width, height}) {
+  return {
+    left: x,
+    top: y,
+    right: x + width,
+    bottom: y + height,
+    width,
+    height,
+  };
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect1
+ * @param {LH.Artifacts.Rect} rect2
+ */
+function getRectXOverlap(rect1, rect2) {
+  // https://stackoverflow.com/a/9325084/1290545
+  return Math.max(
+    0,
+    Math.min(rect1.right, rect2.right) - Math.max(rect1.left, rect2.left)
+  );
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect1
+ * @param {LH.Artifacts.Rect} rect2
+ */
+function getRectYOverlap(rect1, rect2) {
+  // https://stackoverflow.com/a/9325084/1290545
+  return Math.max(
+    0,
+    Math.min(rect1.bottom, rect2.bottom) - Math.max(rect1.top, rect2.top)
+  );
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect1
+ * @param {LH.Artifacts.Rect} rect2
+ */
+function getRectOverlapArea(rect1, rect2) {
+  return getRectXOverlap(rect1, rect2) * getRectYOverlap(rect1, rect2);
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect
+ * @param {number} centerRectSize
+ */
+function getRectAtCenter(rect, centerRectSize) {
+  return addRectWidthAndHeight({
+    left: rect.left + rect.width / 2 - centerRectSize / 2,
+    top: rect.top + rect.height / 2 - centerRectSize / 2,
+    right: rect.right - rect.width / 2 + centerRectSize / 2,
+    bottom: rect.bottom - rect.height / 2 + centerRectSize / 2,
+  });
+}
+
+/**
+ * @param {LH.Artifacts.Rect} rect
+ */
+function getRectArea(rect) {
+  return rect.width * rect.height;
+}
+
+/**
+ * @param {LH.Artifacts.Rect[]} rects
+ */
+function getLargestRect(rects) {
+  let largestRect = rects[0];
+  for (const rect of rects) {
+    if (getRectArea(rect) > getRectArea(largestRect)) {
+      largestRect = rect;
+    }
+  }
+  return largestRect;
+}
+
+/**
+ *
+ * @param {LH.Artifacts.Rect[]} rectListA
+ * @param {LH.Artifacts.Rect[]} rectListB
+ */
+function allRectsContainedWithinEachOther(rectListA, rectListB) {
+  for (const rectA of rectListA) {
+    for (const rectB of rectListB) {
+      if (!rectContains(rectA, rectB) && !rectContains(rectB, rectA)) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+module.exports = {
+  rectContainsPoint,
+  rectContains,
+  rectContainsString,
+  addRectWidthAndHeight,
+  addRectTopAndBottom,
+  getRectXOverlap,
+  getRectYOverlap,
+  getRectOverlapArea,
+  getRectAtCenter,
+  getLargestRect,
+  getRectCenterPoint,
+  getBoundingRect,
+  rectsTouchOrOverlap,
+  allRectsContainedWithinEachOther,
+  filterOutRectsContainedByOthers,
+  filterOutTinyRects,
+};
diff --git a/lighthouse-core/lib/tappable-rects.js b/lighthouse-core/lib/tappable-rects.js
new file mode 100644
index 000000000..118d87916
--- /dev/null
+++ b/lighthouse-core/lib/tappable-rects.js
@@ -0,0 +1,104 @@
+/**
+ * @license Copyright 2018 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 {
+  filterOutRectsContainedByOthers,
+  filterOutTinyRects,
+  rectsTouchOrOverlap,
+  rectContainsPoint,
+  getBoundingRect,
+  getRectCenterPoint,
+} = require('./rect-helpers');
+
+/**
+ * Merge client rects together and remove small ones. This may result in a larger overall
+ * size than that of the individual client rects.
+ * We use this to simulate a finger tap on those targets later on.
+ * @param {LH.Artifacts.Rect[]} clientRects
+ */
+function getTappableRectsFromClientRects(clientRects) {
+  // 1x1px rect shouldn't be reason to treat the rect as something the user should tap on.
+  // Often they're made invisble in some obscure way anyway, and only exist for e.g. accessibiliity.
+  clientRects = filterOutTinyRects(clientRects);
+  clientRects = filterOutRectsContainedByOthers(clientRects);
+  clientRects = mergeTouchingClientRects(clientRects);
+  return clientRects;
+}
+
+/**
+ * @param {number} a
+ * @param {number} b
+ */
+function almostEqual(a, b) {
+  // Sometimes a child will reach out of the parent by
+  // 1px or 2px, so be somewhat tolerant for merging
+  return Math.abs(a - b) <= 2;
+}
+
+/**
+ * Merge touching rects based on what appears as one tappable area to the user.
+ * @param {LH.Artifacts.Rect[]} clientRects
+ * @returns {LH.Artifacts.Rect[]}
+ */
+function mergeTouchingClientRects(clientRects) {
+  for (let i = 0; i < clientRects.length; i++) {
+    for (let j = i + 1; j < clientRects.length; j++) {
+      const crA = clientRects[i];
+      const crB = clientRects[j];
+
+      /**
+       * We try to determine whether the rects appear as a single tappable
+       * area to the user, so that they'd tap in the middle of the merged rect.
+       * Examples of what we want to merge:
+       *
+       * AAABBB
+       *
+       * AAA
+       * AAA
+       * BBBBB
+       */
+      const rectsLineUpHorizontally =
+        almostEqual(crA.top, crB.top) || almostEqual(crA.bottom, crB.bottom);
+      const rectsLineUpVertically =
+        almostEqual(crA.left, crB.left) || almostEqual(crA.right, crB.right);
+      const canMerge =
+        rectsTouchOrOverlap(crA, crB) &&
+        (rectsLineUpHorizontally || rectsLineUpVertically);
+
+      if (canMerge) {
+        const replacementClientRect = getBoundingRect(crA, crB);
+        const mergedRectCenter = getRectCenterPoint(replacementClientRect);
+
+        if (
+          !(
+            rectContainsPoint(crA, mergedRectCenter) ||
+            rectContainsPoint(crB, mergedRectCenter)
+          )
+        ) {
+          // Don't merge because the new shape is too different from the
+          // merged rects, and tapping in the middle wouldn't actually hit
+          // either rect
+          continue;
+        }
+
+        // Replace client rects with merged version
+        clientRects = clientRects.filter(cr => cr !== crA && cr !== crB);
+        clientRects.push(replacementClientRect);
+
+        // Start over so we don't have to handle complexity introduced by array mutation.
+        // Client rect ararys rarely contain more than 5 rects, so starting again doesn't cause perf issues.
+        return mergeTouchingClientRects(clientRects);
+      }
+    }
+  }
+
+  return clientRects;
+}
+
+module.exports = {
+  getTappableRectsFromClientRects,
+};
diff --git a/lighthouse-core/lib/url-shim.js b/lighthouse-core/lib/url-shim.js
index 260136058..0bf06d2fa 100644
--- a/lighthouse-core/lib/url-shim.js
+++ b/lighthouse-core/lib/url-shim.js
@@ -209,7 +209,7 @@ URLShim.URL = URL;
 URLShim.URLSearchParams = (typeof self !== 'undefined' && self.URLSearchParams) ||
     require('url').URLSearchParams;
 
-URLShim.NON_NETWORK_PROTOCOLS = ['blob', 'data'];
+URLShim.NON_NETWORK_PROTOCOLS = ['blob', 'data', 'intent'];
 
 URLShim.INVALID_URL_DEBUG_STRING =
     'Lighthouse was unable to determine the URL of some script executions. ' +
diff --git a/lighthouse-core/report/html/report-styles.css b/lighthouse-core/report/html/report-styles.css
index da536ff7a..4001365bc 100644
--- a/lighthouse-core/report/html/report-styles.css
+++ b/lighthouse-core/report/html/report-styles.css
@@ -652,6 +652,7 @@
   border-bottom: none;
 }
 .lh-audit-group--metrics .lh-audit-group__summary {
+  margin-top: 0;
   margin-bottom: 0;
 }
 
@@ -659,7 +660,8 @@
   display: flex;
   justify-content: space-between;
   padding-right: var(--text-indent);
-  margin-bottom: var(--lh-audit-vpadding);
+  margin-top: calc(var(--section-padding) * 1.5);
+  margin-bottom: var(--section-padding);
 }
 
 .lh-audit-group__itemcount {
diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts
index aebaa284d..d37048300 100644
--- a/types/artifacts.d.ts
+++ b/types/artifacts.d.ts
@@ -278,6 +278,15 @@ declare global {
         };
       }
 
+      export interface Rect {
+        width: number;
+        height: number;
+        top: number;
+        right: number;
+        bottom: number;
+        left: number;
+      }
+
       export interface ViewportDimensions {
         innerWidth: number;
         innerHeight: number;


Test patch:
diff --git a/lighthouse-core/test/audits/uses-rel-preload-test.js b/lighthouse-core/test/audits/uses-rel-preload-test.js
index 12176f309..6765a7370 100644
--- a/lighthouse-core/test/audits/uses-rel-preload-test.js
+++ b/lighthouse-core/test/audits/uses-rel-preload-test.js
@@ -210,6 +210,18 @@ describe('Performance: uses-rel-preload audit', () => {
     });
   });
 
+  it(`shouldn't suggest preload for protocol intent`, () => {
+    const networkRecords = getMockNetworkRecords();
+    networkRecords[2].protocol = 'intent';
+
+    const artifacts = mockArtifacts(networkRecords, defaultMainResourceUrl);
+    const context = {settings: {}, computedCache: new Map()};
+    return UsesRelPreload.audit(artifacts, context).then(output => {
+      assert.equal(output.rawValue, 0);
+      assert.equal(output.details.items.length, 0);
+    });
+  });
+
   it('does not throw on a real trace/devtools log', async () => {
     const artifacts = {
       URL: {finalUrl: 'https://pwa.rocks/'},
diff --git a/lighthouse-core/test/lib/minification-estimator-test.js b/lighthouse-core/test/lib/minification-estimator-test.js
index e04771723..f82c13b41 100644
--- a/lighthouse-core/test/lib/minification-estimator-test.js
+++ b/lighthouse-core/test/lib/minification-estimator-test.js
@@ -180,6 +180,31 @@ describe('minification estimator', () => {
       assert.equal(computeJSTokenLength(js), 9);
     });
 
+    it('should handle regular expression character classes', () => {
+      // test a slash inside of a character class to make sure it doesn't end the regex
+      // The below is the string-equivalent of
+      const _ = /regex [^/]\//.test('this should be in string not comment 123456789');
+
+      const js = `
+        /regex [^/]\\//.test('this should be in string not comment 123456789')
+      `;
+
+      assert.equal(computeJSTokenLength(js), 69);
+      assert.equal(computeJSTokenLength(js), js.trim().length);
+    });
+
+    it('should handle escaped regular expression characters', () => {
+      // test an escaped [ to make sure we can still close regexes
+      // This is the string-equivalent of
+      const _ = /regex \[/; // this should be in comment not string 123456789
+
+      const js = `
+        /regex \\[/ // this should be in comment not string 123456789
+      `;
+
+      assert.equal(computeJSTokenLength(js), 10);
+    });
+
     it('should distinguish regex from divide', () => {
       const js = `
         return 1 / 2 // hello
@@ -190,8 +215,8 @@ describe('minification estimator', () => {
 
     it('should handle large, real javscript files', () => {
       assert.equal(angularFullScript.length, 1364217);
-      // 1 - 405199 / 1364217 = estimated 70% smaller minified
-      assert.equal(computeJSTokenLength(angularFullScript), 405199);
+      // 1 - 334968 / 1364217 = estimated 75% smaller minified
+      assert.equal(computeJSTokenLength(angularFullScript), 334968);
     });
   });
 });
diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js
index 8db4648fb..d04114a0c 100644
--- a/lighthouse-core/test/lib/page-functions-test.js
+++ b/lighthouse-core/test/lib/page-functions-test.js
@@ -12,7 +12,7 @@ const pageFunctions = require('../../lib/page-functions');
 
 /* eslint-env jest */
 
-describe('DetailsRenderer', () => {
+describe('Page Functions', () => {
   let dom;
 
   beforeAll(() => {
@@ -44,5 +44,19 @@ describe('DetailsRenderer', () => {
         ['style-missing', 'aria-label-missing']
       ), '<div id="1" style="style" aria-label="label">');
     });
+
+    it('works if attribute values contain line breaks', () => {
+      assert.equal(pageFunctions.getOuterHTMLSnippet(
+        dom.createElement('div', '', {style: 'style1\nstyle2'})), '<div style="style1\nstyle2">');
+    });
+  });
+
+  describe('getNodeSelector', () => {
+    it('Uses IDs where available and otherwise falls back to classes', () => {
+      const parentEl = dom.createElement('div', '', {id: 'wrapper', class: 'dont-use-this'});
+      const childEl = dom.createElement('div', '', {class: 'child'});
+      parentEl.appendChild(childEl);
+      assert.equal(pageFunctions.getNodeSelector(childEl), 'div#wrapper > div.child');
+    });
   });
 });
diff --git a/lighthouse-core/test/lib/rect-helpers-test.js b/lighthouse-core/test/lib/rect-helpers-test.js
new file mode 100644
index 000000000..22c0a19be
--- /dev/null
+++ b/lighthouse-core/test/lib/rect-helpers-test.js
@@ -0,0 +1,87 @@
+/**
+ * @license Copyright 2018 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';
+
+/* eslint-env jest */
+
+const {
+  addRectTopAndBottom,
+  getRectOverlapArea,
+  getLargestRect,
+  getRectAtCenter,
+  allRectsContainedWithinEachOther,
+} = require('../../lib/rect-helpers');
+
+describe('Rect Helpers', () => {
+  it('getRectOverlapArea', () => {
+    const overlapArea = getRectOverlapArea(
+      addRectTopAndBottom({
+        x: 0,
+        y: 0,
+        width: 10,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 8,
+        y: 6,
+        width: 10,
+        height: 10,
+      })
+    );
+    expect(overlapArea).toBe(8);
+  });
+
+  it('getLargestRect', () => {
+    const rect1 = addRectTopAndBottom({
+      x: 0,
+      y: 0,
+      width: 5,
+      height: 5,
+    });
+    const rect2 = addRectTopAndBottom({
+      x: 0,
+      y: 0,
+      width: 10,
+      height: 10,
+    });
+    const largestRect = getLargestRect([rect1, rect2]);
+    expect(largestRect).toBe(rect2);
+  });
+
+  it('getRectAtCenter', () => {
+    const rect = addRectTopAndBottom({
+      x: 0,
+      y: 0,
+      width: 100,
+      height: 100,
+    });
+    const largestRect = getRectAtCenter(rect, 10);
+    expect(largestRect).toEqual(
+      addRectTopAndBottom({
+        x: 45,
+        y: 45,
+        width: 10,
+        height: 10,
+      })
+    );
+  });
+
+  describe('allRectsContainedWithinEachOther', () => {
+    it('Returns true if the rect lists are both empty', () => {
+      expect(allRectsContainedWithinEachOther([], [])).toBe(true);
+    });
+    it('Returns true if rects are contained in each other', () => {
+      const rect1 = addRectTopAndBottom({x: 0, y: 0, width: 100, height: 100});
+      const rect2 = addRectTopAndBottom({x: 40, y: 40, width: 20, height: 20});
+      expect(allRectsContainedWithinEachOther([rect1], [rect2])).toBe(true);
+    });
+    it('Returns true if rects aren\'t contained in each other', () => {
+      const rect1 = addRectTopAndBottom({x: 0, y: 0, width: 100, height: 100});
+      const rect2 = addRectTopAndBottom({x: 200, y: 200, width: 20, height: 20});
+      expect(allRectsContainedWithinEachOther([rect1], [rect2])).toBe(false);
+    });
+  });
+});
diff --git a/lighthouse-core/test/lib/tappable-rects-test.js b/lighthouse-core/test/lib/tappable-rects-test.js
new file mode 100644
index 000000000..030a4e397
--- /dev/null
+++ b/lighthouse-core/test/lib/tappable-rects-test.js
@@ -0,0 +1,246 @@
+/**
+ * @license Copyright 2018 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';
+
+/* eslint-env jest */
+
+const {addRectTopAndBottom} = require('../../lib/rect-helpers');
+const {getTappableRectsFromClientRects} = require('../../lib/tappable-rects');
+const assert = require('assert');
+
+describe('getTappableRectsFromClientRects', () => {
+  it('Merges rects if a smaller rect is inside a larger one', () => {
+    const containingRect = addRectTopAndBottom({
+      x: 10,
+      y: 10,
+      width: 100,
+      height: 10,
+    });
+    const containedRect = addRectTopAndBottom({
+      x: 10,
+      y: 10,
+      width: 50,
+      height: 10,
+    });
+
+    assert.deepEqual(getTappableRectsFromClientRects([
+      containingRect,
+      containedRect,
+    ]), [containingRect]);
+    assert.deepEqual(getTappableRectsFromClientRects([
+      containedRect,
+      containingRect,
+    ]), [containingRect]);
+  });
+  it('Merges two horizontally adjacent client rects', () => {
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 110,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 200,
+        height: 10,
+      }),
+    ]);
+  });
+
+  it('Merges three horizontally adjacent client rects', () => {
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 110,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 210,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 300,
+        height: 10,
+      }),
+    ]);
+  });
+
+  it('Merges client rects correctly if one is duplicated', () => {
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 90,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 90,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 100,
+        y: 10,
+        width: 10,
+        height: 10,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+    ]);
+  });
+
+  it('Merges two vertically adjacent client rects even if one is wider than the other', () => {
+    // We do this because to fix issues with children (e.g. images) inside links.
+    // The link itself might be small, so if we put a finger on it directly then it's
+    // likely to overlap with something.
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 10,
+        y: 15,
+        width: 200,
+        height: 15,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 200,
+        height: 20,
+      }),
+    ]);
+  });
+
+  it('Does not merge if the center of the merged rect wouldn\'t be in the original rects', () => {
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 10,
+        height: 100,
+      }),
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 200,
+        height: 10,
+      }),
+    ]);
+    assert.equal(res.length, 2);
+  });
+
+  it('Merges two horizontally adjacent client rects that don\'t line up exactly', () => {
+    // 2px difference is ok, often there are cases where an image is a 1px or 2px out of the main link client rect
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 110,
+        y: 12,
+        width: 100,
+        height: 10,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 200,
+        height: 12,
+      }),
+    ]);
+  });
+
+  it('Merges two identical client rects into one', () => {
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 10,
+        height: 10,
+      }),
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 10,
+        height: 10,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 10,
+        height: 10,
+      }),
+    ]);
+  });
+
+  it('Removes tiny 1x1px client rects', () => {
+    const res = getTappableRectsFromClientRects([
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 100,
+      }),
+      addRectTopAndBottom({
+        x: 5,
+        y: 5,
+        width: 1,
+        height: 1,
+      }),
+    ]);
+    assert.deepEqual(res, [
+      addRectTopAndBottom({
+        x: 10,
+        y: 10,
+        width: 100,
+        height: 100,
+      }),
+    ]);
+  });
+});