GoogleChrome · patrickhulce · May 25, 2019 · Mar 28, 2019 · Mar 28, 2019 · Mar 28, 2019
diff --git a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
@@ -27,9 +27,19 @@
   <!-- Big tap target, but it's invisible because it's behind the main content div -->
   <a style="background: red; position: absolute; top: 0; bottom: 0; left: 0; right: 0; z-index: -1;"></a>
 
-  <a style="background: red; position: sticky; top: 0; left: 0; display: block; width: 100vw; height: 40px; z-index: 1;">sticky</a>
+  <a style="background: red; position: sticky; top: 0; left: 0; display: block; width: 100vw; height: 30px; z-index: 1;">sticky</a>
 
   <div style="background: white; padding: 20px;">
+
+    <div style="height: 300px">
+      <a
+        data-gathered-target="large-link-at-bottom-of-page"
+        style="background: #afa; position: absolute; top: 0; display: block; width: calc(100% - 40px); height: 300px;">
+        This link is intentionally placed at the top of the page, so that the sticky header is hard to tap on.
+        It should not fail though because overlap with a sticky element depends on the scroll position.
+      </a>
+    </div>
+
     <h1>SEO Tap targets</h1>
 
     <!-- Invisible nodes don't cause failures -->
@@ -47,14 +57,14 @@ <h1>SEO Tap targets</h1>
         width 0 and overflow x hidden
       </a>
       <!-- Visible target should not fail because nothing overlaps it -->
-      <a>visible target</a>
+      <a data-gathered-target="visible-target">visible target</a>
     </div>
     <br/><br/>
 
 
     <div style="overflow: hidden; position: relative">
       <!-- Should be counted as visible although part of it is outside the scroll container -->
-      <a>
+      <a data-gathered-target="target-with-client-rect-outside-scroll-container">
         <div style="position: absolute;top: -100px">invisible</div>
         visible
       </a>
@@ -64,18 +74,18 @@ <h1>SEO Tap targets</h1>
 
     <!-- Link contains large inline block element - no failure because finger
       should be placed in the center of the whole link area -->
-    <a style="background: red;">
+    <a style="background: red;" data-gathered-target="link-containing-large-inline-block-element">
       <div style="display: inline-block; height: 30px; width: 30px; background: orange;"></div>
       Link
     </a>
     <br/>
-    <a style="display: block; padding: 30px; background: #ddf;">
+    <a style="display: block; padding: 30px; background: #ddf;" data-gathered-target="link-next-to-link-containing-large-inline-block-element">
       Link that the top one would overlap with, if it weren't for the inline-block child.
     </a>
 
     <br/><br/>
 
-    <div role="button">
+    <div role="button" data-gathered-target="tap-target-containing-other-tap-targets">
       Tap target with children that are also tap targets should not fail.
       (Children should not be counted as independent tap targets that appear
       in the list.)
@@ -85,29 +95,78 @@ <h1>SEO Tap targets</h1>
 
     <br/><br/>
 
-    <div style="position: relative">
-      <a style="display: block; position: absolute; top:0; height: 40px; width: 40px;">inner</a>
-      <a style="display: block; height: 100px; width: 100px; background: yellowgreen">outer</a>
+    <!-- The left tap target has a large client rect that would normally overlap with the right tap target, 
+         but the left link is overflow hidden, so there should be no failure. We should however
+         detect both tap targets (and not say the left one is invisible because the center of the big client rect is invisible) -->
+    <div style="overflow: hidden">
+      <a
+        data-gathered-target="child-client-rect-hidden-by-overflow-hidden"
+        style="display: block; height: 40px; width: 40px; background: #fca; float: left; overflow: hidden; position: relative;">
+        <div style="background: #afa; height: 40px; width: 80px; position: absolute; left: 30px">
+          left    
+        </div>
+      </a>
+      <a
+        data-gathered-target="tap-target-next-to-child-client-rect-hidden-by-overflow-hidden"
+        style="display: block; height: 40px; width: 60px; background: yellowgreen; float: left">right</a>
     </div>
 
     <br/><br/>
 
