From d7c7c39b8da4214afcc963da3860b73717aa1e59 Mon Sep 17 00:00:00 2001
From: Paul Irish <paulirish@google.com>
Date: Fri, 24 May 2019 17:23:54 -0700
Subject: [PATCH 1/5] core(perf): speed up tap-target's isVisible()

---
 .../gather/gatherers/seo/tap-targets.js       | 30 ++-----------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js
index aed7ce0863d9..ea6efacf022f 100644
--- a/lighthouse-core/gather/gatherers/seo/tap-targets.js
+++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js
@@ -32,38 +32,12 @@ const TARGET_SELECTORS = [
 const tapTargetsSelector = TARGET_SELECTORS.join(',');
 
 /**
- * @param {Element} element
+ * @param {HTMLElement} element
  * @returns {boolean}
  */
 /* istanbul ignore next */
 function elementIsVisible(element) {
-  const {overflowX, overflowY, display, visibility} = getComputedStyle(element);
-
-  if (
-    display === 'none' ||
-    (visibility === 'collapse' && ['TR', 'TBODY', 'COL', 'COLGROUP'].includes(element.tagName))
-  ) {
-    // Element not displayed
-    return false;
-  }
-
-  // only for block and inline-block, since clientWidth/Height are always 0 for inline elements
-  if (display === 'block' || display === 'inline-block') {
-    // if height/width is 0 and no overflow in that direction then
-    // there's no content that the user can see and tap on
-    if ((element.clientWidth === 0 && overflowX === 'hidden') ||
-        (element.clientHeight === 0 && overflowY === 'hidden')) {
-      return false;
-    }
-  }
-
-  const parent = element.parentElement;
-  if (parent && parent.tagName !== 'BODY') {
-    // if a parent is invisible then the current element is also invisible
-    return elementIsVisible(parent);
-  }
-
-  return true;
+  return !!(element.offsetWidth || element.offsetHeight || element.getClientRects().length);
 }
 
 /**

From 9f58389e5e1127583ae701f2dd826680d2c076b4 Mon Sep 17 00:00:00 2001
From: Matt Zeunert <matt@mostlystatic.com>
Date: Sat, 25 May 2019 17:24:52 +0100
Subject: [PATCH 2/5] Fix types

---
 lighthouse-core/gather/gatherers/seo/tap-targets.js | 2 +-
 lighthouse-core/lib/page-functions.js               | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js
index 0d607f52111d..4682361378da 100644
--- a/lighthouse-core/gather/gatherers/seo/tap-targets.js
+++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js
@@ -212,7 +212,7 @@ function gatherTapTargets() {
   // Capture element positions relative to the top of the page
   window.scrollTo(0, 0);
 
-  /** @type {Element[]} */
+  /** @type {HTMLElement[]} */
   // @ts-ignore - getElementsInDocument put into scope via stringification
   const tapTargetElements = getElementsInDocument(tapTargetsSelector);
 
diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js
index 4a09367b8468..7fc31fec90a0 100644
--- a/lighthouse-core/lib/page-functions.js
+++ b/lighthouse-core/lib/page-functions.js
@@ -80,15 +80,15 @@ function checkTimeSinceLastLongTask() {
 /**
  * @param {string=} selector Optional simple CSS selector to filter nodes on.
  *     Combinators are not supported.
- * @return {Array<Element>}
+ * @return {Array<HTMLElement>}
  */
 /* istanbul ignore next */
 function getElementsInDocument(selector) {
   const realMatchesFn = window.__ElementMatches || window.Element.prototype.matches;
-  /** @type {Array<Element>} */
+  /** @type {Array<HTMLElement>} */
   const results = [];
 
-  /** @param {NodeListOf<Element>} nodes */
+  /** @param {NodeListOf<HTMLElement>} nodes */
   const _findAllElements = nodes => {
     for (let i = 0, el; el = nodes[i]; ++i) {
       if (!selector || realMatchesFn.call(el, selector)) {

From f012ce6879cd210b498d155e1ebd78e218880cff Mon Sep 17 00:00:00 2001
From: Matt Zeunert <matt@mostlystatic.com>
Date: Sat, 25 May 2019 18:57:36 +0100
Subject: [PATCH 3/5] Smoke test for failing tap targets with width 0

---
 .../test/fixtures/seo/seo-tap-targets.html    | 18 +++++++++++
 .../test/smokehouse/seo/expectations.js       | 31 +++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
index 9f76e82d8a12..91176f7dfe5b 100644
--- a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
+++ b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
@@ -159,6 +159,24 @@ <h1>SEO Tap targets</h1>
 
     <br/><br/>
 
+    <!-- even if a tap target's size is 0x0px it can still fail if overflowing content is visible -->
+    <div>
+      <a
+        data-gathered-target="zero-width-tap-target-with-overflowing-child-content"
+        style="display: block; width: 0; white-space: nowrap">
+        <!-- TODO: having the span should not be necessary to cause a failure here, but
+             right now we don't try to get the client rects of children that are text nodes -->
+        <span>zero width target</span>
+      </a>
+      <a
+        data-gathered-target="passing-tap-target-next-to-zero-width-target"
+        style="display: block; width: 100px; height: 100px;background: #aaa;">
+        passing target
+      </a>
+    </div>
+
+    <br/><br/>
+
     <!-- Should not fail if the two links have the same link target -->
     <div>
       <a
diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
index a913495e746b..b3b3bbf044c5 100644
--- a/lighthouse-cli/test/smokehouse/seo/expectations.js
+++ b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -62,6 +62,13 @@ const expectedGatheredTapTargets = [
   {
     snippet: /large-enough-tap-target-next-to-too-small-tap-target/,
   },
+  {
+    snippet: /zero-width-tap-target-with-overflowing-child-content/,
+    shouldFail: true,
+  },
+  {
+    snippet: /passing-tap-target-next-to-zero-width-target/,
+  },
   {
     snippet: /links-with-same-link-target-1/,
   },
@@ -251,6 +258,28 @@ module.exports = [
           })(),
           details: {
             items: [
+              {
+                'tapTarget': {
+                  'type': 'node',
+                  'snippet': '<a data-gathered-target="zero-width-tap-target-with-overflowing-child-content" style="display: block; width: 0; white-space: nowrap">\n        <!-- TODO: having the span should not be necessary to cause a failure here, but\n             right now we don\'t try to get the client rects of children that …',
+                  'path': '2,HTML,1,BODY,14,DIV,0,A',
+                  'selector': 'body > div > a',
+                  'nodeLabel': 'zero width target',
+                },
+                'overlappingTarget': {
+                  'type': 'node',
+                  'snippet': '<a data-gathered-target="passing-tap-target-next-to-zero-width-target" style="display: block; width: 100px; height: 100px;background: #aaa;">\n        passing target\n      </a>',
+                  'path': '2,HTML,1,BODY,14,DIV,1,A',
+                  'selector': 'body > div > a',
+                  'nodeLabel': 'passing target',
+                },
+                'tapTargetScore': 864,
+                'overlappingTargetScore': 720,
+                'overlapScoreRatio': 0.8333333333333334,
+                'size': '108x18',
+                'width': 108,
+                'height': 18,
+              },
               {
                 'tapTarget': {
                   'type': 'node',
@@ -276,11 +305,13 @@ module.exports = [
                   'type': 'node',
                   'path': '2,HTML,1,BODY,3,DIV,24,A',
                   'selector': 'body > div > a',
+                  'nodeLabel': 'left',
                 },
                 'overlappingTarget': {
                   'type': 'node',
                   'path': '2,HTML,1,BODY,3,DIV,25,A',
                   'selector': 'body > div > a',
+                  'nodeLabel': 'right',
                 },
                 'tapTargetScore': 1920,
                 'overlappingTargetScore': 560,

From 19f2e55d54937b2012b08edd6eaaab49216167c9 Mon Sep 17 00:00:00 2001
From: Matt Zeunert <matt@mostlystatic.com>
Date: Tue, 28 May 2019 17:54:40 +0100
Subject: [PATCH 4/5] Fix lint

---
 lighthouse-cli/test/smokehouse/seo/expectations.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
index b3b3bbf044c5..487e2b7761c6 100644
--- a/lighthouse-cli/test/smokehouse/seo/expectations.js
+++ b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -261,6 +261,7 @@ module.exports = [
               {
                 'tapTarget': {
                   'type': 'node',
+                  /* eslint-disable max-len */
                   'snippet': '<a data-gathered-target="zero-width-tap-target-with-overflowing-child-content" style="display: block; width: 0; white-space: nowrap">\n        <!-- TODO: having the span should not be necessary to cause a failure here, but\n             right now we don\'t try to get the client rects of children that …',
                   'path': '2,HTML,1,BODY,14,DIV,0,A',
                   'selector': 'body > div > a',
@@ -268,6 +269,7 @@ module.exports = [
                 },
                 'overlappingTarget': {
                   'type': 'node',
+                  /* eslint-disable max-len */
                   'snippet': '<a data-gathered-target="passing-tap-target-next-to-zero-width-target" style="display: block; width: 100px; height: 100px;background: #aaa;">\n        passing target\n      </a>',
                   'path': '2,HTML,1,BODY,14,DIV,1,A',
                   'selector': 'body > div > a',

From 0e3daceb0286e9aa8b0299621060022a048420ce Mon Sep 17 00:00:00 2001
From: Matt Zeunert <matt@mostlystatic.com>
Date: Wed, 29 May 2019 20:21:29 +0100
Subject: [PATCH 5/5] Fix element height in test

---
 lighthouse-cli/test/fixtures/seo/seo-tap-targets.html | 4 ++--
 lighthouse-cli/test/smokehouse/seo/expectations.js    | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
index 91176f7dfe5b..25bd225636c0 100644
--- a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
+++ b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
@@ -166,11 +166,11 @@ <h1>SEO Tap targets</h1>
         style="display: block; width: 0; white-space: nowrap">
         <!-- TODO: having the span should not be necessary to cause a failure here, but
              right now we don't try to get the client rects of children that are text nodes -->
-        <span>zero width target</span>
+        <span style="display: inline-block; width: 110px; height: 18px">zero width target</span>
       </a>
       <a
         data-gathered-target="passing-tap-target-next-to-zero-width-target"
-        style="display: block; width: 100px; height: 100px;background: #aaa;">
+        style="display: block; width: 110px; height: 100px;background: #aaa;">
         passing target
       </a>
     </div>
diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
index 487e2b7761c6..f5b9a7b182f0 100644
--- a/lighthouse-cli/test/smokehouse/seo/expectations.js
+++ b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -270,7 +270,7 @@ module.exports = [
                 'overlappingTarget': {
                   'type': 'node',
                   /* eslint-disable max-len */
-                  'snippet': '<a data-gathered-target="passing-tap-target-next-to-zero-width-target" style="display: block; width: 100px; height: 100px;background: #aaa;">\n        passing target\n      </a>',
+                  'snippet': '<a data-gathered-target="passing-tap-target-next-to-zero-width-target" style="display: block; width: 110px; height: 100px;background: #aaa;">\n        passing target\n      </a>',
                   'path': '2,HTML,1,BODY,14,DIV,1,A',
                   'selector': 'body > div > a',
                   'nodeLabel': 'passing target',
@@ -278,8 +278,8 @@ module.exports = [
                 'tapTargetScore': 864,
                 'overlappingTargetScore': 720,
                 'overlapScoreRatio': 0.8333333333333334,
-                'size': '108x18',
-                'width': 108,
+                'size': '110x18',
+                'width': 110,
                 'height': 18,
               },
               {