Repository: GoogleChrome/lighthouse
Pull request #7603 (2019-03-20 16:14:41)

core(tap-targets): exclude sticky elements from tap targets audit
**Summary**

Whether sticky elements overlap depends on the current scroll position, so we should exclude them like we do with position absolute and fixed.

**Related Issues/PRs**

Pointed out here: https://github.com/GoogleChrome/lighthouse/issues/7365#issuecomment-474095920


Patch:
diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js
index 945146f40..6314c60f7 100644
--- a/lighthouse-core/gather/gatherers/seo/tap-targets.js
+++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js
@@ -225,13 +225,13 @@ function elementIsInTextBlock(element) {
  * @returns {boolean}
  */
 /* istanbul ignore next */
-function elementIsPositionFixedOrAbsolute(element) {
+function elementIsPositionFixedStickyOrAbsolute(element) {
   const {position} = getComputedStyle(element);
-  if (position === 'fixed' || position === 'absolute') {
+  if (position === 'fixed' || position === 'absolute' || position === 'sticky') {
     return true;
   }
   if (element.parentElement) {
-    return elementIsPositionFixedOrAbsolute(element.parentElement);
+    return elementIsPositionFixedStickyOrAbsolute(element.parentElement);
   }
   return false;
 }
@@ -273,7 +273,7 @@ function gatherTapTargets() {
       // in the Web Content Accessibility Guidelines https://www.w3.org/TR/WCAG21/#target-size
       return;
     }
-    if (elementIsPositionFixedOrAbsolute(tapTargetElement)) {
+    if (elementIsPositionFixedStickyOrAbsolute(tapTargetElement)) {
       // Absolutely positioned elements might not be visible if they have a lower z-index
       // than other tap targets, but if we don't ignore them we can get false failures.
       //
@@ -311,7 +311,7 @@ class TapTargets extends Gatherer {
       const tapTargetsSelector = "${tapTargetsSelector}";
       ${pageFunctions.getElementsInDocumentString};
       ${filterClientRectsWithinAncestorsVisibleScrollArea.toString()};
-      ${elementIsPositionFixedOrAbsolute.toString()};
+      ${elementIsPositionFixedStickyOrAbsolute.toString()};
       ${elementIsVisible.toString()};
       ${elementHasAncestorTapTarget.toString()};
       ${getVisibleClientRects.toString()};
diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js
index 5ed1acea4..687924e0b 100644
--- a/lighthouse-core/lib/network-request.js
+++ b/lighthouse-core/lib/network-request.js
@@ -302,7 +302,7 @@ module.exports = class NetworkRequest {
    * The total length of the encoded data is spread out among multiple events. The sum of the
    * values in onResponseReceived and all the onDataReceived events typically equals the value
    * seen on the onLoadingFinished event. In <1% of cases we see the values differ. As we process
-   * onResonseReceived and onDataReceived we accumulate the total encodedDataLength. When we
+   * onResponseReceived and onDataReceived we accumulate the total encodedDataLength. When we
    * process onLoadingFinished, we override the accumulated total. We do this so that if the
    * request is aborted or fails, we still get a value via the accumulation.
    *
diff --git a/lighthouse-core/report/html/renderer/dom.js b/lighthouse-core/report/html/renderer/dom.js
index 4d178fcdc..dee3fa981 100644
--- a/lighthouse-core/report/html/renderer/dom.js
+++ b/lighthouse-core/report/html/renderer/dom.js
@@ -27,6 +27,8 @@ class DOM {
   constructor(document) {
     /** @type {Document} */
     this._document = document;
+    /** @type {string} */
+    this._lighthouseChannel = 'unknown';
   }
 
   /**
@@ -125,11 +127,19 @@ class DOM {
 
       // Append link if there are any.
       if (linkText && linkHref) {
+        const url = new URL(linkHref);
+
+        const DEVELOPERS_GOOGLE_ORIGIN = 'https://developers.google.com';
+        if (url.origin === DEVELOPERS_GOOGLE_ORIGIN) {
+          url.searchParams.set('utm_source', 'lighthouse');
+          url.searchParams.set('utm_medium', this._lighthouseChannel);
+        }
+
         const a = this.createElement('a');
         a.rel = 'noopener';
         a.target = '_blank';
         a.textContent = linkText;
-        a.href = (new URL(linkHref)).href;
+        a.href = url.href;
         element.appendChild(a);
       }
     }
@@ -159,6 +169,14 @@ class DOM {
     return element;
   }
 
+  /**
+   * The channel to use for UTM data when rendering links to the documentation.
+   * @param {string} lighthouseChannel
+   */
+  setLighthouseChannel(lighthouseChannel) {
+    this._lighthouseChannel = lighthouseChannel;
+  }
+
   /**
    * @return {Document}
    */
diff --git a/lighthouse-core/report/html/renderer/report-renderer.js b/lighthouse-core/report/html/renderer/report-renderer.js
index 325c27dc5..7a997ee26 100644
--- a/lighthouse-core/report/html/renderer/report-renderer.js
+++ b/lighthouse-core/report/html/renderer/report-renderer.js
@@ -47,6 +47,8 @@ class ReportRenderer {
     // Mutate the UIStrings if necessary (while saving originals)
     const originalUIStrings = JSON.parse(JSON.stringify(Util.UIStrings));
 
+    this._dom.setLighthouseChannel(result.configSettings.channel || 'unknown');
+
     const report = Util.prepareReportResult(result);
 
     container.textContent = ''; // Remove previous report.


Test patch:
diff --git a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
index 9db7c9934..141011099 100644
--- a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
+++ b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html
@@ -26,6 +26,9 @@
 <body>
   <!-- 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>
+
   <div style="background: white; padding: 20px;">
     <h1>SEO Tap targets</h1>
 
@@ -108,13 +111,19 @@
     </div>
 
     <!-- Links in text blocks are exempted from size/overlap requirements and should not fail -->
-    <p style="width: 100px">
+    <p style="width: 100px;margin-bottom: 30px;">
       This is <a>a link in a text block.</a>
       This is <a>a link in a text block.</a>
       This is <a>a link in a text block.</a>
       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
index 1d0f8d70f..6fe1913f9 100644
--- a/lighthouse-cli/test/smokehouse/seo/expectations.js
+++ b/lighthouse-cli/test/smokehouse/seo/expectations.js
@@ -184,7 +184,12 @@ module.exports = [
     finalUrl: BASE_URL + 'seo-tap-targets.html',
     audits: {
       'tap-targets': {
-        score: 0.8, // 10 passing targets/11 total visible targets, multiplied by 0.89 to get the score
+        score: (() => {
+          const PASSING_TAP_TARGETS = 11;
+          const TOTAL_TAP_TARGETS = 12;
+          const SCORE_FACTOR = 0.89;
+          return Math.floor(PASSING_TAP_TARGETS / TOTAL_TAP_TARGETS * SCORE_FACTOR * 100) / 100;
+        })(),
         details: {
           items: [
             {
@@ -193,7 +198,7 @@ module.exports = [
                 'snippet': '<a ' +
                  'style="display: block; width: 100px; height: 30px;background: #ddd;">' +
                  '\n        too small target\n      </a>',
-                'path': '2,HTML,1,BODY,2,DIV,21,DIV,0,A',
+                'path': '2,HTML,1,BODY,3,DIV,21,DIV,0,A',
                 'selector': 'body > div > div > a',
               },
               'overlappingTarget': {
@@ -201,7 +206,7 @@ module.exports = [
                 'snippet': '<a ' +
                   'style="display: block; width: 100px; height: 100px;background: #aaa;">' +
                   '\n        big enough target\n      </a>',
-                'path': '2,HTML,1,BODY,2,DIV,21,DIV,1,A',
+                'path': '2,HTML,1,BODY,3,DIV,21,DIV,1,A',
                 'selector': 'body > div > div > a',
               },
               'size': '100x30',
diff --git a/lighthouse-core/test/report/html/renderer/dom-test.js b/lighthouse-core/test/report/html/renderer/dom-test.js
index 96bda5074..ea91f810f 100644
--- a/lighthouse-core/test/report/html/renderer/dom-test.js
+++ b/lighthouse-core/test/report/html/renderer/dom-test.js
@@ -23,6 +23,7 @@ describe('DOM', () => {
     global.URL = URL;
     const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window;
     dom = new DOM(document);
+    dom.setLighthouseChannel('someChannel');
   });
 
   afterAll(() => {
@@ -115,6 +116,20 @@ describe('DOM', () => {
       assert.equal(result.innerHTML, 'Ensuring `&lt;td&gt;` cells using the `[headers]` are ' +
           'good. <a rel="noopener" target="_blank" href="https://dequeuniversity.com/rules/axe/3.1/td-headers-attr">Learn more</a>.');
     });
+
+    it('appends utm params to the URLs with https://developers.google.com origin', () => {
+      const text = '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/description).';
+
+      const result = dom.convertMarkdownLinkSnippets(text);
+      assert.equal(result.innerHTML, '<a rel="noopener" target="_blank" href="https://developers.google.com/web/tools/lighthouse/audits/description?utm_source=lighthouse&amp;utm_medium=someChannel">Learn more</a>.');
+    });
+
+    it('doesn\'t append utm params to non https://developers.google.com origins', () => {
+      const text = '[Learn more](https://example.com/info).';
+
+      const result = dom.convertMarkdownLinkSnippets(text);
+      assert.equal(result.innerHTML, '<a rel="noopener" target="_blank" href="https://example.com/info">Learn more</a>.');
+    });
   });
 
   describe('convertMarkdownCodeSnippets', () => {
diff --git a/lighthouse-core/test/report/html/renderer/report-renderer-test.js b/lighthouse-core/test/report/html/renderer/report-renderer-test.js
index 478d8dd81..f8362c66c 100644
--- a/lighthouse-core/test/report/html/renderer/report-renderer-test.js
+++ b/lighthouse-core/test/report/html/renderer/report-renderer-test.js
@@ -199,6 +199,25 @@ describe('ReportRenderer', () => {
     assert.ok(output.querySelector('.score100'), 'has fireworks treatment');
   });
 
+  it('should add LHR channel to doc link parameters', () => {
+    const lhrChannel = sampleResults.configSettings.channel;
+    // Make sure we have a channel in the LHR.
+    assert.ok(lhrChannel.length > 2);
+
+    const container = renderer._dom._document.body;
+    const output = renderer.renderReport(sampleResults, container);
+
+    const utmChannels = [...output.querySelectorAll('a[href*="utm_source=lighthouse"')]
+      .map(a => new URL(a.href))
+      .filter(url => url.origin === 'https://developers.google.com')
+      .map(url => url.searchParams.get('utm_medium'));
+
+    assert.ok(utmChannels.length > 20);
+    utmChannels.forEach(anchorChannel => {
+      assert.strictEqual(anchorChannel, lhrChannel);
+    });
+  });
+
   it('renders `not_applicable` audits as `notApplicable`', () => {
     const clonedSampleResult = JSON.parse(JSON.stringify(sampleResultsOrig));