-    <!-- Only target that's being overlapped should fail, the overlapping one shouldn't -->
-    <div>
-      <a style="display: block; width: 100px; height: 30px;background: #ddd;">
-        too small target
-      </a>
-      <a style="display: block; width: 100px; height: 100px;background: #aaa;">
-        big enough target
+    <!--Similar to previous test, except that the position absolute child isn't hidden by "overflow: hidden" on the parent. -->
+      <div style="overflow: hidden"></div>
+      <a
+        data-gathered-target="child-client-rect-overlapping-other-target"
+        style="display: block; height: 40px; width: 40px; background: #fca; float: left; position: relative;">
+        <div style="background: #afa; height: 40px; width: 30px; position: absolute; left: 30px">
+          left    
+        </div>
       </a>
+      <a
+        data-gathered-target="tap-target-overlapped-by-other-targets-position-absolute-child-rect"
+        style="display: block; height: 40px; width: 60px; background: yellowgreen; float: left">right</a>
+    </div>
+
+    <br/><br/>
+
+    <div style="position: relative">
+      <a
+        data-gathered-target="position-absolute-tap-target-fully-contained-in-other-target"
+        style="display: block; position: absolute; top:0; height: 40px; width: 40px;">inner</a>
+      <a
+        data-gathered-target="tap-target-fully-containing-position-absolute-target"
+        style="display: block; height: 100px; width: 100px; background: yellowgreen">outer</a>
+    </div>
+
+    <br/><br/>
+
+    <!-- position: absolute wrapper shouldn't stop not prevent the check (fixed/sticky would) -->
+    <div style="position: relative; height: 200px;">
+      <div style="position: absolute;">
+        <!-- Only target that's being overlapped should fail, the overlapping one shouldn't -->
+        <a
+          data-gathered-target="too-small-failing-tap-target"
+          style="display: block; width: 100px; height: 30px;background: #ddd;">
+          too small target
+        </a>
+        <a
+          data-gathered-target="large-enough-tap-target-next-to-too-small-tap-target"
+          style="display: block; width: 100px; height: 100px;background: #aaa;">
+          big enough target
+        </a>
+      </div>
     </div>
 
     <br/><br/>
 
     <!-- Should not fail if the two links have the same link target -->
     <div>
-      <a style="display: block; width: 10px; height: 10px;" href="../seo/"></a>
-      <a style="display: block; width: 10px; height: 10px;" href="./"></a>
+      <a
+        data-gathered-target="links-with-same-link-target-1"
+        style="display: block; width: 10px; height: 10px;" href="../seo/"></a>
+      <a
+        data-gathered-target="links-with-same-link-target-2"
+        style="display: block; width: 10px; height: 10px;" href="./"></a>
     </div>
 
     <!-- Links in text blocks are exempted from size/overlap requirements and should not fail -->
@@ -118,12 +177,6 @@ <h1>SEO Tap targets</h1>
       This is <a>a link in a text block.</a>
       This is <a>a link in a text block.</a>
     </p>
-
-    <a style="background: #afa; display: block; width: 100%; height: 800px;">
-      This link is intentionally placed at the bottom of the page, so that when we've scrolled
-      to the bottom of the page it overlaps with the sticky header. (Should not fail though
-      because the overlap depends on the current scroll position.)
-    </a>
   </div>
 </body>
 </html>
diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -16,6 +16,60 @@ function headersParam(headers) {
   return new URLSearchParams([['extra_header', headerString]]).toString();
 }
 
