From ffb839835693e12bbc2ebc81e3ab7baa1c8985f5 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Sat, 22 Sep 2018 11:46:41 -0700 Subject: [PATCH] core: use cssstyle to parse CSS colors instead of WebInspector (#6091) --- lighthouse-core/audits/themed-omnibox.js | 12 ++++++++-- lighthouse-core/lib/manifest-parser.js | 15 ++++++++---- lighthouse-core/lib/web-inspector.js | 3 --- .../test/lib/web-inspector-test.js | 1 - package.json | 1 + typings/cssstyle/index.d.ts | 23 +++++++++++++++++++ yarn.lock | 6 +++++ 7 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 typings/cssstyle/index.d.ts diff --git a/lighthouse-core/audits/themed-omnibox.js b/lighthouse-core/audits/themed-omnibox.js index 50534ecac9b1..b3760b7ca85b 100644 --- a/lighthouse-core/audits/themed-omnibox.js +++ b/lighthouse-core/audits/themed-omnibox.js @@ -6,8 +6,8 @@ 'use strict'; const MultiCheckAudit = require('./multi-check-audit'); -const validColor = require('../lib/web-inspector').Color.parse; const ManifestValues = require('../gather/computed/manifest-values'); +const cssParsers = require('cssstyle/lib/parsers'); /** * @fileoverview @@ -34,6 +34,14 @@ class ThemedOmnibox extends MultiCheckAudit { }; } + /** + * @param {string} color + * @return {boolean} + */ + static isValidColor(color) { + return cssParsers.valueType(color) === cssParsers.TYPES.COLOR; + } + /** * @param {LH.Artifacts['ThemeColor']} themeColorMeta * @param {Array} failures @@ -41,7 +49,7 @@ class ThemedOmnibox extends MultiCheckAudit { static assessMetaThemecolor(themeColorMeta, failures) { if (themeColorMeta === null) { failures.push('No `` tag found'); - } else if (!validColor(themeColorMeta)) { + } else if (!ThemedOmnibox.isValidColor(themeColorMeta)) { failures.push('The theme-color meta tag did not contain a valid CSS color'); } } diff --git a/lighthouse-core/lib/manifest-parser.js b/lighthouse-core/lib/manifest-parser.js index 17b208af8573..7468583b8052 100644 --- a/lighthouse-core/lib/manifest-parser.js +++ b/lighthouse-core/lib/manifest-parser.js @@ -6,7 +6,7 @@ 'use strict'; const URL = require('./url-shim'); -const validateColor = require('./web-inspector').Color.parse; +const cssParsers = require('cssstyle/lib/parsers'); const ALLOWED_DISPLAY_VALUES = [ 'fullscreen', @@ -31,6 +31,14 @@ const ALLOWED_ORIENTATION_VALUES = [ 'landscape-secondary', ]; +/** + * @param {string} color + * @return {boolean} + */ +function isValidColor(color) { + return cssParsers.valueType(color) === cssParsers.TYPES.COLOR; +} + /** * @param {*} raw * @param {boolean=} trim @@ -66,9 +74,8 @@ function parseColor(raw) { return color; } - // Use DevTools's color parser to check CSS3 Color parsing. - const validatedColor = validateColor(color.raw); - if (!validatedColor) { + // Use color parser to check CSS3 Color parsing. + if (!isValidColor(color.raw)) { color.value = undefined; color.warning = 'ERROR: color parsing failed.'; } diff --git a/lighthouse-core/lib/web-inspector.js b/lighthouse-core/lib/web-inspector.js index c181fef2c5a5..e9ac86ba5204 100644 --- a/lighthouse-core/lib/web-inspector.js +++ b/lighthouse-core/lib/web-inspector.js @@ -149,9 +149,6 @@ module.exports = (function() { Log: 'log', }; - // Dependencies for color parsing. - require('chrome-devtools-frontend/front_end/common/Color.js'); - // Dependencies for effective CSS rule calculation. require('chrome-devtools-frontend/front_end/common/TextRange.js'); require('chrome-devtools-frontend/front_end/sdk/CSSMatchedStyles.js'); diff --git a/lighthouse-core/test/lib/web-inspector-test.js b/lighthouse-core/test/lib/web-inspector-test.js index 46761aaeecb1..6e5f41e993fa 100644 --- a/lighthouse-core/test/lib/web-inspector-test.js +++ b/lighthouse-core/test/lib/web-inspector-test.js @@ -12,7 +12,6 @@ const assert = require('assert'); describe('Web Inspector lib', function() { it('WebInspector exported is the real one', () => { assert.equal(typeof WebInspector, 'object'); - assert.ok(WebInspector.Color); assert.ok(WebInspector.CSSMetadata); }); diff --git a/package.json b/package.json index f98a3bdec7bf..a97a99ce9165 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,7 @@ "chrome-devtools-frontend": "1.0.422034", "chrome-launcher": "^0.10.4", "configstore": "^3.1.1", + "cssstyle": "1.1.1", "devtools-timeline-model": "1.1.6", "esprima": "^4.0.1", "http-link-header": "^0.8.0", diff --git a/typings/cssstyle/index.d.ts b/typings/cssstyle/index.d.ts new file mode 100644 index 000000000000..06b9a631c2dd --- /dev/null +++ b/typings/cssstyle/index.d.ts @@ -0,0 +1,23 @@ +/** + * @license Copyright 2018 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. + */ + +declare module 'cssstyle/lib/parsers' { + interface TYPES { + INTEGER: 1; + NUMBER: 2; + LENGTH: 3; + PERCENT: 4; + URL: 5; + COLOR: 6; + STRING: 7; + ANGLE: 8; + KEYWORD: 9; + NULL_OR_EMPTY_STR: 10; + } + + export var TYPES: TYPES; + export function valueType(val: any): TYPES[keyof TYPES] | undefined; +} diff --git a/yarn.lock b/yarn.lock index 05e7ae2b380e..836946eac601 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1482,6 +1482,12 @@ cssom@0.3.x, "cssom@>= 0.3.2 < 0.4.0": version "0.3.2" resolved "https://registry.yarnpkg.com/cssom/-/cssom-0.3.2.tgz#b8036170c79f07a90ff2f16e22284027a243848b" +cssstyle@1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/cssstyle/-/cssstyle-1.1.1.tgz#18b038a9c44d65f7a8e428a653b9f6fe42faf5fb" + dependencies: + cssom "0.3.x" + "cssstyle@>= 0.2.37 < 0.3.0": version "0.2.37" resolved "https://registry.yarnpkg.com/cssstyle/-/cssstyle-0.2.37.tgz#541097234cb2513c83ceed3acddc27ff27987d54"