From 46c2d8c49e41a4cef39dc769e6d3c312f9a3a8ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Fri, 18 Sep 2015 13:49:24 +0200 Subject: [PATCH 1/5] Fix backtraces in the profiler when using PhantomJS 2 --- .../src/js/directives/offendersDirectives.js | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) 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 }); }); From d32b9f78a41362eb8158317228ee847ca1f5ca66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Fri, 18 Sep 2015 18:51:41 +0200 Subject: [PATCH 2/5] New metric: hiddenImages --- front/src/css/rule.css | 7 ++-- front/src/less/rule.less | 7 ++-- front/src/views/rule.html | 4 +- lib/metadata/policies.js | 11 ++++- lib/metadata/scoreProfileGeneric.json | 3 +- .../modules/domHiddenYLT/domHiddenYLT.js | 40 +++++++++++++++++-- 6 files changed, 59 insertions(+), 13 deletions(-) diff --git a/front/src/css/rule.css b/front/src/css/rule.css index ccc72a5..24137f4 100644 --- a/front/src/css/rule.css +++ b/front/src/css/rule.css @@ -244,8 +244,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/less/rule.less b/front/src/less/rule.less index 7a2571f..6f1fe61 100644 --- a/front/src/less/rule.less +++ b/front/src/less/rule.less @@ -271,8 +271,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..0ca11f6 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -145,9 +145,9 @@ -
- +
+
diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index e7aab2c..fc6bbf0 100644 --- a/lib/metadata/policies.js +++ b/lib/metadata/policies.js @@ -959,13 +959,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": 3, + "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..8dd9670 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": { 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)); }); From 453a5cda2e1103cb1d0f52f214a3751691875b38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Sat, 19 Sep 2015 11:55:38 +0200 Subject: [PATCH 3/5] Stricter hiddenImages rule --- lib/metadata/policies.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index fc6bbf0..e3b1127 100644 --- a/lib/metadata/policies.js +++ b/lib/metadata/policies.js @@ -970,7 +970,7 @@ var policies = { "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": 3, + "isOkThreshold": 1, "isBadThreshold": 12, "isAbnormalThreshold": 30, "hasOffenders": true From d0257fea8fa3ec85ff4be276cc511185fc284369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Sat, 19 Sep 2015 11:55:38 +0200 Subject: [PATCH 4/5] New rule: similar colors --- front/src/css/rule.css | 11 ++++ front/src/less/rule.less | 13 ++++ front/src/views/rule.html | 4 ++ lib/metadata/policies.js | 19 +++++- lib/metadata/scoreProfileGeneric.json | 3 +- lib/runner.js | 4 ++ lib/tools/colorDiff.js | 90 ++++++++++++++++++++++++++ package.json | 2 + test/core/colorDiffTest.js | 92 +++++++++++++++++++++++++++ 9 files changed, 235 insertions(+), 3 deletions(-) create mode 100644 lib/tools/colorDiff.js create mode 100644 test/core/colorDiffTest.js diff --git a/front/src/css/rule.css b/front/src/css/rule.css index 24137f4..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; diff --git a/front/src/less/rule.less b/front/src/less/rule.less index 6f1fe61..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; diff --git a/front/src/views/rule.html b/front/src/views/rule.html index 0ca11f6..001b0f7 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -96,6 +96,10 @@
+
+
{{offender.color1}}
{{offender.color2}}
+
+
{{offender.error}} diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index fc6bbf0..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", @@ -970,7 +985,7 @@ var policies = { "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": 3, + "isOkThreshold": 1, "isBadThreshold": 12, "isAbnormalThreshold": 30, "hasOffenders": true diff --git a/lib/metadata/scoreProfileGeneric.json b/lib/metadata/scoreProfileGeneric.json index 8dd9670..30230ec 100644 --- a/lib/metadata/scoreProfileGeneric.json +++ b/lib/metadata/scoreProfileGeneric.json @@ -70,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/package.json b/package.json index 3a792b5..ea62935 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,7 @@ "body-parser": "1.13.3", "chart.js": "1.0.2", "clean-css": "3.4.2", + "color-diff": "0.1.7", "compression": "1.5.2", "cors": "2.7.1", "debug": "2.2.0", @@ -38,6 +39,7 @@ "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", 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 + } + ]); + }); + +}); From 82693a8587fd1c1d3520b6f75eaf9e2ab253fa47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Sun, 20 Sep 2015 01:29:45 +0200 Subject: [PATCH 5/5] v1.8 + update dependencies --- package.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index ea62935..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,15 @@ "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", @@ -43,7 +43,7 @@ "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" @@ -55,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",