From a04ab43f03e4f79fc598f3ee259d98ac900c6e1c Mon Sep 17 00:00:00 2001 From: Matt Gaunt Date: Thu, 24 Mar 2016 14:33:17 +0000 Subject: [PATCH] Adding audit for theme-color in HTML and moving to jsdom for testing html elements --- audits/html/theme-color.js | 48 +++++++++++++++++ audits/mobile-friendly/viewport.js | 15 ++++-- extension/.eslintrc | 6 +-- extension/gulpfile.babel.js | 4 +- gatherers/html.js | 18 ++++++- index.js | 3 +- package.json | 1 + test/audits/html/theme-colors.js | 72 +++++++++++++++++++++++++ test/audits/mobile-friendly/viewport.js | 40 ++++++++------ test/helpers/mock-html.js | 24 +++++++++ 10 files changed, 206 insertions(+), 25 deletions(-) create mode 100644 audits/html/theme-color.js create mode 100644 test/audits/html/theme-colors.js create mode 100644 test/helpers/mock-html.js diff --git a/audits/html/theme-color.js b/audits/html/theme-color.js new file mode 100644 index 000000000000..c94acafb8324 --- /dev/null +++ b/audits/html/theme-color.js @@ -0,0 +1,48 @@ +/** + * Copyright 2016 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'; + +class ThemeColor { + + static get tags() { + return ['HTML']; + } + + static get description() { + return 'Site has a theme-color meta tag'; + } + + static audit(inputs) { + let hasThemeColor = false; + if (inputs.window) { + const themeColorElements = + inputs.window.document.querySelectorAll('head meta[name="theme-color"]'); + + if (themeColorElements.length === 1 && + themeColorElements[0].getAttribute('content').length > 0) { + hasThemeColor = true; + } + } + + return { + value: hasThemeColor, + tags: ThemeColor.tags, + description: ThemeColor.description + }; + } +} + +module.exports = ThemeColor; diff --git a/audits/mobile-friendly/viewport.js b/audits/mobile-friendly/viewport.js index 6653f0608704..bb1b9f228634 100644 --- a/audits/mobile-friendly/viewport.js +++ b/audits/mobile-friendly/viewport.js @@ -26,12 +26,19 @@ class Viewport { } static audit(inputs) { - // TODO: This'll be a false positive if the body content has this string. - // Unless we want to parse the original HTML, we should query for `head > .meta[name=viewport]` - const viewportExpression = / { .pipe($.babel({ presets: ['es2015'] })) - .pipe(browserify()) + .pipe(browserify({ + ignore: 'jsdom' // Ignore jsdom since it's only needed for node env + })) .pipe(gulp.dest('app/scripts')) .pipe(gulp.dest('dist/scripts')); }); diff --git a/gatherers/html.js b/gatherers/html.js index 9bbcce644176..f79ebb215019 100644 --- a/gatherers/html.js +++ b/gatherers/html.js @@ -15,7 +15,10 @@ */ 'use strict'; +/* global window */ + const Gather = require('./gather'); +const jsdom = require('jsdom').jsdom; class HTML extends Gather { @@ -27,7 +30,20 @@ class HTML extends Gather { .then(nodeId => driver.sendCommand('DOM.getOuterHTML', { nodeId: nodeId })) - .then(nodeHTML => ({html: nodeHTML.outerHTML})); + .then(nodeHTML => { + if (typeof window !== 'undefined') { + return { + window: window, + html: nodeHTML + }; + } + + const doc = jsdom(nodeHTML); + return { + window: doc.defaultView, + html: nodeHTML + }; + }); } } diff --git a/index.js b/index.js index ac5bbd28a827..a2be173d34a2 100644 --- a/index.js +++ b/index.js @@ -42,7 +42,8 @@ const audits = [ require('./audits/manifest/icons-192'), require('./audits/manifest/name'), require('./audits/manifest/short-name'), - require('./audits/manifest/start-url') + require('./audits/manifest/start-url'), + require('./audits/html/theme-color') ]; module.exports = function(opts) { diff --git a/package.json b/package.json index aed362276cb3..9d0d975aa29c 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "chrome-devtools-frontend": "1.0.381789", "chrome-remote-interface": "^0.11.0", "devtools-timeline-model": "1.0.16", + "jsdom": "^8.1.0", "meow": "^3.7.0", "npmlog": "^2.0.3", "traceviewer": "^1.0.8" diff --git a/test/audits/html/theme-colors.js b/test/audits/html/theme-colors.js new file mode 100644 index 000000000000..3b0bb32fe802 --- /dev/null +++ b/test/audits/html/theme-colors.js @@ -0,0 +1,72 @@ +/** + * Copyright 2016 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. + */ +const Audit = require('../../../audits/html/theme-color.js'); +const assert = require('assert'); +const mockHtml = require('../../helpers/mock-html.js'); + +/* global describe, it*/ + +describe('HTML: theme-color audit', () => { + it('fails when no window or html present', () => { + return assert.equal(Audit.audit({}).value, false); + }); + + it('fails when invalid HTML and window given', () => { + return assert.equal(Audit.audit({ + window: null, + html: null + }).value, false); + }); + + it('fails when no theme-color is present in the html', () => { + return assert.equal(Audit.audit(mockHtml()).value, false); + }); + + it('fails when theme-color has no content value', () => { + return assert.equal(Audit.audit( + mockHtml('')).value, false); + }); + + it('fails when multiple theme-colors exist', () => { + return assert.equal(Audit.audit( + mockHtml(` + `)).value, false); + }); + + it('fails when theme-color exists in the body', () => { + return assert.equal(Audit.audit( + mockHtml('', '') + ).value, false); + }); + + it('succeeds when theme-color present in the html', () => { + return assert.equal(Audit.audit( + mockHtml('') + ).value, true); + }); + + it('succeeds when theme-color present in the html with id', () => { + return assert.equal(Audit.audit( + mockHtml('') + ).value, true); + }); + + it('succeeds when theme-color has a CSS name content value', () => { + return assert.equal(Audit.audit( + mockHtml('') + ).value, true); + }); +}); diff --git a/test/audits/mobile-friendly/viewport.js b/test/audits/mobile-friendly/viewport.js index 6d0af05e0dd5..09726090f61c 100644 --- a/test/audits/mobile-friendly/viewport.js +++ b/test/audits/mobile-friendly/viewport.js @@ -15,6 +15,7 @@ */ const Audit = require('../../../audits/mobile-friendly/viewport.js'); const assert = require('assert'); +const mockHtml = require('../../helpers/mock-html.js'); /* global describe, it*/ @@ -25,31 +26,40 @@ describe('Mobile-friendly: viewport audit', () => { return assert.equal(Audit.audit({}).value, false); }); - it('fails when invalid HTML given', () => { + it('fails when invalid HTML and window given', () => { return assert.equal(Audit.audit({ + window: null, html: null }).value, false); }); it('fails when HTML does not contain a viewport meta tag', () => { - return assert.equal(Audit.audit({ - html: '' - }).value, false); + return assert.equal(Audit.audit(mockHtml()).value, false); + }); + + it('fails when a viewport is in the body', () => { + return assert.equal(Audit.audit( + mockHtml('', '') + ).value, false); + }); + + it('fails when multiple viewports defined', () => { + return assert.equal(Audit.audit( + mockHtml(` + `) + ).value, false); }); it('passes when a viewport is provided', () => { - return assert.equal(Audit.audit({ - html: ` - - - - Sample page - - - ` - }).value, true); + return assert.equal(Audit.audit( + mockHtml('') + ).value, true); }); - // TODO: add test for ensuring the meta tag is in the head. + it('passes when a viewport is provided with an id', () => { + return assert.equal(Audit.audit( + mockHtml('') + ).value, true); + }); }); /* eslint-enable */ diff --git a/test/helpers/mock-html.js b/test/helpers/mock-html.js new file mode 100644 index 000000000000..bc000f0482c7 --- /dev/null +++ b/test/helpers/mock-html.js @@ -0,0 +1,24 @@ +const jsdom = require('jsdom').jsdom; + +module.exports = (headInsert, bodyInsert) => { + headInsert = headInsert || ''; + bodyInsert = bodyInsert || ''; + + const htmlString = ` + + + ${headInsert} + Sample page + + + ${bodyInsert} + + `; + + const doc = jsdom(htmlString); + + return { + window: doc.defaultView, + html: htmlString + }; +};