diff --git a/front/src/css/rule.css b/front/src/css/rule.css index ccc72a5..b9b3014 100644 --- a/front/src/css/rule.css +++ b/front/src/css/rule.css @@ -214,6 +214,17 @@ z-index: 2; border: 0.2em solid #f1c40f; } +.similarColors { + margin: 1em; + width: 20em; + height: 6em; +} +.similarColors > div { + display: inline-block; + width: 10em; + height: 3.5em; + padding-top: 2.5em; +} .totalWeightPie { max-width: 39em; margin: 2em auto 4em; @@ -244,8 +255,9 @@ margin-top: 0.5em; } .smallPreview { - max-height: 1.6em; - max-width: 4em; + display: block; + max-height: 4em; + max-width: 8em; border: 1px solid #000; - margin-top: 0.2em; + margin: 1em auto 0.2em; } diff --git a/front/src/js/directives/offendersDirectives.js b/front/src/js/directives/offendersDirectives.js index d122393..3ed389a 100644 --- a/front/src/js/directives/offendersDirectives.js +++ b/front/src/js/directives/offendersDirectives.js @@ -608,7 +608,12 @@ for (var i = 0 ; i < parsedBacktrace.length ; i++) { html += '
'; html += '
' + (parsedBacktrace[i].fnName || '(anonymous function)') + '
'; - html += '
' + getUrlLink(parsedBacktrace[i].filePath, 40) + ':' + parsedBacktrace[i].line + '
'; + html += '
' + getUrlLink(parsedBacktrace[i].filePath, 40) + '
'; + if (parsedBacktrace[i].column) { + html += '
' + parsedBacktrace[i].line + ':' + parsedBacktrace[i].column + '
'; + } else { + html += '
line ' + parsedBacktrace[i].line + '
'; + } html += '
'; } } @@ -630,6 +635,17 @@ var fnName = null, fileAndLine; var withFnResult = /^([^\s\(]+) \((.+:\d+)\)$/.exec(trace); + + if (withFnResult === null) { + // Try the PhantomJS 2 format + withFnResult = /^([^\s\(]+) \((.+:\d+:\d+)\)$/.exec(trace); + } + + if (withFnResult === null) { + // Try the PhantomJS 2 ERROR format + withFnResult = /^([^\s\(]+) (http.+:\d+)$/.exec(trace); + } + if (withFnResult === null) { fileAndLine = trace; } else { @@ -637,14 +653,22 @@ fileAndLine = withFnResult[2]; } - var fileAndLineSplit = /^(.*):(\d+)$/.exec(fileAndLine); + // And now the second part + var fileAndLineSplit = /^(.*):(\d+):(\d+)$/.exec(fileAndLine); + + if (fileAndLineSplit === null) { + fileAndLineSplit = /^(.*):(\d+)$/.exec(fileAndLine); + } + var filePath = fileAndLineSplit[1]; var line = fileAndLineSplit[2]; + var column = fileAndLineSplit[3]; out.push({ fnName: fnName, filePath: filePath, - line: line + line: line, + column: column }); }); diff --git a/front/src/less/rule.less b/front/src/less/rule.less index 7a2571f..c3da1e9 100644 --- a/front/src/less/rule.less +++ b/front/src/less/rule.less @@ -234,6 +234,19 @@ } } +.similarColors { + margin: 1em; + width: 20em; + height: 6em; + + > div { + display: inline-block; + width: 10em; + height: 3.5em; + padding-top: 2.5em; + } +} + .totalWeightPie { max-width: 39em; margin: 2em auto 4em; @@ -271,8 +284,9 @@ } .smallPreview { - max-height: 1.6em; - max-width: 4em; + display: block; + max-height: 4em; + max-width: 8em; border: 1px solid #000; - margin-top: 0.2em; + margin: 1em auto 0.2em; } \ No newline at end of file diff --git a/front/src/views/rule.html b/front/src/views/rule.html index b484ead..001b0f7 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -96,6 +96,10 @@ +
+
{{offender.color1}}
{{offender.color2}}
+
+
{{offender.error}} @@ -145,9 +149,9 @@
-
- +
+
diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index e7aab2c..ae59880 100644 --- a/lib/metadata/policies.js +++ b/lib/metadata/policies.js @@ -463,7 +463,7 @@ var policies = { }, "cssColors": { "tool": "phantomas", - "label": "Different colors", + "label": "Colors count", "message": "

This is the number of different colors defined in CSS.

Your CSS project will be easier to maintain if you keep a small color set.

", "isOkThreshold": 30, "isBadThreshold": 150, @@ -507,6 +507,21 @@ var policies = { }; } }, + "similarColors": { + "tool": "colorDiff", + "label": "Similar colors", + "message": "

This is the list of colors found in the stylesheets, that are very close to each other. The eye can barely see the difference.

Use this list to reduce the number of colors in your palette, it will be easier to maintain.

", + "isOkThreshold": 0, + "isBadThreshold": 40, + "isAbnormalThreshold": 80, + "hasOffenders": true, + "offendersTransformFn": function(offenders) { + return { + count: offenders.length, + list: offenders + }; + } + }, "cssImports": { "tool": "phantomas", "label": "Uses of @import", @@ -959,13 +974,22 @@ var policies = { }, "lazyLoadableImagesBelowTheFold": { "tool": "phantomas", - "label": "Not lazyloaded images", + "label": "Below the fold images", "message": "

This is the number of images displayed below the fold that could be lazy-loaded. This is an excellent way to accelerate the loading time of an heavy page.

I recommend using this lazyloader.

", "isOkThreshold": 1, "isBadThreshold": 12, "isAbnormalThreshold": 30, "hasOffenders": true }, + "hiddenImages": { + "tool": "phantomas", + "label": "Hidden images", + "message": "

List of all images that have a display:none property, or one of their parents. These images are loaded by the browser even if they're not visible. You might be able to find a way to lazy-load them, only when they get visible.

Trackers are an exception, you'd better hide them.

", + "isOkThreshold": 1, + "isBadThreshold": 12, + "isAbnormalThreshold": 30, + "hasOffenders": true + }, "cachingDisabled": { "tool": "phantomas", "label": "Caching disabled", diff --git a/lib/metadata/scoreProfileGeneric.json b/lib/metadata/scoreProfileGeneric.json index eed3045..30230ec 100644 --- a/lib/metadata/scoreProfileGeneric.json +++ b/lib/metadata/scoreProfileGeneric.json @@ -17,7 +17,8 @@ "notFound": 3, "multipleRequests": 2, "smallRequests": 1, - "lazyLoadableImagesBelowTheFold": 2 + "lazyLoadableImagesBelowTheFold": 2, + "hiddenImages": 1 } }, "domComplexity": { @@ -69,7 +70,8 @@ "cssRules": 2, "cssComplexSelectors": 2, "cssComplexSelectorsByAttribute": 1.5, - "cssColors": 0.5 + "cssColors": 0.5, + "similarColors": 0.5 } }, "badCSS": { diff --git a/lib/runner.js b/lib/runner.js index 786286d..f35879d 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -3,6 +3,7 @@ var debug = require('debug')('ylt:runner'); var phantomasWrapper = require('./tools/phantomas/phantomasWrapper'); var jsExecutionTransformer = require('./tools/jsExecutionTransformer'); +var colorDiff = require('./tools/colorDiff'); var weightChecker = require('./tools/weightChecker/weightChecker'); var rulesChecker = require('./rulesChecker'); var scoreCalculator = require('./scoreCalculator'); @@ -28,6 +29,9 @@ var Runner = function(params) { // Treat the JS Execution Tree from offenders data = jsExecutionTransformer.transform(data); + // Compare colors + data = colorDiff.compareAllColors(data); + // Redownload every file return weightChecker.recheckAllFiles(data); diff --git a/lib/tools/colorDiff.js b/lib/tools/colorDiff.js new file mode 100644 index 0000000..bf9cdae --- /dev/null +++ b/lib/tools/colorDiff.js @@ -0,0 +1,90 @@ +var debug = require('debug')('ylt:colorDiff'); +var diff = require('color-diff'); +var parse = require('parse-color'); + + +var colorDiff = function() { + + var COLOR_DIFF_THRESHOLD = 0.9; + + this.compareAllColors = function(data) { + debug('Starting to compare all colors...'); + + var offenders = data.toolsResults.phantomas.offenders.cssColors; + var colors = (offenders) ? this.parseAllColors(offenders) : []; + var result = []; + + // Compare colors to each other + for (var i = 0; i < colors.length - 1 ; i++) { + for (var j = i + 1; j < colors.length; j++) { + debug('Comparing color %s with color %s', colors[i].original, colors[j].original); + if (this.compareTwoColors(colors[i], colors[j])) { + debug('Colors are similar!'); + result.push({ + color1: colors[i].original, + color2: colors[j].original, + isDark: (colors[i].Lab.L < 60) + }); + } + } + } + + data.toolsResults.colorDiff = { + metrics: { + similarColors: result.length + }, + offenders: { + similarColors: result + } + }; + + return data; + }; + + this.parseAllColors = function(offenders) { + var parsedOffenders = offenders.map(this.parseOffender); + var deduplicatedColors = {}; + + parsedOffenders.forEach(function(color) { + deduplicatedColors[color] = color; + }); + + return Object.keys(deduplicatedColors).map(this.parseColor); + }; + + this.parseOffender = function(offender) { + var regexResult = /^(.*) \(\d+ times\)$/.exec(offender); + return regexResult[1]; + }; + + this.parseColor = function(color) { + var colorArray = parse(color).rgba; + + var obj = { + R: colorArray[0], + G: colorArray[1], + B: colorArray[2], + A: colorArray[3] + }; + + obj.Lab = diff.rgb_to_lab(obj); + obj.original = color; + + return obj; + }; + + this.compareTwoColors = function(color1, color2) { + if (color1.A !== color2.A) { + debug('Comparison not possible, because the alpha channel is different'); + return false; + } + + var delta = diff.diff(color1.Lab, color2.Lab); + debug('Delta is %d', delta); + + return delta <= COLOR_DIFF_THRESHOLD; + }; + +}; + +module.exports = new colorDiff(); \ No newline at end of file diff --git a/lib/tools/phantomas/custom_modules/modules/domHiddenYLT/domHiddenYLT.js b/lib/tools/phantomas/custom_modules/modules/domHiddenYLT/domHiddenYLT.js index 56aafd5..097aef7 100644 --- a/lib/tools/phantomas/custom_modules/modules/domHiddenYLT/domHiddenYLT.js +++ b/lib/tools/phantomas/custom_modules/modules/domHiddenYLT/domHiddenYLT.js @@ -3,21 +3,23 @@ */ /* global document: true, Node: true, window: true */ -exports.version = '0.1.a'; +exports.version = '1.0.a'; exports.module = function(phantomas) { 'use strict'; // total length of HTML of hidden elements (i.e. display: none) phantomas.setMetric('hiddenContentSize'); // @desc the size of content of hidden elements on the page (with CSS display: none) @offenders + phantomas.setMetric('hiddenImages'); // @desc number of hidden images that can be lazy-loaded @offenders // HTML size phantomas.on('report', function() { phantomas.evaluate(function() { (function(phantomas) { - phantomas.spyEnabled(false, 'checking the hiddenContentSize'); + var runner = new phantomas.nodeRunner(), + lazyLoadableImages = {}; - var runner = new phantomas.nodeRunner(); + phantomas.spyEnabled(false, 'analyzing hidden content'); runner.walk(document.body, function(node, depth) { switch (node.nodeType) { @@ -36,6 +38,29 @@ exports.module = function(phantomas) { } } + // count hidden images that can be lazy loaded (issue #524) + var images = []; + if (node.tagName === 'IMG') { + images = [node]; + } else if (typeof node.querySelectorAll === 'function') { + images = node.querySelectorAll('img') || []; + } + + for (var i = 0, len = images.length; i < len; i++) { + var src = images[i].src, + path; + + if (src === '' || src.indexOf('data:image') === 0) continue; + + if (!lazyLoadableImages[src]) { + path = phantomas.getDOMPath(images[i]); + + lazyLoadableImages[src] = { + path: path + }; + } + } + // don't run for child nodes as they're hidden as well return false; } @@ -43,6 +68,15 @@ exports.module = function(phantomas) { } }); + Object.keys(lazyLoadableImages).forEach(function(img) { + var entry = lazyLoadableImages[img]; + + phantomas.incrMetric('hiddenImages'); + phantomas.addOffender('hiddenImages', img); + + phantomas.log('hiddenImages: <%s> image (%s) is hidden and can be lazy-loaded', img, entry.path); + }); + phantomas.spyEnabled(true); }(window.__phantomas)); }); diff --git a/package.json b/package.json index 3a792b5..1e15436 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "yellowlabtools", - "version": "1.7.8", + "version": "1.8.0", "description": "Online tool to audit a webpage for performance and front-end quality issues", "license": "GPL-2.0", "author": { @@ -19,15 +19,16 @@ "dependencies": { "angular": "1.4.5", "angular-animate": "1.4.5", - "angular-chart.js": "0.8.3", + "angular-chart.js": "0.8.4", "angular-local-storage": "0.2.2", "angular-resource": "1.4.5", "angular-route": "1.4.5", "angular-sanitize": "1.4.5", "async": "1.4.2", - "body-parser": "1.13.3", + "body-parser": "1.14.0", "chart.js": "1.0.2", - "clean-css": "3.4.2", + "clean-css": "3.4.3", + "color-diff": "0.1.7", "compression": "1.5.2", "cors": "2.7.1", "debug": "2.2.0", @@ -38,10 +39,11 @@ "lwip": "0.0.7", "meow": "3.3.0", "minimize": "1.7.1", + "parse-color": "1.0.0", "phantomas": "1.12.0", "ps-node": "0.0.4", "q": "1.4.1", - "request": "2.61.0", + "request": "2.62.0", "rimraf": "2.4.3", "temporary": "0.0.8", "uglify-js": "2.4.24" @@ -53,7 +55,7 @@ "grunt-contrib-clean": "~0.6.0", "grunt-contrib-concat": "~0.5.1", "grunt-contrib-copy": "~0.8.1", - "grunt-contrib-cssmin": "~0.13.0", + "grunt-contrib-cssmin": "~0.14.0", "grunt-contrib-htmlmin": "~0.4.0", "grunt-contrib-jshint": "~0.11.3", "grunt-contrib-less": "~1.0.1", diff --git a/test/core/colorDiffTest.js b/test/core/colorDiffTest.js new file mode 100644 index 0000000..d26aad1 --- /dev/null +++ b/test/core/colorDiffTest.js @@ -0,0 +1,92 @@ +var should = require('chai').should(); +var colorDiff = require('../../lib/tools/colorDiff'); + +describe('colorDiff', function() { + + it('should parse offenders correctly', function() { + colorDiff.parseOffender('#000 (1 times)').should.equal('#000'); + colorDiff.parseOffender('#5bc0de (2 times)').should.equal('#5bc0de'); + colorDiff.parseOffender('rgba(0,0,0,0.075) (100 times)').should.equal('rgba(0,0,0,0.075)'); + colorDiff.parseOffender('rgb(91,192,222) (1000 times)').should.equal('rgb(91,192,222)'); + }); + + it('should parse colors correctly', function() { + colorDiff.parseColor('#000').should.deep.equal({R: 0, G: 0, B: 0, A: 1, Lab: {L: 0, a: 0, b: 0}, original: '#000'}); + colorDiff.parseColor('rgba(255,255,255,0.5)').should.deep.equal({R: 255, G: 255, B: 255, A: 0.5, Lab: {L: 100, a: 0.00526049995830391, b: -0.010408184525267927}, original: 'rgba(255,255,255,0.5)'}); + }); + + it('should not compare colors with different alpha', function() { + var color1 = colorDiff.parseColor("rgba(0, 0, 0, 1)"); + var color2 = colorDiff.parseColor("rgba(0, 0, 0, 0.5)"); + + colorDiff.compareTwoColors(color1, color2).should.equal(false); + }); + + it('should find that two colors are similar', function() { + var color1 = colorDiff.parseColor("rgba(0, 0, 0, 1)"); + var color2 = colorDiff.parseColor("rgba(0, 0, 1, 1)"); + + colorDiff.compareTwoColors(color1, color2).should.equal(true); + }); + + it('should find that two colors are similar even with alpha', function() { + var color1 = colorDiff.parseColor("rgba(0, 0, 0, 0.3)"); + var color2 = colorDiff.parseColor("rgba(0, 0, 2, 0.3)"); + + colorDiff.compareTwoColors(color1, color2).should.equal(true); + }); + + it('should find that two colors are different', function() { + var color1 = colorDiff.parseColor("rgba(0, 0, 0, 1)"); + var color2 = colorDiff.parseColor("rgba(99, 99, 99, 1)"); + + colorDiff.compareTwoColors(color1, color2).should.equal(false); + }); + + it('should compare all colors to each other', function() { + var colors = [ + '#000 (1 times)', + '#5bc0de (2 times)', + 'rgba(0,0,0,0.075) (100 times)', + 'rgb(91,192,222) (1000 times)', + 'rgba(0,0,2,1) (1 times)', + 'rgba(99,99,99,1) (1 times)', + 'rgba(100,100,100,1) (1 times)' + ]; + + var data = { + toolsResults: { + phantomas: { + offenders: { + cssColors: colors + } + } + } + }; + + newData = colorDiff.compareAllColors(data); + + newData.toolsResults.should.have.a.property('colorDiff'); + newData.toolsResults.colorDiff.should.have.a.property('metrics'); + newData.toolsResults.colorDiff.metrics.should.have.a.property('similarColors').that.equals(3); + newData.toolsResults.colorDiff.should.have.a.property('offenders'); + newData.toolsResults.colorDiff.offenders.should.have.a.property('similarColors').that.deep.equals([ + { + color1: '#000', + color2: 'rgba(0,0,2,1)', + isDark: true + }, + { + color1: '#5bc0de', + color2: 'rgb(91,192,222)', + isDark: false + }, + { + color1: 'rgba(99,99,99,1)', + color2: 'rgba(100,100,100,1)', + isDark: true + } + ]); + }); + +});