From 8fe4b17a209c2669c9359f0729c2e5b2c0b5e88f Mon Sep 17 00:00:00 2001 From: Matt Zeunert Date: Wed, 20 Mar 2019 16:11:24 +0000 Subject: [PATCH 1/2] Exclude sticky elements from tap targets audit --- lighthouse-cli/test/fixtures/seo/seo-tap-targets.html | 11 ++++++++++- lighthouse-cli/test/smokehouse/seo/expectations.js | 11 ++++++++--- lighthouse-core/gather/gatherers/seo/tap-targets.js | 10 +++++----- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html index 9db7c9934bef..ecc006a8f485 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 @@ + + sticky +

SEO Tap targets

@@ -108,13 +111,19 @@

SEO Tap targets

-

+

This is a link in a text block. This is a link in a text block. This is a link in a text block. This is a link in a text block. This is a link in a text block.

+ + + 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.) + diff --git a/lighthouse-cli/test/smokehouse/seo/expectations.js b/lighthouse-cli/test/smokehouse/seo/expectations.js index 1d0f8d70f4da..6fe1913f9c34 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': '' + '\n too small target\n ', - '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': '' + '\n big enough target\n ', - '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/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index 945146f40c88..6314c60f7e73 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()}; From 15267562767501bcdabac51cc2b88ed2c45097bd Mon Sep 17 00:00:00 2001 From: Matt Zeunert Date: Wed, 20 Mar 2019 16:55:09 +0000 Subject: [PATCH 2/2] Remove extra css from position static element --- lighthouse-cli/test/fixtures/seo/seo-tap-targets.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html index ecc006a8f485..14101109927f 100644 --- a/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html +++ b/lighthouse-cli/test/fixtures/seo/seo-tap-targets.html @@ -119,7 +119,7 @@

SEO Tap targets

This is a link in a text block.

- + 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.)