From a821e7877bd029433f6ec3f40ec8617d156fadee Mon Sep 17 00:00:00 2001 From: Matt Zeunert Date: Sun, 17 Feb 2019 16:18:37 +0000 Subject: [PATCH] Create viewportMeta computed artifact and use that instead of calling ViewportMeta audit --- lighthouse-core/audits/seo/font-size.js | 11 ++-- lighthouse-core/audits/seo/tap-targets.js | 11 ++-- lighthouse-core/audits/viewport.js | 32 ++++------ lighthouse-core/computed/viewport-meta.js | 50 ++++++++++++++++ .../test/audits/seo/font-size-test.js | 37 ++++++------ .../test/audits/seo/tap-targets-test.js | 59 ++++++++++--------- lighthouse-core/test/audits/viewport-test.js | 59 ++++++++++--------- types/artifacts.d.ts | 9 +++ 8 files changed, 162 insertions(+), 106 deletions(-) create mode 100644 lighthouse-core/computed/viewport-meta.js diff --git a/lighthouse-core/audits/seo/font-size.js b/lighthouse-core/audits/seo/font-size.js index 3473a502f8a5..f5436d6b3c97 100644 --- a/lighthouse-core/audits/seo/font-size.js +++ b/lighthouse-core/audits/seo/font-size.js @@ -10,7 +10,7 @@ const URL = require('../../lib/url-shim'); const i18n = require('../../lib/i18n/i18n.js'); const Audit = require('../audit'); -const ViewportAudit = require('../viewport'); +const ComputedViewportMeta = require('../../computed/viewport-meta'); const MINIMAL_PERCENTAGE_OF_LEGIBLE_TEXT = 60; const UIStrings = { @@ -213,11 +213,12 @@ class FontSize extends Audit { /** * @param {LH.Artifacts} artifacts - * @return {LH.Audit.Product} + * @param {LH.Audit.Context} context + * @return {Promise} */ - static audit(artifacts) { - const hasViewportSet = ViewportAudit.audit(artifacts).rawValue; - if (!hasViewportSet) { + static async audit(artifacts, context) { + const viewportMeta = await ComputedViewportMeta.request(artifacts, context) + if (!viewportMeta.hasMobileViewport) { return { rawValue: false, explanation: str_(UIStrings.explanationViewport), diff --git a/lighthouse-core/audits/seo/tap-targets.js b/lighthouse-core/audits/seo/tap-targets.js index 08b0c387da2c..1ef89b28dcbc 100644 --- a/lighthouse-core/audits/seo/tap-targets.js +++ b/lighthouse-core/audits/seo/tap-targets.js @@ -10,7 +10,7 @@ * no other tap target that's too close so that the user might accidentally tap on. */ const Audit = require('../audit'); -const ViewportAudit = require('../viewport'); +const ComputedViewportMeta = require('../../computed/viewport-meta'); const { rectsTouchOrOverlap, getRectOverlapArea, @@ -270,11 +270,12 @@ class TapTargets extends Audit { /** * @param {LH.Artifacts} artifacts - * @return {LH.Audit.Product} + * @param {LH.Audit.Context} context + * @return {Promise} */ - static audit(artifacts) { - const hasViewportSet = ViewportAudit.audit(artifacts).rawValue; - if (!hasViewportSet) { + static async audit(artifacts, context) { + const viewportMeta = await ComputedViewportMeta.request(artifacts, context); + if (!viewportMeta.hasMobileViewport) { return { rawValue: false, explanation: str_(UIStrings.explanationViewportMetaNotOptimized), diff --git a/lighthouse-core/audits/viewport.js b/lighthouse-core/audits/viewport.js index 90216c10f317..5be3da0aedd7 100644 --- a/lighthouse-core/audits/viewport.js +++ b/lighthouse-core/audits/viewport.js @@ -6,7 +6,8 @@ 'use strict'; const Audit = require('./audit'); -const Parser = require('metaviewport-parser'); +const ComputedViewportMeta = require('../computed/viewport-meta.js'); + class Viewport extends Audit { /** @@ -26,33 +27,22 @@ class Viewport extends Audit { /** * @param {LH.Artifacts} artifacts - * @return {LH.Audit.Product} + * @param {LH.Audit.Context} context + * @return {Promise} */ - static audit(artifacts) { - const viewportMeta = artifacts.MetaElements.find(meta => meta.name === 'viewport'); - if (!viewportMeta) { + static async audit(artifacts, context) { + const viewportMeta = await ComputedViewportMeta.request(artifacts, context); + + if (!viewportMeta.hasViewportTag) { return { - explanation: 'No viewport meta tag found', rawValue: false, + explanation: 'No viewport meta tag found', }; } - const warnings = []; - const parsedProps = Parser.parseMetaViewPortContent(viewportMeta.content || ''); - - if (Object.keys(parsedProps.unknownProperties).length) { - warnings.push(`Invalid properties found: ${JSON.stringify(parsedProps.unknownProperties)}`); - } - if (Object.keys(parsedProps.invalidValues).length) { - warnings.push(`Invalid values found: ${JSON.stringify(parsedProps.invalidValues)}`); - } - - const viewportProps = parsedProps.validProperties; - const hasMobileViewport = viewportProps.width || viewportProps['initial-scale']; - return { - rawValue: !!hasMobileViewport, - warnings, + rawValue: !!viewportMeta.hasMobileViewport, + warnings: viewportMeta.parserWarnings, }; } } diff --git a/lighthouse-core/computed/viewport-meta.js b/lighthouse-core/computed/viewport-meta.js new file mode 100644 index 000000000000..e077c013f2c6 --- /dev/null +++ b/lighthouse-core/computed/viewport-meta.js @@ -0,0 +1,50 @@ +/** + * @license Copyright 2019 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 Parser = require('metaviewport-parser'); + +const makeComputedArtifact = require('./computed-artifact.js'); + +class ViewportMeta { + /** + * @param {{MetaElements: LH.GathererArtifacts['MetaElements']}} artifacts + * @return {Promise} + */ + + static async compute_({MetaElements}) { + const viewportMeta = MetaElements.find(meta => meta.name === 'viewport'); + + if (!viewportMeta) { + return { + hasViewportTag: false, + hasMobileViewport: false, + parserWarnings: [], + }; + } + + const warnings = []; + const parsedProps = Parser.parseMetaViewPortContent(viewportMeta.content || ''); + + if (Object.keys(parsedProps.unknownProperties).length) { + warnings.push(`Invalid properties found: ${JSON.stringify(parsedProps.unknownProperties)}`); + } + if (Object.keys(parsedProps.invalidValues).length) { + warnings.push(`Invalid values found: ${JSON.stringify(parsedProps.invalidValues)}`); + } + + const viewportProps = parsedProps.validProperties; + const hasMobileViewport = Boolean(viewportProps.width || viewportProps['initial-scale']); + + return { + hasMobileViewport, + hasViewportTag: true, + parserWarnings: warnings, + }; + } +} + +module.exports = makeComputedArtifact(ViewportMeta); diff --git a/lighthouse-core/test/audits/seo/font-size-test.js b/lighthouse-core/test/audits/seo/font-size-test.js index ee6612d092da..b757cbb058ca 100644 --- a/lighthouse-core/test/audits/seo/font-size-test.js +++ b/lighthouse-core/test/audits/seo/font-size-test.js @@ -15,22 +15,23 @@ const validViewport = 'width=device-width'; describe('SEO: Font size audit', () => { const makeMetaElements = viewport => [{name: 'viewport', content: viewport}]; + const fakeContext = {computedCache: new Map()}; - it('fails when viewport is not set', () => { + it('fails when viewport is not set', async () => { const artifacts = { URL, MetaElements: [], FontSize: [], }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, false); expect(auditResult.explanation) .toBeDisplayString('Text is illegible because there\'s ' + 'no viewport meta tag optimized for mobile screens.'); }); - it('fails when less than 60% of text is legible', () => { + it('fails when less than 60% of text is legible', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -46,13 +47,13 @@ describe('SEO: Font size audit', () => { }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, false); expect(auditResult.explanation).toBeDisplayString('41% of text is too small.'); expect(auditResult.displayValue).toBeDisplayString('59% legible text'); }); - it('passes when there is no text', () => { + it('passes when there is no text', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -67,11 +68,11 @@ describe('SEO: Font size audit', () => { }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, true); }); - it('passes when more than 60% of text is legible', () => { + it('passes when more than 60% of text is legible', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -86,12 +87,12 @@ describe('SEO: Font size audit', () => { ], }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, true); expect(auditResult.displayValue).toBeDisplayString('90% legible text'); }); - it('groups entries with same source, sorts them by coverage', () => { + it('groups entries with same source, sorts them by coverage', async () => { const style1 = { styleSheetId: 1, type: 'Regular', @@ -123,7 +124,7 @@ describe('SEO: Font size audit', () => { ], }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, false); assert.equal(auditResult.details.items.length, 2); @@ -131,7 +132,7 @@ describe('SEO: Font size audit', () => { expect(auditResult.displayValue).toBeDisplayString('0% legible text'); }); - it('adds a category for failing text that wasn\'t analyzed', () => { + it('adds a category for failing text that wasn\'t analyzed', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -145,7 +146,7 @@ describe('SEO: Font size audit', () => { ], }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, false); assert.equal(auditResult.details.items.length, 3); assert.equal(auditResult.details.items[1].source, 'Add\'l illegible text'); @@ -153,7 +154,7 @@ describe('SEO: Font size audit', () => { expect(auditResult.displayValue).toBeDisplayString('50% legible text'); }); - it('informs user if audit haven\'t covered all text on the page', () => { + it('informs user if audit haven\'t covered all text on the page', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -167,14 +168,14 @@ describe('SEO: Font size audit', () => { ], }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); assert.equal(auditResult.rawValue, false); expect(auditResult.explanation) .toBeDisplayString('100% of text is too small (based on 50% sample).'); expect(auditResult.displayValue).toBeDisplayString('0% legible text'); }); - it('maintains 2 trailing decimal places', () => { + it('maintains 2 trailing decimal places', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -189,11 +190,11 @@ describe('SEO: Font size audit', () => { ], }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); expect(auditResult.displayValue).toBeDisplayString('89.78% legible text'); }); - it('maintains 2 trailing decimal places with only 1 leading digit', () => { + it('maintains 2 trailing decimal places with only 1 leading digit', async () => { const artifacts = { URL, MetaElements: makeMetaElements(validViewport), @@ -208,7 +209,7 @@ describe('SEO: Font size audit', () => { ], }, }; - const auditResult = FontSizeAudit.audit(artifacts); + const auditResult = await FontSizeAudit.audit(artifacts, fakeContext); expect(auditResult.displayValue).toBeDisplayString('2.48% legible text'); }); }); diff --git a/lighthouse-core/test/audits/seo/tap-targets-test.js b/lighthouse-core/test/audits/seo/tap-targets-test.js index 679e702bb7b6..3d5581912a63 100644 --- a/lighthouse-core/test/audits/seo/tap-targets-test.js +++ b/lighthouse-core/test/audits/seo/tap-targets-test.js @@ -10,7 +10,9 @@ const TapTargetsAudit = require('../../../audits/seo/tap-targets.js'); const assert = require('assert'); -function auditTapTargets(tapTargets, metaElements = [{ +const fakeContext = {computedCache: new Map()}; + +async function auditTapTargets(tapTargets, metaElements = [{ name: 'viewport', content: 'width=device-width', }]) { @@ -19,7 +21,7 @@ function auditTapTargets(tapTargets, metaElements = [{ MetaElements: metaElements, }; - return TapTargetsAudit.audit(artifacts); + return TapTargetsAudit.audit(artifacts, fakeContext); } const tapTargetSize = 10; @@ -113,27 +115,27 @@ function getBorderlineTapTargets(options = {}) { } describe('SEO: Tap targets audit', () => { - it('passes when there are no tap targets', () => { - const auditResult = auditTapTargets([]); + it('passes when there are no tap targets', async () => { + const auditResult = await auditTapTargets([]); assert.equal(auditResult.rawValue, true); expect(auditResult.displayValue).toBeDisplayString('100% appropriately sized tap targets'); assert.equal(auditResult.score, 1); }); - it('passes when tap targets don\'t overlap', () => { - const auditResult = auditTapTargets(getBorderlineTapTargets()); + it('passes when tap targets don\'t overlap', async () => { + const auditResult = await auditTapTargets(getBorderlineTapTargets()); assert.equal(auditResult.rawValue, true); }); - it('passes when a target is fully contained in an overlapping target', () => { - const auditResult = auditTapTargets(getBorderlineTapTargets({ + it('passes when a target is fully contained in an overlapping target', async () => { + const auditResult = await auditTapTargets(getBorderlineTapTargets({ addFullyContainedTapTarget: true, })); assert.equal(auditResult.rawValue, true); }); - it('fails if two tap targets overlaps each other horizontally', () => { - const auditResult = auditTapTargets( + it('fails if two tap targets overlaps each other horizontally', async () => { + const auditResult = await auditTapTargets( getBorderlineTapTargets({ overlapRight: true, }) @@ -152,8 +154,8 @@ describe('SEO: Tap targets audit', () => { assert.equal(failure.height, 10); }); - it('fails if a tap target overlaps vertically', () => { - const auditResult = auditTapTargets( + it('fails if a tap target overlaps vertically', async () => { + const auditResult = await auditTapTargets( getBorderlineTapTargets({ overlapBelow: true, }) @@ -161,8 +163,8 @@ describe('SEO: Tap targets audit', () => { assert.equal(auditResult.rawValue, false); }); - it('fails when one of the client rects overlaps', () => { - const auditResult = auditTapTargets( + it('fails when one of the client rects overlaps', async () => { + const auditResult = await auditTapTargets( getBorderlineTapTargets({ overlapSecondClientRect: true, }) @@ -170,25 +172,26 @@ describe('SEO: Tap targets audit', () => { assert.equal(auditResult.rawValue, false); }); - it('reports 2 items if the main target is overlapped both vertically and horizontally', () => { + it('reports 2 items if the main target is overlapped both vertically and horizontally', + async () => { // Main is overlapped by right + below, right and below are each overlapped by main - const auditResult = auditTapTargets( + const auditResult = await auditTapTargets( getBorderlineTapTargets({ overlapRight: true, reduceRightWidth: true, overlapBelow: true, }) - ); - assert.equal(Math.round(auditResult.score * 100), 0); // all tap targets are overlapped by something - const failures = auditResult.details.items; - assert.equal(failures.length, 2); - // Right and Main overlap each other, but Right has a worse score because it's smaller - // so it's the failure that appears in the report - assert.equal(failures[0].tapTarget.snippet, ''); - }); + ); + assert.equal(Math.round(auditResult.score * 100), 0); // all tap targets are overlapped by something + const failures = auditResult.details.items; + assert.equal(failures.length, 2); + // Right and Main overlap each other, but Right has a worse score because it's smaller + // so it's the failure that appears in the report + assert.equal(failures[0].tapTarget.snippet, ''); + }); - it('reports 1 failure if only one tap target involved in an overlap fails', () => { - const auditResult = auditTapTargets( + it('reports 1 failure if only one tap target involved in an overlap fails', async () => { + const auditResult = await auditTapTargets( getBorderlineTapTargets({ overlapRight: true, increaseRightWidth: true, @@ -200,8 +203,8 @@ describe('SEO: Tap targets audit', () => { assert.equal(failures[0].tapTarget.snippet, '
'); }); - it('fails if no meta viewport tag is provided', () => { - const auditResult = auditTapTargets([], []); + it('fails if no meta viewport tag is provided', async () => { + const auditResult = await auditTapTargets([], []); assert.equal(auditResult.rawValue, false); expect(auditResult.explanation).toBeDisplayString( diff --git a/lighthouse-core/test/audits/viewport-test.js b/lighthouse-core/test/audits/viewport-test.js index 8756eb491a6d..d0db78830fc3 100644 --- a/lighthouse-core/test/audits/viewport-test.js +++ b/lighthouse-core/test/audits/viewport-test.js @@ -12,68 +12,69 @@ const assert = require('assert'); describe('Mobile-friendly: viewport audit', () => { const makeMetaElements = viewport => [{name: 'viewport', content: viewport}]; + const fakeContext = {computedCache: new Map()}; - it('fails when HTML does not contain a viewport meta tag', () => { - return assert.equal(Audit.audit({ + it('fails when HTML does not contain a viewport meta tag', async () => { + const auditResult = await Audit.audit({ MetaElements: [], - }).rawValue, false); + }, fakeContext); + return assert.equal(auditResult.rawValue, false); }); - it('fails when HTML contains a non-mobile friendly viewport meta tag', () => { + it('fails when HTML contains a non-mobile friendly viewport meta tag', async () => { const viewport = 'maximum-scale=1'; - assert.equal(Audit.audit({MetaElements: makeMetaElements(viewport)}).rawValue, false); - assert.equal(Audit.audit({ - MetaElements: makeMetaElements(viewport), - }).warnings[0], undefined); + const auditResult = await Audit.audit({MetaElements: makeMetaElements(viewport)}, fakeContext); + assert.equal(auditResult.rawValue, false); + assert.equal(auditResult.warnings[0], undefined); }); - it('fails when HTML contains an invalid viewport meta tag key', () => { + it('fails when HTML contains an invalid viewport meta tag key', async () => { const viewport = 'nonsense=true'; - assert.equal(Audit.audit({MetaElements: makeMetaElements(viewport)}).rawValue, false); - assert.equal(Audit.audit({ - MetaElements: makeMetaElements(viewport), - }).warnings[0], 'Invalid properties found: {"nonsense":"true"}'); + const auditResult = await Audit.audit({MetaElements: makeMetaElements(viewport)}, fakeContext); + assert.equal(auditResult.rawValue, false); + assert.equal(auditResult.warnings[0], 'Invalid properties found: {"nonsense":"true"}'); }); - it('fails when HTML contains an invalid viewport meta tag value', () => { + it('fails when HTML contains an invalid viewport meta tag value', async () => { const viewport = 'initial-scale=microscopic'; - assert.equal(Audit.audit({MetaElements: makeMetaElements(viewport)}).rawValue, false); - assert.equal(Audit.audit({ - MetaElements: makeMetaElements(viewport), - }).warnings[0], 'Invalid values found: {"initial-scale":"microscopic"}'); + const auditResult = await Audit.audit({MetaElements: makeMetaElements(viewport)}, fakeContext); + assert.equal(auditResult.rawValue, false); + assert.equal(auditResult.warnings[0], 'Invalid values found: {"initial-scale":"microscopic"}'); }); - it('fails when HTML contains an invalid viewport meta tag key and value', () => { + it('fails when HTML contains an invalid viewport meta tag key and value', async () => { const viewport = 'nonsense=true, initial-scale=microscopic'; - const {rawValue, warnings} = Audit.audit({MetaElements: makeMetaElements(viewport)}); + const {rawValue, warnings} = + await Audit.audit({MetaElements: makeMetaElements(viewport)}, fakeContext); assert.equal(rawValue, false); assert.equal(warnings[0], 'Invalid properties found: {"nonsense":"true"}'); assert.equal(warnings[1], 'Invalid values found: {"initial-scale":"microscopic"}'); }); - it('passes when a valid viewport is provided', () => { + it('passes when a valid viewport is provided', async () => { const viewports = [ 'width=device-width, initial-scale=1, minimum-scale=1, maximum-scale=1', 'width = device-width, initial-scale = 1', 'initial-scale=1', 'width=device-width ', ]; - viewports.forEach(viewport => { - assert.equal(Audit.audit({ + await Promise.all(viewports.map(async viewport => { + const auditResult = await Audit.audit({ MetaElements: makeMetaElements(viewport), - }).rawValue, true); - }); + }, fakeContext); + assert.equal(auditResult.rawValue, true); + })); }); - it('doesn\'t throw when viewport contains "invalid" iOS properties', () => { + it('doesn\'t throw when viewport contains "invalid" iOS properties', async () => { const viewports = [ 'width=device-width, shrink-to-fit=no', 'width=device-width, viewport-fit=cover', ]; - viewports.forEach(viewport => { - const result = Audit.audit({MetaElements: makeMetaElements(viewport)}); + await Promise.all(viewports.map(async viewport => { + const result = await Audit.audit({MetaElements: makeMetaElements(viewport)}, fakeContext); assert.equal(result.rawValue, true); assert.equal(result.warnings[0], undefined); - }); + })); }); }); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 9dca79bb4160..999e52064abf 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -348,6 +348,15 @@ declare global { }[]; } + export type ViewportMeta = { + /** Whether the page has any viewport tag */ + hasViewportTag: boolean; + /** Whether the viewport tag is optimized for mobile screens */ + hasMobileViewport: boolean; + /** Warnings if the parser encountered invalid content in the viewport tag */ + parserWarnings: string[]; + } + export interface MeasureEntry extends PerformanceEntry { /** Whether timing entry was collected during artifact gathering. */ gather?: boolean;