+const expectedGatheredTapTargets = [
+  {
+    snippet: /large-link-at-bottom-of-page/,
+  },
+  {
+    snippet: /visible-target/,
+  },
+  {
+    snippet: /target-with-client-rect-outside-scroll-container/,
+  },
+  {
+    snippet: /link-containing-large-inline-block-element/,
+  },
+  {
+    snippet: /link-next-to-link-containing-large-inline-block-element/,
+  },
+  {
+    snippet: /tap-target-containing-other-tap-targets/,
+  },
+  {
+    snippet: /child-client-rect-hidden-by-overflow-hidden/,
+  },
+  {
+    snippet: /tap-target-next-to-child-client-rect-hidden-by-overflow-hidden/,
+  },
+  {
+    snippet: /child-client-rect-overlapping-other-target/,
+    shouldFail: true,
+  },
+  {
+    snippet: /tap-target-overlapped-by-other-targets-position-absolute-child-rect/,
+    shouldFail: true,
+  },
+  {
+    snippet: /position-absolute-tap-target-fully-contained-in-other-target/,
+  },
+  {
+    snippet: /tap-target-fully-containing-position-absolute-target/,
+  },
+  {
+    snippet: /too-small-failing-tap-target/,
+    shouldFail: true,
+  },
+  {
+    snippet: /large-enough-tap-target-next-to-too-small-tap-target/,
+  },
+  {
+    snippet: /links-with-same-link-target-1/,
+  },
+  {
+    snippet: /links-with-same-link-target-2/,
+  },
+];
+
 const failureHeaders = headersParam([[
   'x-robots-tag',
   'none',
@@ -190,48 +244,58 @@ module.exports = [
       audits: {
         'tap-targets': {
           score: (() => {
-            const PASSING_TAP_TARGETS = 11;
-            const TOTAL_TAP_TARGETS = 12;
+            const totalTapTargets = expectedGatheredTapTargets.length;
+            const passingTapTargets = expectedGatheredTapTargets.filter(t => !t.shouldFail).length;
             const SCORE_FACTOR = 0.89;
-            return Math.floor(PASSING_TAP_TARGETS / TOTAL_TAP_TARGETS * SCORE_FACTOR * 100) / 100;
+            return Math.round(passingTapTargets / totalTapTargets * SCORE_FACTOR * 100) / 100;
           })(),
           details: {
             items: [
               {
                 'tapTarget': {
                   'type': 'node',
-                  'snippet': '<a ' +
-                 'style="display: block; width: 100px; height: 30px;background: #ddd;">' +
-                 '\n        too small target\n      </a>',
-                  'path': '2,HTML,1,BODY,3,DIV,21,DIV,0,A',
+                  'path': '2,HTML,1,BODY,10,DIV,0,DIV,1,A',
                   'selector': 'body > div > div > a',
                   'nodeLabel': 'too small target',
                 },
                 'overlappingTarget': {
                   'type': 'node',
-                  'snippet': '<a ' +
-                  'style="display: block; width: 100px; height: 100px;background: #aaa;">' +
-                  '\n        big enough target\n      </a>',
-                  'path': '2,HTML,1,BODY,3,DIV,21,DIV,1,A',
+                  'path': '2,HTML,1,BODY,10,DIV,0,DIV,2,A',
                   'selector': 'body > div > div > a',
                   'nodeLabel': 'big enough target',
                 },
-                'size': '100x30',
-                'width': 100,
-                'height': 30,
                 'tapTargetScore': 1440,
                 'overlappingTargetScore': 432,
                 'overlapScoreRatio': 0.3,
+                'size': '100x30',
+                'width': 100,
+                'height': 30,
+              },
+              {
+                'tapTarget': {
+                  'type': 'node',
+                  'path': '2,HTML,1,BODY,3,DIV,24,A',
+                  'selector': 'body > div > a',
+                },
+                'overlappingTarget': {
+                  'type': 'node',
+                  'path': '2,HTML,1,BODY,3,DIV,25,A',
+                  'selector': 'body > div > a',
+                },
+                'tapTargetScore': 1920,
+                'overlappingTargetScore': 560,
+                'overlapScoreRatio': 0.2916666666666667,
+                'size': '40x40',
+                'width': 40,
+                'height': 40,
               },
             ],
           },
         },
       },
     },
     artifacts: {
-      TapTargets: {
-        length: 11,
-      },
+      TapTargets: expectedGatheredTapTargets.map(({snippet}) => ({snippet})),
     },
   },
 ];